Compare commits
35 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 491ce1d1f0 | |||
| a23c0217ae | |||
| 5c989fef2f | |||
| 5e5e10a8dc | |||
| 52a31072a3 | |||
| 7b40a03c45 | |||
| e9d32c09d3 | |||
| e89f0ce605 | |||
| 1278d57c12 | |||
| 14d91ef032 | |||
| 5f6aa3da69 | |||
| 01226cfc73 | |||
| 7054b75650 | |||
| dd4bba8913 | |||
| 876ef122be | |||
| b5b95de19a | |||
| 302235da23 | |||
| 7c751ef675 | |||
| c7b523a0a9 | |||
| fcf08647c5 | |||
| 244263430d | |||
| cf1438a525 | |||
| e27ce29e81 | |||
| ec4c8d81ae | |||
| 75b51028c3 | |||
| 6597e2408f | |||
| 517327aa1e | |||
| 00351b4551 | |||
| c6e89219e1 | |||
| c8fbcced3d | |||
| 685f6d19f4 | |||
| 509bad2c68 | |||
| ab8ff865e4 | |||
| c5534700f8 | |||
| 5687a71476 |
@@ -218,6 +218,31 @@ def is_red(status: dict) -> tuple[bool, list[dict]]:
|
||||
|
||||
`failed_statuses` is the list of per-context entries whose own
|
||||
`state` is in the red set; useful for the issue body.
|
||||
|
||||
Cancel-cascade filter (mc#1564, 2026-05-19):
|
||||
Gitea maps BOTH `action_run.status=2 (Failure)` AND
|
||||
`action_run.status=3 (Cancelled)` to commit-status string
|
||||
`"failure"`. On a busy main with
|
||||
`concurrency: cancel-in-progress: true`, every merge burst
|
||||
cancels prior in-flight runs (status=3) — those bubble to the
|
||||
combined-status `failure` and inflate the watchdog's red%,
|
||||
generating phantom `[main-red]` issues (mc#1562/#1552/#1540/...).
|
||||
Canonical Gitea 1.22.6 enum per `models/actions/status.go` +
|
||||
`reference_gitea_action_status_enum_corrected_2026_05_19`:
|
||||
1=Success, 2=Failure, 3=Cancelled, 4=Skipped,
|
||||
5=Waiting, 6=Running, 7=Blocked
|
||||
We only want status=2 (real defects) to file. At the
|
||||
commit-status layer we don't have the integer enum directly
|
||||
(only the `failure` rollup string), so we use the description
|
||||
string Gitea writes when a run is cancelled — empirically
|
||||
`"Has been cancelled"` (verified 2026-05-19 via #1562 body).
|
||||
Real failures show `"Failing after Ns"` and are unaffected.
|
||||
This is option B from mc#1564 (description-string filter, no
|
||||
extra API call). Description-string stability is a soft contract
|
||||
with Gitea; if a future release renames it, the cancel-cascade
|
||||
entries will simply leak back through (visible-not-silent), and
|
||||
we'll either re-pin the string or upgrade to option A (resolve
|
||||
the underlying action_run.status integer via target_url).
|
||||
"""
|
||||
combined = status.get("state")
|
||||
statuses = status.get("statuses") or []
|
||||
@@ -233,11 +258,30 @@ def is_red(status: dict) -> tuple[bool, list[dict]]:
|
||||
def _entry_state(s: dict) -> str:
|
||||
return s.get("status") or s.get("state") or ""
|
||||
|
||||
def _is_cancel_cascade(s: dict) -> bool:
|
||||
"""status=3 entry per Gitea 1.22.6 description-string contract.
|
||||
Match exactly (after strip) — substring match would catch
|
||||
legitimate test names like "Has been cancelled by the user
|
||||
unexpectedly" in failure logs."""
|
||||
desc = (s.get("description") or "").strip()
|
||||
return desc == "Has been cancelled"
|
||||
|
||||
failed = [
|
||||
s for s in statuses
|
||||
if isinstance(s, dict) and _entry_state(s) in red_states
|
||||
if isinstance(s, dict)
|
||||
and _entry_state(s) in red_states
|
||||
and not _is_cancel_cascade(s)
|
||||
]
|
||||
return (combined in red_states or bool(failed), failed)
|
||||
# Combined state alone is no longer sufficient — combined=failure
|
||||
# may be 100% cancel-cascade. Drive `red` off the FILTERED list:
|
||||
# if every red-shaped per-entry was cancel-cascade, `failed` is
|
||||
# empty and we report green. Combined-failure with no per-entry
|
||||
# detail (empty `statuses[]`) still trips red — that's the
|
||||
# "CI emitter set combined-status directly" edge case from
|
||||
# render_body's fallback path; we keep filing on it so the
|
||||
# operator sees the breadcrumb.
|
||||
combined_red_no_detail = combined in red_states and not statuses
|
||||
return (bool(failed) or combined_red_no_detail, failed)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
@@ -401,7 +401,7 @@ jobs:
|
||||
|
||||
canvas-deploy-reminder:
|
||||
name: Canvas Deploy Reminder
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: docker-host
|
||||
# mc#774 root-fix: added job-level `if:` so ci-required-drift.py's
|
||||
# ci_job_names() detects this as github.ref-gated and skips it from F1.
|
||||
# The step-level exit 0 handles the "not main push" case; the job-level
|
||||
|
||||
@@ -108,7 +108,20 @@ env:
|
||||
|
||||
jobs:
|
||||
detect-changes:
|
||||
runs-on: ubuntu-latest
|
||||
# mc#1529 follow-on: pin to `docker-host` so the e2e-api lane lands
|
||||
# on Linux operator-host runners (molecule-runner-*) that carry the
|
||||
# `molecule-core-net` bridge network + a working `aws ecr get-login-
|
||||
# password | docker login` path. The bare `ubuntu-latest` label is
|
||||
# also accepted by hongming-pc-runner-* (Windows act_runner v1.0.3),
|
||||
# where the docker.sock-bound steps below fail non-deterministically
|
||||
# (e.g. `docker run -d --name pg-e2e-api-...` with port-bind +
|
||||
# `docker exec ... pg_isready` cannot work against a Windows daemon).
|
||||
# detect-changes itself doesn't bind docker.sock, but pinning here too
|
||||
# keeps both jobs on the same lane so we don't re-roll the dice on
|
||||
# workspace-volume cross-host surprises and the routing rule is
|
||||
# discoverable in one place. Mirror of mc#1543 (handlers-postgres-
|
||||
# integration). See internal#512 for the class defect.
|
||||
runs-on: docker-host
|
||||
# Phase 3 (RFC #219 §1): surface broken workflows without blocking.
|
||||
# mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
@@ -160,7 +173,10 @@ jobs:
|
||||
e2e-api:
|
||||
needs: detect-changes
|
||||
name: E2E API Smoke Test
|
||||
runs-on: ubuntu-latest
|
||||
# mc#1529 follow-on: must run on operator-host Linux runners (where
|
||||
# docker.sock + `molecule-core-net` + `aws ecr ...` work). See
|
||||
# detect-changes for the full rationale.
|
||||
runs-on: docker-host
|
||||
# Phase 3 (RFC #219 §1): surface broken workflows without blocking.
|
||||
# mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
@@ -365,6 +381,9 @@ jobs:
|
||||
- name: Run poll-mode chat upload E2E (RFC #2891)
|
||||
if: needs.detect-changes.outputs.api == 'true'
|
||||
run: bash tests/e2e/test_poll_mode_chat_upload_e2e.sh
|
||||
- name: Run today's-PR-coverage E2E (mc#1525/1535/1536/1539/1542 fix-specific assertions)
|
||||
if: needs.detect-changes.outputs.api == 'true'
|
||||
run: bash tests/e2e/test_today_pr_coverage_e2e.sh
|
||||
- name: Dump platform log on failure
|
||||
if: failure() && needs.detect-changes.outputs.api == 'true'
|
||||
run: cat workspace-server/platform.log || true
|
||||
|
||||
@@ -33,7 +33,13 @@ env:
|
||||
jobs:
|
||||
# bp-exempt: helper job; real gate is E2E Chat / E2E Chat (pull_request)
|
||||
detect-changes:
|
||||
runs-on: ubuntu-latest
|
||||
# mc#1529 follow-on: pin to `docker-host` (Linux operator-host
|
||||
# runners). The bare `ubuntu-latest` label is also advertised by
|
||||
# hongming-pc-runner-* (Windows act_runner v1.0.3) where the
|
||||
# docker.sock-bound steps below fail. Mirror of mc#1543
|
||||
# (handlers-postgres-integration). See internal#512 for the class
|
||||
# defect.
|
||||
runs-on: docker-host
|
||||
# Phase 3 (RFC #219 §1): surface broken workflows without blocking.
|
||||
# mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
@@ -71,7 +77,9 @@ jobs:
|
||||
e2e-chat:
|
||||
needs: detect-changes
|
||||
name: E2E Chat
|
||||
runs-on: ubuntu-latest
|
||||
# mc#1529 follow-on: docker run/exec for postgres + redis containers.
|
||||
# Must land on operator-host Linux (docker-host).
|
||||
runs-on: docker-host
|
||||
# Phase 3 (RFC #219 §1): surface broken workflows without blocking.
|
||||
# mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
|
||||
@@ -164,7 +164,7 @@ jobs:
|
||||
# bp-required: pending #1296
|
||||
peer-visibility-local:
|
||||
name: E2E Peer Visibility (local)
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: docker-host
|
||||
timeout-minutes: 30
|
||||
env:
|
||||
# Per-run names + ephemeral ports — same collision-avoidance as
|
||||
|
||||
@@ -77,7 +77,16 @@ env:
|
||||
jobs:
|
||||
detect-changes:
|
||||
name: detect-changes
|
||||
runs-on: ubuntu-latest
|
||||
# mc#1529 §1: pin to `docker-host` so the integration job runs on the
|
||||
# operator-host runners (molecule-runner-*), which carry the
|
||||
# `molecule-core-net` bridge network this workflow depends on. PC2
|
||||
# runners (hongming-pc-runner-*) also advertise ubuntu-latest but
|
||||
# don't have that network — the previous `runs-on: ubuntu-latest`
|
||||
# rolled the dice and hard-failed the bridge-inspect step ~30% of
|
||||
# the time. detect-changes itself doesn't need the bridge, but keeping
|
||||
# both jobs on the same label avoids workspace-volume cross-host
|
||||
# surprises and keeps the routing rule discoverable in one place.
|
||||
runs-on: docker-host
|
||||
# mc#774 Phase 3 (RFC §1): surface broken workflows without blocking.
|
||||
# mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
@@ -129,7 +138,9 @@ jobs:
|
||||
integration:
|
||||
name: Handlers Postgres Integration
|
||||
needs: detect-changes
|
||||
runs-on: ubuntu-latest
|
||||
# mc#1529 §1: must run on operator-host (where `molecule-core-net`
|
||||
# exists). See detect-changes for the full routing rationale.
|
||||
runs-on: docker-host
|
||||
# mc#774 Phase 3 (RFC §1): surface broken workflows without blocking.
|
||||
# mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
|
||||
@@ -62,7 +62,13 @@ env:
|
||||
jobs:
|
||||
# bp-exempt: change detector only; downstream Harness Replays is the meaningful gate.
|
||||
detect-changes:
|
||||
runs-on: ubuntu-latest
|
||||
# mc#1529 follow-on: pin to `docker-host` so this lane lands on
|
||||
# Linux operator-host runners (the only ones with a working
|
||||
# docker.sock + `molecule-core-net`). The bare `ubuntu-latest`
|
||||
# label is also matched by hongming-pc-runner-* (Windows act_runner
|
||||
# v1.0.3), where the `docker compose ...` exec below fails. Mirror
|
||||
# of mc#1543; see internal#512 for class defect.
|
||||
runs-on: docker-host
|
||||
# Phase 3 (RFC #219 §1): surface broken workflows without blocking.
|
||||
# mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
@@ -162,7 +168,9 @@ jobs:
|
||||
harness-replays:
|
||||
needs: detect-changes
|
||||
name: Harness Replays
|
||||
runs-on: ubuntu-latest
|
||||
# mc#1529 follow-on: `docker compose ... ps/logs` against tenant-alpha/
|
||||
# beta containers. Must run on operator-host Linux (docker-host).
|
||||
runs-on: docker-host
|
||||
# Phase 3 (RFC #219 §1): surface broken workflows without blocking.
|
||||
# mc#774: pre-existing continue-on-error mask; root-fix and remove, do not renew silently.
|
||||
continue-on-error: true
|
||||
|
||||
@@ -0,0 +1,179 @@
|
||||
name: Lint no tenant GITEA/GITHUB token write
|
||||
|
||||
# Task #146 — CI guardrail companion to RFC#523's `lint-forbidden-env-keys.yml`.
|
||||
#
|
||||
# `lint-forbidden-env-keys.yml` (Layer 3) catches code that hardcodes a
|
||||
# forbidden env-var key NAME as a quoted literal in workspace_secrets
|
||||
# writer paths under workspace-server/internal/.
|
||||
#
|
||||
# This workflow catches a BROADER class: any code path that reads a
|
||||
# repo-host token (GITEA_TOKEN / GITHUB_TOKEN / GH_TOKEN) and then writes
|
||||
# it into a TENANT WORKSPACE's env, secret store, user-data, or
|
||||
# provision payload. This is the actual RFC#523 threat-model statement —
|
||||
# the goal is "no tenant workspace ever receives an operator-scope repo
|
||||
# token," not just "no _quoted_ literal `GITEA_TOKEN`." A future writer
|
||||
# could route the value via a variable, a struct field, or a config key
|
||||
# and slip past the existing literal scan; this lint catches those
|
||||
# routing patterns at PR review time.
|
||||
#
|
||||
# Scope
|
||||
# Scans the WHOLE repo's Go sources (not just workspace-server/) for
|
||||
# co-occurrences of:
|
||||
# - a repo-host token NAME (GITEA_TOKEN / GITHUB_TOKEN / GH_TOKEN /
|
||||
# GITEA_PAT / GITHUB_PAT) used as os.Getenv argument or string
|
||||
# literal
|
||||
# - within a file that ALSO references a tenant-writer surface
|
||||
# (`tenant`, `workspace_secrets`, `global_secrets`, `seedAllowList`,
|
||||
# `/settings/secrets`, `userData`, `provisionPayload`,
|
||||
# `envVars[`, `containerEnv`).
|
||||
#
|
||||
# Co-occurrence (not single-line) is the false-positive control: a
|
||||
# file that just LOGS the variable name (e.g. "missing GITEA_TOKEN")
|
||||
# without touching any tenant surface won't fire.
|
||||
#
|
||||
# Drift contract with lint-forbidden-env-keys.yml
|
||||
# Both lints share the same FORBIDDEN_KEYS list (a subset — only the
|
||||
# repo-host tokens, since this lint's threat model is "tenant gets
|
||||
# write access to operator's git host"). If RFC#523's deny set grows,
|
||||
# update BOTH this file AND lint-forbidden-env-keys.yml AND the Go
|
||||
# source-of-truth in
|
||||
# workspace-server/internal/handlers/workspace_provision_forbidden_env.go.
|
||||
#
|
||||
# Open-source-template-friendly
|
||||
# The patterns scanned are generic (no MOLECULE_-prefix literals).
|
||||
# A fork can copy this workflow as-is and adjust FORBIDDEN_KEYS.
|
||||
#
|
||||
# Path-filter discipline
|
||||
# No `paths:` filter — required-status workflows must run on every PR
|
||||
# per `feedback_path_filtered_workflow_cant_be_required`. Scan is
|
||||
# sub-second.
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [opened, synchronize, reopened]
|
||||
push:
|
||||
branches: [main, staging]
|
||||
|
||||
env:
|
||||
GITHUB_SERVER_URL: https://git.moleculesai.app
|
||||
|
||||
jobs:
|
||||
scan:
|
||||
name: Scan for repo-host token write into tenant workspace surface
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
fetch-depth: 1
|
||||
|
||||
- name: Find Go files referencing a tenant-writer surface AND a repo-host token
|
||||
run: |
|
||||
set -euo pipefail
|
||||
|
||||
# Repo-host token NAMES — the threat-model subset. Operator-fleet
|
||||
# tokens (CP_ADMIN_API_TOKEN, RAILWAY_TOKEN, INFISICAL_*) are
|
||||
# caught by lint-forbidden-env-keys.yml's broader deny set; this
|
||||
# lint focuses on the git-host class so a single co-occurrence
|
||||
# match has a low false-positive rate.
|
||||
FORBIDDEN_KEYS=(
|
||||
"GITEA_TOKEN"
|
||||
"GITEA_PAT"
|
||||
"GITHUB_TOKEN"
|
||||
"GITHUB_PAT"
|
||||
"GH_TOKEN"
|
||||
)
|
||||
|
||||
# Tenant-writer surface markers. A file matches the surface set
|
||||
# if it references ANY of these strings. This is the "is this
|
||||
# code path writing into a tenant workspace?" heuristic.
|
||||
# Curated to catch the actual code shapes used in this repo
|
||||
# (verified by grep against current main 2026-05-19):
|
||||
# - "workspace_secrets" / "global_secrets" → DB table writes
|
||||
# - "seedAllowList" → CP-side seed table
|
||||
# - "/settings/secrets" → tenant HTTP API write
|
||||
# - "envVars[" → in-memory env map write
|
||||
# - "containerEnv" → docker-run env-set
|
||||
# - "userData" → EC2 user-data script
|
||||
# - "provisionPayload" / "provisionContext" → provision-request shape
|
||||
SURFACE_PATTERN='workspace_secrets|global_secrets|seedAllowList|/settings/secrets|envVars\[|containerEnv|userData|provisionPayload|provisionContext'
|
||||
|
||||
# Files that legitimately reference these names AND a surface
|
||||
# marker, but do so for guard / strip / test / doc-comment
|
||||
# reasons. New entries require reviewer signoff and a one-line
|
||||
# justification in the diff.
|
||||
EXEMPT_FILES=(
|
||||
# RFC#523 L1 deny-set source-of-truth + tests
|
||||
"workspace-server/internal/handlers/workspace_provision_forbidden_env.go"
|
||||
"workspace-server/internal/handlers/workspace_provision_forbidden_env_test.go"
|
||||
# Forensic-#145 silent-strip denylist (defense-in-depth, by design lists the names)
|
||||
"workspace-server/internal/provisioner/provisioner.go"
|
||||
"workspace-server/internal/provisioner/provisioner_test.go"
|
||||
# Pre-RFC#523 persona-fallback / org-helper paths. The L1
|
||||
# fail-closed runs BEFORE these writers; downstream silent-strip
|
||||
# also covers them. See applyAgentGitHTTPCreds doc-comment.
|
||||
"workspace-server/internal/handlers/agent_git_identity.go"
|
||||
"workspace-server/internal/handlers/org_helpers.go"
|
||||
"workspace-server/internal/handlers/org.go"
|
||||
# CP→platform admin auth (NOT a tenant env write).
|
||||
"workspace-server/internal/provisioner/cp_provisioner.go"
|
||||
)
|
||||
|
||||
# Build an extended-regex alternation of forbidden keys.
|
||||
KEY_ALT="$(IFS='|'; echo "${FORBIDDEN_KEYS[*]}")"
|
||||
|
||||
# Find candidate files: Go non-test sources that contain a
|
||||
# tenant-writer surface marker.
|
||||
mapfile -t CANDIDATES < <(
|
||||
grep -rlE --include='*.go' --exclude='*_test.go' \
|
||||
"${SURFACE_PATTERN}" . 2>/dev/null \
|
||||
| sed 's|^\./||' \
|
||||
| sort -u
|
||||
)
|
||||
|
||||
if [ "${#CANDIDATES[@]}" -eq 0 ]; then
|
||||
echo "OK No tenant-writer-surface files found in tree (unexpected, but not a lint failure)."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
HITS=""
|
||||
for f in "${CANDIDATES[@]}"; do
|
||||
# Skip exempt files.
|
||||
skip=0
|
||||
for ex in "${EXEMPT_FILES[@]}"; do
|
||||
if [ "$f" = "$ex" ]; then skip=1; break; fi
|
||||
done
|
||||
[ "$skip" = "1" ] && continue
|
||||
|
||||
# File contains a surface marker; now grep for a forbidden
|
||||
# key NAME. We require a QUOTED-literal match to avoid
|
||||
# firing on a comment like "// also handle GITEA_TOKEN".
|
||||
#
|
||||
# The literal form catches:
|
||||
# - os.Getenv("GITEA_TOKEN")
|
||||
# - envVars["GITEA_TOKEN"] = ...
|
||||
# - {envKey: "GITEA_TOKEN", tenantKey: "GITEA_TOKEN"}
|
||||
# but not:
|
||||
# - // see GITEA_TOKEN below (no quotes)
|
||||
found=$(grep -nE "\"(${KEY_ALT})\"" "$f" 2>/dev/null || true)
|
||||
if [ -n "$found" ]; then
|
||||
HITS="${HITS}--- ${f} ---\n${found}\n"
|
||||
fi
|
||||
done
|
||||
|
||||
if [ -n "$HITS" ]; then
|
||||
echo "::error::Task #146 lint: repo-host token name(s) quoted in a tenant-writer-surface file:"
|
||||
printf "$HITS"
|
||||
echo ""
|
||||
echo "These files reference a tenant-writer surface (workspace_secrets,"
|
||||
echo "seedAllowList, /settings/secrets, containerEnv, userData, etc.)"
|
||||
echo "AND quote a repo-host token name (GITEA_TOKEN/GITHUB_TOKEN/…)."
|
||||
echo "Per RFC#523 threat model, tenant workspaces MUST NOT receive"
|
||||
echo "operator-scope repo-host tokens. If your code legitimately needs"
|
||||
echo "to reference one of these names in a tenant-writer file (e.g."
|
||||
echo "a deny-set definition or silent-strip list), add the file to"
|
||||
echo "EXEMPT_FILES with a one-line justification — reviewer signoff"
|
||||
echo "required."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "OK No tenant-writer-surface file co-mentions a repo-host token literal."
|
||||
@@ -0,0 +1,163 @@
|
||||
name: lint-required-workflows-docker-host-pinned
|
||||
|
||||
# Fail-closed lint that catches workflows touching docker.sock without
|
||||
# pinning `runs-on:` to a Linux-only label.
|
||||
#
|
||||
# Class defect (internal#512 + mc#1529 + today's oc#81/82/83 + autogen#8):
|
||||
# the `ubuntu-latest` label is advertised by BOTH the Linux operator-host
|
||||
# runners (molecule-runner-*) AND the Windows act_runner v1.0.3 on
|
||||
# hongming-pc-runner-*. Job placement is non-deterministic. When a docker-
|
||||
# bound job lands on a Windows runner, `docker run`/`docker login`/
|
||||
# `docker compose` fail with platform-specific errors ("protocol not
|
||||
# available", "cannot exec", etc.) — placement-dependent, not transient.
|
||||
#
|
||||
# This lint enforces the convention: any workflow whose YAML body
|
||||
# contains a docker exec (`docker run|build|buildx|compose|pull|push|
|
||||
# exec|tag|login|cp|inspect|ps` OR `docker/build-push-action|docker/
|
||||
# login-action|docker/setup-buildx`) MUST pin every job's `runs-on:` to
|
||||
# one of:
|
||||
# - docker-host (general docker.sock work — molecule-runner-*)
|
||||
# - publish (image build/push — molecule-runner-publish-*)
|
||||
#
|
||||
# Comments and heredoc/markdown bodies that merely MENTION docker are
|
||||
# excluded by the detection rule (see scan.py below).
|
||||
#
|
||||
# Per `feedback_never_skip_ci`: this is fail-closed (exit 1 on miss).
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
paths:
|
||||
- '.gitea/workflows/**'
|
||||
- '.github/workflows/**'
|
||||
push:
|
||||
branches: [main, staging]
|
||||
paths:
|
||||
- '.gitea/workflows/**'
|
||||
- '.github/workflows/**'
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
env:
|
||||
GITHUB_SERVER_URL: https://git.moleculesai.app
|
||||
|
||||
jobs:
|
||||
lint-docker-host-pin:
|
||||
name: Lint docker-host pin on docker-touching workflows
|
||||
runs-on: docker-host
|
||||
timeout-minutes: 5
|
||||
steps:
|
||||
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
|
||||
- name: Scan workflows for docker-bound jobs missing docker-host/publish pin
|
||||
run: |
|
||||
set -euo pipefail
|
||||
python3 - <<'PY'
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
# Docker-step detection: real exec, not just word-mention in comments.
|
||||
# We strip comment-only lines, then look for the docker subcommand
|
||||
# tokens at word-boundary, OR uses: docker/* actions.
|
||||
DOCKER_EXEC = re.compile(
|
||||
r'(?<!\w)docker\s+(run|build|buildx|compose|pull|push|exec|tag|login|cp|inspect|ps)\b'
|
||||
)
|
||||
DOCKER_ACTION = re.compile(
|
||||
r'uses:\s*docker/(build-push-action|login-action|setup-buildx-action|setup-qemu-action)'
|
||||
)
|
||||
# Detect a job header line like ` myjob:` (2-space indent) AND its runs-on.
|
||||
JOB_HEADER = re.compile(r'^( {2})([a-zA-Z0-9_-]+):\s*$')
|
||||
RUNS_ON = re.compile(r'^( {4})runs-on:\s*(.+?)\s*$')
|
||||
|
||||
ALLOWED_LABELS = {'docker-host', 'publish'}
|
||||
|
||||
fails = []
|
||||
warnings = []
|
||||
|
||||
roots = []
|
||||
for root in ('.gitea/workflows', '.github/workflows'):
|
||||
if os.path.isdir(root):
|
||||
roots.append(root)
|
||||
|
||||
for root in roots:
|
||||
for fn in sorted(os.listdir(root)):
|
||||
if not (fn.endswith('.yml') or fn.endswith('.yaml')):
|
||||
continue
|
||||
path = os.path.join(root, fn)
|
||||
with open(path) as f:
|
||||
raw_lines = f.readlines()
|
||||
|
||||
# Parse job headers + their runs-on. Simple line scan; relies on
|
||||
# 2-space job indent + 4-space runs-on indent under `jobs:`.
|
||||
jobs = []
|
||||
current = None
|
||||
in_jobs = False
|
||||
for i, line in enumerate(raw_lines, 1):
|
||||
if re.match(r'^jobs:\s*$', line):
|
||||
in_jobs = True
|
||||
continue
|
||||
if not in_jobs:
|
||||
continue
|
||||
mh = JOB_HEADER.match(line)
|
||||
if mh:
|
||||
if current:
|
||||
current['end'] = i - 1
|
||||
jobs.append(current)
|
||||
current = {'name': mh.group(2), 'line': i, 'end': len(raw_lines), 'runs_on': None}
|
||||
continue
|
||||
mr = RUNS_ON.match(line)
|
||||
if mr and current and current['runs_on'] is None:
|
||||
current['runs_on'] = mr.group(2).strip()
|
||||
if current:
|
||||
jobs.append(current)
|
||||
|
||||
for j in jobs:
|
||||
# Strip pure-comment lines for docker-exec detection so
|
||||
# documentation comments don't trigger the lint. Scan the
|
||||
# current job body only: a workflow may contain one
|
||||
# docker-bound job and several harmless metadata jobs.
|
||||
job_lines = raw_lines[j['line'] - 1:j['end']]
|
||||
scan_text = ''.join(
|
||||
l for l in job_lines
|
||||
if not re.match(r'^\s*#', l)
|
||||
)
|
||||
has_docker = bool(DOCKER_EXEC.search(scan_text)) or bool(DOCKER_ACTION.search(scan_text))
|
||||
if not has_docker:
|
||||
continue
|
||||
ro = j['runs_on']
|
||||
if ro is None:
|
||||
# Reusable workflow caller (`uses:` instead of `runs-on:`) —
|
||||
# skip; rule enforced in the called workflow.
|
||||
continue
|
||||
# Strip surrounding [ ] and quotes.
|
||||
ro_norm = ro.strip('[]').strip().strip('"\'')
|
||||
# Multi-label "[a, b]" — split.
|
||||
labels = [t.strip().strip('"\'') for t in ro_norm.split(',') if t.strip()]
|
||||
if any(lbl in ALLOWED_LABELS for lbl in labels):
|
||||
continue
|
||||
# Allow caller-supplied label expressions; spell the
|
||||
# marker indirectly so Gitea's expression parser does
|
||||
# not try to parse this Python heredoc.
|
||||
expression_marker = '$' + '{{'
|
||||
if any(expression_marker in lbl for lbl in labels):
|
||||
continue
|
||||
fails.append(
|
||||
f"{path}:{j['line']}: job `{j['name']}` uses docker but runs-on={ro!r} "
|
||||
f"(must be one of {sorted(ALLOWED_LABELS)})"
|
||||
)
|
||||
|
||||
if fails:
|
||||
print("FAIL: docker-bound jobs missing docker-host/publish pin:")
|
||||
for f in fails:
|
||||
print(f" - {f}")
|
||||
print()
|
||||
print("Why this rule exists (internal#512 + mc#1529):")
|
||||
print(" Bare `ubuntu-latest` is advertised by BOTH Linux operator-host")
|
||||
print(" runners AND Windows hongming-pc-runner-* (act_runner v1.0.3).")
|
||||
print(" Docker-bound jobs that land on Windows fail non-deterministically.")
|
||||
print(" Pin to `docker-host` (general) or `publish` (image build/push).")
|
||||
sys.exit(1)
|
||||
|
||||
print("OK: all docker-bound jobs are pinned to docker-host or publish.")
|
||||
PY
|
||||
@@ -29,6 +29,14 @@ on:
|
||||
pull_request:
|
||||
paths:
|
||||
- "workspace/**"
|
||||
# mc#1578 / a05add29 cure: build_runtime_package.py owns PYPROJECT_TEMPLATE
|
||||
# (deps, classifiers, project metadata). A change there is publish-affecting
|
||||
# even when workspace/** is untouched, so the autobump must fire to claim
|
||||
# the next runtime-v$VERSION tag. Without this, manual tagging races PyPI
|
||||
# (e.g. runtime-v0.1.18 collided with the 2026-04-27 PyPI 0.1.18 publish,
|
||||
# blocking the python-multipart pin from reaching prod).
|
||||
- "scripts/build_runtime_package.py"
|
||||
- "scripts/test_build_runtime_package.py"
|
||||
# Bump-and-tag on main/staging push (the actual operational trigger).
|
||||
push:
|
||||
branches:
|
||||
@@ -36,6 +44,8 @@ on:
|
||||
- staging
|
||||
paths:
|
||||
- "workspace/**"
|
||||
- "scripts/build_runtime_package.py"
|
||||
- "scripts/test_build_runtime_package.py"
|
||||
# Manual dispatch — useful when Gitea Actions API (/actions/*) is
|
||||
# unreachable (e.g. act_runner 404 on Gitea 1.22.6) and we cannot
|
||||
# re-trigger via curl.
|
||||
|
||||
@@ -1,11 +1,16 @@
|
||||
# Consolidated comment dispatcher for manual review/tier refires.
|
||||
# DEPRECATED — superseded by `.gitea/workflows/sop-checklist.yml`.
|
||||
#
|
||||
# The review-refire logic (qa/security/tier slash-command dispatch) has been
|
||||
# merged into sop-checklist.yml as the `review-refire` job. This workflow
|
||||
# is kept as a no-op stub to avoid a gap during the transition window where
|
||||
# this file may be deleted while sop-checklist.yml has not yet been merged.
|
||||
#
|
||||
# After sop-checklist.yml lands, this file will be deleted (issue #1280).
|
||||
#
|
||||
# Historical behavior (superseded):
|
||||
# Gitea 1.22 queues one run per workflow subscribed to `issue_comment` before
|
||||
# evaluating job-level `if:`. SOP-heavy PRs therefore created queue storms when
|
||||
# qa-review, security-review, sop-checklist, and sop-tier-refire all
|
||||
# listened to comments. This workflow is the single non-SOP comment subscriber:
|
||||
# ordinary comments no-op quickly; slash commands post the required status
|
||||
# contexts to the PR head SHA.
|
||||
# evaluating job-level `if:`. Previously this workflow was the single
|
||||
# non-SOP comment subscriber for qa/security/tier refire slash commands.
|
||||
|
||||
name: review-refire-comments
|
||||
|
||||
@@ -23,91 +28,12 @@ concurrency:
|
||||
cancel-in-progress: true
|
||||
|
||||
jobs:
|
||||
# No-op stub — all refire logic moved to sop-checklist.yml review-refire job.
|
||||
# Kept to avoid transition gap; will be deleted after sop-checklist.yml merges.
|
||||
dispatch:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Classify comment
|
||||
id: classify
|
||||
env:
|
||||
COMMENT_BODY: ${{ github.event.comment.body }}
|
||||
IS_PR: ${{ github.event.issue.pull_request != null }}
|
||||
- name: Deprecated — refire logic moved to sop-checklist.yml
|
||||
run: |
|
||||
set -euo pipefail
|
||||
{
|
||||
echo "run_qa=false"
|
||||
echo "run_security=false"
|
||||
echo "run_tier=false"
|
||||
} >> "$GITHUB_OUTPUT"
|
||||
if [ "$IS_PR" != "true" ]; then
|
||||
echo "::notice::not a PR comment; no-op"
|
||||
exit 0
|
||||
fi
|
||||
first_line=$(printf '%s\n' "$COMMENT_BODY" | sed -n '1p')
|
||||
case "$first_line" in
|
||||
/qa-recheck*)
|
||||
echo "run_qa=true" >> "$GITHUB_OUTPUT"
|
||||
;;
|
||||
/security-recheck*)
|
||||
echo "run_security=true" >> "$GITHUB_OUTPUT"
|
||||
;;
|
||||
/refire-tier-check*)
|
||||
echo "run_tier=true" >> "$GITHUB_OUTPUT"
|
||||
;;
|
||||
*)
|
||||
echo "::notice::no supported review refire slash command; no-op"
|
||||
;;
|
||||
esac
|
||||
|
||||
- name: Check out BASE ref for trusted scripts
|
||||
if: |
|
||||
steps.classify.outputs.run_qa == 'true' ||
|
||||
steps.classify.outputs.run_security == 'true' ||
|
||||
steps.classify.outputs.run_tier == 'true'
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
ref: ${{ github.event.repository.default_branch }}
|
||||
|
||||
- name: Refire qa-review status
|
||||
if: steps.classify.outputs.run_qa == 'true'
|
||||
env:
|
||||
GITEA_TOKEN: ${{ secrets.RFC_324_TEAM_READ_TOKEN || secrets.GITHUB_TOKEN }}
|
||||
GITEA_HOST: git.moleculesai.app
|
||||
REPO: ${{ github.repository }}
|
||||
PR_NUMBER: ${{ github.event.issue.number }}
|
||||
DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
|
||||
TEAM: qa
|
||||
TEAM_ID: '20'
|
||||
REVIEW_CHECK_DEBUG: '0'
|
||||
REVIEW_CHECK_STRICT: '0'
|
||||
COMMENT_AUTHOR: ${{ github.event.comment.user.login }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
.gitea/scripts/review-refire-status.sh
|
||||
|
||||
- name: Refire security-review status
|
||||
if: steps.classify.outputs.run_security == 'true'
|
||||
env:
|
||||
GITEA_TOKEN: ${{ secrets.RFC_324_TEAM_READ_TOKEN || secrets.GITHUB_TOKEN }}
|
||||
GITEA_HOST: git.moleculesai.app
|
||||
REPO: ${{ github.repository }}
|
||||
PR_NUMBER: ${{ github.event.issue.number }}
|
||||
DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
|
||||
TEAM: security
|
||||
TEAM_ID: '21'
|
||||
REVIEW_CHECK_DEBUG: '0'
|
||||
REVIEW_CHECK_STRICT: '0'
|
||||
COMMENT_AUTHOR: ${{ github.event.comment.user.login }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
.gitea/scripts/review-refire-status.sh
|
||||
|
||||
- name: Refire sop-tier-check status
|
||||
if: steps.classify.outputs.run_tier == 'true'
|
||||
env:
|
||||
GITEA_TOKEN: ${{ secrets.SOP_TIER_CHECK_TOKEN || secrets.GITHUB_TOKEN }}
|
||||
GITEA_HOST: git.moleculesai.app
|
||||
REPO: ${{ github.repository }}
|
||||
PR_NUMBER: ${{ github.event.issue.number }}
|
||||
COMMENT_AUTHOR: ${{ github.event.comment.user.login }}
|
||||
SOP_DEBUG: '0'
|
||||
run: bash .gitea/scripts/sop-tier-refire.sh
|
||||
echo "::warning::review-refire-comments.yml is deprecated. Refire logic is now in sop-checklist.yml review-refire job. This workflow is a no-op stub pending deletion (issue #1280)."
|
||||
exit 0
|
||||
|
||||
@@ -2,24 +2,20 @@
|
||||
#
|
||||
# RFC#351 Step 2 of 6 (implementation MVP).
|
||||
#
|
||||
# === DESIGN ===
|
||||
# === CONSOLIDATION (issue #1280) ===
|
||||
#
|
||||
# Goal: each PR must answer 7 SOP-checklist questions in its body,
|
||||
# and each item must have at least one /sop-ack <slug> comment from
|
||||
# a non-author peer in the required team. BP requires the
|
||||
# `sop-checklist / all-items-acked (pull_request)` status to merge.
|
||||
# This workflow is the SINGLE `issue_comment` subscriber — the logic from
|
||||
# `review-refire-comments.yml` has been merged in. Before this change:
|
||||
# - sop-checklist.yml (pre-2026-05-16) → issue_comment:[created,edited,deleted] → runner slot used, job no-oped
|
||||
# - review-refire-comments.yml → issue_comment:[created] → runner slot used, job no-oped
|
||||
# → every non-refire comment occupied 2 runner slots for ~800 s each
|
||||
# (~650 no-op runs/day, ~1,300 runner-slot-occupancy-hours/day).
|
||||
#
|
||||
# Triggers:
|
||||
# - `pull_request_target`: opened, edited, synchronize, reopened
|
||||
# → fires when PR opens, body is edited (refire — RFC#351 §4),
|
||||
# or new code is pushed (head.sha changes → stale status would
|
||||
# be auto-discarded by BP via dismiss_stale_reviews, but the
|
||||
# status itself is per-SHA so we re-post on the new head).
|
||||
# - `issue_comment`: created, edited, deleted
|
||||
# → fires on any new comment so /sop-ack / /sop-revoke take
|
||||
# effect immediately (Gitea 1.22.6 doesn't refire on
|
||||
# pull_request_review per feedback_pull_request_review_no_refire,
|
||||
# so issue_comment is the canonical refire channel).
|
||||
# Fix (PR #1345 / issue #1280):
|
||||
# - ONE workflow, ONE issue_comment:[created] subscription (no edited/deleted)
|
||||
# - all-items-acked job: pull_request_target OR sop slash-command comments
|
||||
# - review-refire job: qa/security/tier refire slash commands
|
||||
# → ~50% reduction in comment-triggered runner occupancy vs pre-fix.
|
||||
#
|
||||
# Trust boundary (mirrors RFC#324 §A4 + sop-tier-check security note):
|
||||
# `pull_request_target` (not `pull_request`) — workflow def is loaded
|
||||
@@ -51,7 +47,7 @@
|
||||
# /sop-ack <slug-or-numeric-alias> [optional note]
|
||||
# — register a peer-ack for one checklist item.
|
||||
# — slug accepts kebab-case, snake_case, or natural-spaces
|
||||
# (all normalize to canonical kebab-case).
|
||||
# (all normalized to canonical kebab-case).
|
||||
# — numeric 1..7 maps via config.items[*].numeric_alias.
|
||||
# — most-recent (user, slug) directive wins.
|
||||
#
|
||||
@@ -61,6 +57,13 @@
|
||||
# — most-recent (user, slug) directive wins, so a later /sop-ack
|
||||
# re-restores the ack.
|
||||
#
|
||||
# /sop-n/a <gate> [reason]
|
||||
# — declare a gate (qa-review, security-review) N/A.
|
||||
# — see sop-checklist-config.yaml n/a_gates section.
|
||||
#
|
||||
# /qa-recheck /security-recheck /refire-tier-check
|
||||
# — refire the corresponding status check on the PR head.
|
||||
#
|
||||
# The eval is read-only + idempotent (read PR + comments + team
|
||||
# membership, compute, post status). Re-running on any event is safe —
|
||||
# the new status overwrites the previous one for the same context.
|
||||
@@ -79,7 +82,10 @@ on:
|
||||
pull_request_target:
|
||||
types: [opened, edited, synchronize, reopened, labeled, unlabeled]
|
||||
issue_comment:
|
||||
types: [created, edited, deleted]
|
||||
types: [created] # NOT [created, edited, deleted] — Gitea 1.22.6 holds a runner slot
|
||||
# at job-parsing time, before job-level if: guards run. edited/deleted events
|
||||
# occupied ~1,300 runner-slot-hours/day on this workflow alone during the
|
||||
# 2026-05-16 freeze. Per PR #1345 fix.
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
@@ -88,10 +94,10 @@ permissions:
|
||||
secrets: read
|
||||
|
||||
jobs:
|
||||
# sop-checklist gate: runs on PR lifecycle events OR sop slash commands.
|
||||
# All other comment types (no-op text comments) no longer assign a runner
|
||||
# because this job's if: guard short-circuits before runner assignment.
|
||||
all-items-acked:
|
||||
# Run on pull_request_target events always. On issue_comment events,
|
||||
# only when the comment is on a PR (issue_comment fires for issues
|
||||
# too) and the body contains one of the slash-commands.
|
||||
if: |
|
||||
github.event_name == 'pull_request_target' ||
|
||||
(github.event_name == 'issue_comment' &&
|
||||
@@ -125,3 +131,95 @@ jobs:
|
||||
--pr "$PR_NUMBER" \
|
||||
--config .gitea/sop-checklist-config.yaml \
|
||||
--gitea-host git.moleculesai.app
|
||||
|
||||
# bp-exempt: informational refire handler, not a merge gate. Emits
|
||||
# qa-review/security-review status updates on /qa-recheck et al slash commands.
|
||||
review-refire:
|
||||
if: |
|
||||
github.event_name == 'issue_comment' &&
|
||||
github.event.issue.pull_request != null
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Classify comment
|
||||
id: classify
|
||||
env:
|
||||
COMMENT_BODY: ${{ github.event.comment.body }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
{
|
||||
echo "run_qa=false"
|
||||
echo "run_security=false"
|
||||
echo "run_tier=false"
|
||||
} >> "$GITHUB_OUTPUT"
|
||||
first_line=$(printf '%s\n' "$COMMENT_BODY" | sed -n '1p')
|
||||
case "$first_line" in
|
||||
/qa-recheck*)
|
||||
echo "run_qa=true" >> "$GITHUB_OUTPUT"
|
||||
;;
|
||||
/security-recheck*)
|
||||
echo "run_security=true" >> "$GITHUB_OUTPUT"
|
||||
;;
|
||||
/refire-tier-check*)
|
||||
echo "run_tier=true" >> "$GITHUB_OUTPUT"
|
||||
;;
|
||||
*)
|
||||
echo "::notice::no supported review refire slash command; no-op"
|
||||
;;
|
||||
esac
|
||||
|
||||
- name: Check out BASE ref for trusted scripts
|
||||
if: |
|
||||
steps.classify.outputs.run_qa == 'true' ||
|
||||
steps.classify.outputs.run_security == 'true' ||
|
||||
steps.classify.outputs.run_tier == 'true'
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
with:
|
||||
ref: ${{ github.event.repository.default_branch }}
|
||||
|
||||
- name: Refire qa-review status
|
||||
if: steps.classify.outputs.run_qa == 'true'
|
||||
env:
|
||||
# RFC_324_TEAM_READ_TOKEN is read-only (team membership read scope only).
|
||||
# review-refire-status.sh POSTs to /statuses — requires write scope.
|
||||
# SOP_TIER_CHECK_TOKEN carries write:repository + write:issue + read:organization.
|
||||
GITEA_TOKEN: ${{ secrets.SOP_TIER_CHECK_TOKEN || secrets.GITHUB_TOKEN }}
|
||||
GITEA_HOST: git.moleculesai.app
|
||||
REPO: ${{ github.repository }}
|
||||
PR_NUMBER: ${{ github.event.issue.number }}
|
||||
DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
|
||||
TEAM: qa
|
||||
TEAM_ID: '20'
|
||||
REVIEW_CHECK_DEBUG: '0'
|
||||
REVIEW_CHECK_STRICT: '0'
|
||||
run: |
|
||||
set -euo pipefail
|
||||
.gitea/scripts/review-refire-status.sh
|
||||
|
||||
- name: Refire security-review status
|
||||
if: steps.classify.outputs.run_security == 'true'
|
||||
env:
|
||||
# RFC_324_TEAM_READ_TOKEN is read-only (team membership read scope only).
|
||||
# review-refire-status.sh POSTs to /statuses — requires write scope.
|
||||
# SOP_TIER_CHECK_TOKEN carries write:repository + write:issue + read:organization.
|
||||
GITEA_TOKEN: ${{ secrets.SOP_TIER_CHECK_TOKEN || secrets.GITHUB_TOKEN }}
|
||||
GITEA_HOST: git.moleculesai.app
|
||||
REPO: ${{ github.repository }}
|
||||
PR_NUMBER: ${{ github.event.issue.number }}
|
||||
DEFAULT_BRANCH: ${{ github.event.repository.default_branch }}
|
||||
TEAM: security
|
||||
TEAM_ID: '21'
|
||||
REVIEW_CHECK_DEBUG: '0'
|
||||
REVIEW_CHECK_STRICT: '0'
|
||||
run: |
|
||||
set -euo pipefail
|
||||
.gitea/scripts/review-refire-status.sh
|
||||
|
||||
- name: Refire sop-tier-check status
|
||||
if: steps.classify.outputs.run_tier == 'true'
|
||||
env:
|
||||
GITEA_TOKEN: ${{ secrets.SOP_TIER_CHECK_TOKEN || secrets.GITHUB_TOKEN }}
|
||||
GITEA_HOST: git.moleculesai.app
|
||||
REPO: ${{ github.repository }}
|
||||
PR_NUMBER: ${{ github.event.issue.number }}
|
||||
SOP_DEBUG: '0'
|
||||
run: bash .gitea/scripts/sop-tier-refire.sh
|
||||
|
||||
@@ -300,7 +300,7 @@ jobs:
|
||||
|
||||
canvas-deploy-reminder:
|
||||
name: Canvas Deploy Reminder
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: docker-host
|
||||
needs: [changes, canvas-build]
|
||||
# Only fires on direct pushes to main (i.e. after staging→main promotion).
|
||||
if: needs.changes.outputs.canvas == 'true' && github.event_name == 'push' && github.ref == 'refs/heads/main'
|
||||
@@ -440,4 +440,3 @@ jobs:
|
||||
|
||||
# SDK + plugin validation moved to standalone repo:
|
||||
# github.com/molecule-ai/molecule-sdk-python
|
||||
|
||||
|
||||
@@ -128,7 +128,7 @@ jobs:
|
||||
e2e-api:
|
||||
needs: detect-changes
|
||||
name: E2E API Smoke Test
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: docker-host
|
||||
timeout-minutes: 15
|
||||
env:
|
||||
# Unique per-run container names so concurrent runs on the host-
|
||||
|
||||
@@ -88,7 +88,7 @@ jobs:
|
||||
integration:
|
||||
name: Handlers Postgres Integration
|
||||
needs: detect-changes
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: docker-host
|
||||
env:
|
||||
# Unique name per run so concurrent jobs don't collide on the
|
||||
# bridge network. ${RUN_ID}-${RUN_ATTEMPT} is unique even across
|
||||
@@ -249,4 +249,3 @@ jobs:
|
||||
# already gone (e.g. concurrent rerun race), don't fail the job.
|
||||
docker rm -f "${PG_NAME}" >/dev/null 2>&1 || true
|
||||
echo "Cleaned up ${PG_NAME}"
|
||||
|
||||
|
||||
@@ -102,7 +102,7 @@ jobs:
|
||||
harness-replays:
|
||||
needs: detect-changes
|
||||
name: Harness Replays
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: docker-host
|
||||
timeout-minutes: 30
|
||||
steps:
|
||||
- name: No-op pass (paths filter excluded this commit)
|
||||
|
||||
@@ -39,7 +39,7 @@ env:
|
||||
jobs:
|
||||
build-and-push:
|
||||
name: Build & push canvas image
|
||||
runs-on: ubuntu-latest
|
||||
runs-on: publish
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
||||
|
||||
+8
-7
@@ -69,7 +69,7 @@ or other removed paths — open against `molecule-ai/docs` instead.
|
||||
| OG images, visual assets | `molecule-ai/docs` → `app/` or `marketing/` |
|
||||
| SEO briefs | `molecule-ai/docs` → `marketing/` |
|
||||
| DevRel demos (runnable code) | Standalone repo under `molecule-ai/`, OR embedded in `molecule-ai/docs` |
|
||||
| Launch checklists, internal tracking | GitHub Issues — **not** committed files |
|
||||
| Launch checklists, internal tracking | Gitea Issues — **not** committed files |
|
||||
| Engineering docs (`docs/adr/`, `docs/architecture/`, `docs/incidents/`) | This repo (internal, not published) |
|
||||
| Live product pages (e.g. `canvas/src/app/pricing/page.tsx`) | This repo (these are app code, not marketing copy) |
|
||||
|
||||
@@ -106,7 +106,7 @@ causing a render loop when any node position changed.
|
||||
|
||||
#### Auto-merge & the "extra commit" trap
|
||||
|
||||
**Two system guards protect against pushing commits after auto-merge has been enabled.** Don't try to work around them — they exist because we shipped a half-merged PR on 2026-04-27 (`#2174` merged with only its first commit; the second was orphaned on a branch GitHub had already deleted).
|
||||
**Two system guards protect against pushing commits after auto-merge has been enabled.** Don't try to work around them — they exist because we shipped a half-merged PR on 2026-04-27 (`#2174` merged with only its first commit; the second was orphaned on a branch the host had already deleted).
|
||||
|
||||
1. **Repo-wide:** "Automatically delete head branches" is on. Once a PR merges, the branch is deleted server-side. Any subsequent `git push` to that branch fails with `remote rejected — no such branch`.
|
||||
|
||||
@@ -145,7 +145,7 @@ Fix violations before committing — the hook will reject the commit.
|
||||
|
||||
### CI Pipeline
|
||||
|
||||
CI runs on GitHub Actions with a self-hosted runner. External contributors:
|
||||
CI runs on Gitea Actions with self-hosted runners. External contributors:
|
||||
PRs from forks will not trigger CI automatically. A maintainer will review
|
||||
and run CI manually.
|
||||
|
||||
@@ -192,7 +192,7 @@ live in their own repos:
|
||||
|
||||
- [`molecule-ai/molecule-ai-workspace-runtime`](https://git.moleculesai.app/molecule-ai/molecule-ai-workspace-runtime) — Python adapter SDK (`molecule_runtime`) that runs inside containerized Molecule workspaces. Bridges Claude Code SDK / hermes / langgraph / etc. → A2A queue.
|
||||
- [`molecule-ai/molecule-sdk-python`](https://git.moleculesai.app/molecule-ai/molecule-sdk-python) — `A2AServer` + `RemoteAgentClient` for external agents that register over the public `/registry/register` flow.
|
||||
- [`molecule-ai/molecule-mcp-claude-channel`](https://git.moleculesai.app/molecule-ai/molecule-mcp-claude-channel) — Claude Code channel plugin. Bridges A2A traffic into a running Claude Code session via MCP `notifications/claude/channel`. Polling-based (no tunnel required); install inside Claude Code via `/plugin marketplace add https://git.moleculesai.app/molecule-ai/molecule-mcp-claude-channel.git` → `/plugin install molecule@molecule-channel`, then launch with `claude --dangerously-load-development-channels --channels plugin:molecule@molecule-channel`.
|
||||
- [`molecule-ai/molecule-mcp-claude-channel`](https://git.moleculesai.app/molecule-ai/molecule-mcp-claude-channel) — Claude Code channel plugin. Bridges A2A traffic into a running Claude Code session via MCP `notifications/claude/channel`. Polling-based (no tunnel required); install inside Claude Code via `/plugin marketplace add https://git.moleculesai.app/molecule-ai/molecule-mcp-claude-channel.git` → `/plugin install molecule@molecule-channel`, then launch with `claude --dangerously-load-development-channels=plugin:molecule@molecule-channel`.
|
||||
|
||||
When extending the **A2A surface** in molecule-core (`workspace-server/internal/handlers/a2a_proxy.go` etc.), consider whether the change has a downstream impact on the runtime SDK or the channel plugin — they're versioned independently but share the wire shape.
|
||||
|
||||
@@ -206,7 +206,7 @@ See `CLAUDE.md` for detailed architecture documentation, including:
|
||||
|
||||
## Reporting Issues
|
||||
|
||||
Use GitHub Issues with a clear title and reproduction steps. Include:
|
||||
Use Gitea Issues with a clear title and reproduction steps. Include:
|
||||
- What you expected
|
||||
- What actually happened
|
||||
- Platform/OS version
|
||||
@@ -214,8 +214,9 @@ Use GitHub Issues with a clear title and reproduction steps. Include:
|
||||
|
||||
## Security
|
||||
|
||||
If you discover a security vulnerability, please report it privately via
|
||||
GitHub Security Advisories rather than opening a public issue.
|
||||
If you discover a security vulnerability, please report it privately by
|
||||
opening an issue against `molecule-ai/internal` (a private repo only
|
||||
maintainers can see) rather than filing a public issue here.
|
||||
|
||||
## License
|
||||
|
||||
|
||||
@@ -238,7 +238,7 @@ The result is not just “an agent that learns.” It is **an organization that
|
||||
- subscribe to one or more workspaces; peer messages surface as conversation turns; replies route back through Molecule's A2A
|
||||
- no tunnel, no public endpoint — the plugin self-registers each watched workspace as `delivery_mode=poll` and long-polls `/activity?since_id=…`
|
||||
- multi-tenant friendly: one plugin install can watch workspaces across multiple Molecule tenants (`MOLECULE_PLATFORM_URLS` per-workspace)
|
||||
- install via the standard marketplace flow: `/plugin marketplace add https://git.moleculesai.app/molecule-ai/molecule-mcp-claude-channel.git` → `/plugin install molecule@molecule-channel`, then launch with `claude --dangerously-load-development-channels --channels plugin:molecule@molecule-channel`
|
||||
- install via the standard marketplace flow: `/plugin marketplace add https://git.moleculesai.app/molecule-ai/molecule-mcp-claude-channel.git` → `/plugin install molecule@molecule-channel`, then launch with `claude --dangerously-load-development-channels=plugin:molecule@molecule-channel`
|
||||
|
||||
## Built For Teams That Need More Than A Demo
|
||||
|
||||
|
||||
+1
-1
@@ -237,7 +237,7 @@ Molecule AI 并不是要替代下面这些 framework,而是把它们纳入更
|
||||
- 订阅一个或多个 workspace;peer 的消息会以 user-turn 出现,回复会经 Molecule A2A 路由出去
|
||||
- 无需公网隧道、无需公开端点 —— 插件启动时自动把每个 watched workspace 注册成 `delivery_mode=poll`,长轮询 `/activity?since_id=…`
|
||||
- 多租户友好:单次安装即可同时 watch 跨多个 Molecule 租户的 workspace(`MOLECULE_PLATFORM_URLS` 按 workspace 配置)
|
||||
- 通过标准 marketplace 流程安装:`/plugin marketplace add https://git.moleculesai.app/molecule-ai/molecule-mcp-claude-channel.git` → `/plugin install molecule@molecule-channel`,然后用 `claude --dangerously-load-development-channels --channels plugin:molecule@molecule-channel` 启动
|
||||
- 通过标准 marketplace 流程安装:`/plugin marketplace add https://git.moleculesai.app/molecule-ai/molecule-mcp-claude-channel.git` → `/plugin install molecule@molecule-channel`,然后用 `claude --dangerously-load-development-channels=plugin:molecule@molecule-channel` 启动
|
||||
|
||||
## 适合什么团队
|
||||
|
||||
|
||||
+2
-1
@@ -1 +1,2 @@
|
||||
trigger
|
||||
trigger
|
||||
retrigger 2026-05-20T04:09Z after op-config#110 (HOME=/home/runner) deploy to fleet — internal#603
|
||||
|
||||
@@ -0,0 +1,179 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import {
|
||||
uploadChatFiles,
|
||||
FileTooLargeError,
|
||||
MAX_UPLOAD_BYTES,
|
||||
computeUploadTimeoutMs,
|
||||
} from "../uploads";
|
||||
|
||||
// Tests for the 100 MB upload-cap raise + correct-reason error mapping
|
||||
// (CTO 2026-05-19 directive on forensic a99ab0a1: "if its file size
|
||||
// issue, should have error that instead saying timeout which is
|
||||
// wrong"). Each case has its own specific reason; conflation is the
|
||||
// bug this PR fixes.
|
||||
|
||||
// File constructor in node's vitest env supports size via array length.
|
||||
// Allocate a typed-array of N bytes and wrap it — File reads .size off
|
||||
// the underlying Blob. Allocating 101 MB once per test is fine (vitest
|
||||
// maxWorkers=1, single test process).
|
||||
function makeFile(name: string, size: number): File {
|
||||
const buf = new Uint8Array(size);
|
||||
return new File([buf], name);
|
||||
}
|
||||
|
||||
const wsId = "00000000-0000-0000-0000-000000000001";
|
||||
|
||||
describe("uploadChatFiles — MAX_UPLOAD_BYTES + pre-flight gate", () => {
|
||||
it("MAX_UPLOAD_BYTES is exactly 100 MB (mirrors server constant)", () => {
|
||||
// Pinned so a regression that flipped the constant back to 50 MB
|
||||
// would fail loudly here — without this the canvas would
|
||||
// silently start rejecting files the server now accepts.
|
||||
expect(MAX_UPLOAD_BYTES).toBe(100 * 1024 * 1024);
|
||||
});
|
||||
|
||||
it("throws FileTooLargeError for a 101 MB file BEFORE any network I/O", async () => {
|
||||
const oversize = makeFile("big.bin", 101 * 1024 * 1024);
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
try {
|
||||
await uploadChatFiles(wsId, [oversize]);
|
||||
throw new Error("expected uploadChatFiles to throw, but it resolved");
|
||||
} catch (e) {
|
||||
// The exact class name matters — useChatSend's mapUploadErrorToReason
|
||||
// routes off `instanceof FileTooLargeError`. A regression that
|
||||
// demoted to a plain Error would silently re-introduce the
|
||||
// wrong-reason conflation CTO flagged.
|
||||
expect(e).toBeInstanceOf(FileTooLargeError);
|
||||
const err = e as FileTooLargeError;
|
||||
// Message must contain the 100MB cap (so the user knows what the
|
||||
// limit is) and a number-with-MB form of the actual size.
|
||||
expect(err.message).toContain("100MB");
|
||||
// Some toFixed(1) renderings: 101.0MB. Loose match: contains "MB".
|
||||
expect(err.message).toMatch(/got\s+\d+(\.\d+)?MB/);
|
||||
expect(err.fileSize).toBe(101 * 1024 * 1024);
|
||||
}
|
||||
// CRITICAL: no fetch may have been initiated. Pre-flight is the
|
||||
// whole point — if a network round-trip happened we'd be back to
|
||||
// surfacing a downstream timeout / 413 instead of the actionable
|
||||
// file-size message.
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("accepts a file exactly at the cap (== MAX_UPLOAD_BYTES)", async () => {
|
||||
// Equality must NOT trip the gate — the cap is inclusive on the
|
||||
// server side and the canvas must match. Without this, an exact-
|
||||
// cap file would 503 client-side while the server accepts it.
|
||||
const exact = makeFile("max.bin", MAX_UPLOAD_BYTES);
|
||||
const fetchSpy = vi
|
||||
.spyOn(globalThis, "fetch")
|
||||
.mockResolvedValue(
|
||||
new Response(JSON.stringify({ files: [] }), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
await expect(uploadChatFiles(wsId, [exact])).resolves.toBeDefined();
|
||||
expect(fetchSpy).toHaveBeenCalledOnce();
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeUploadTimeoutMs — scaled timeout curve", () => {
|
||||
it("100 KB file → 60s floor (small-file ergonomics)", () => {
|
||||
// Below the floor, the small-file UX (typo'd hostname surfacing as
|
||||
// connect-error quickly) takes priority over the slow-uplink
|
||||
// assumption.
|
||||
expect(computeUploadTimeoutMs(100 * 1024)).toBe(60_000);
|
||||
});
|
||||
|
||||
it("1 MB file → 60s floor", () => {
|
||||
expect(computeUploadTimeoutMs(1 * 1024 * 1024)).toBe(60_000);
|
||||
});
|
||||
|
||||
it("100 MB file → ~1000s (matches the slow-uplink design budget)", () => {
|
||||
// Pin the upper-bound case the design targets: at 100 MB / 100 KB/s
|
||||
// a legitimate slow uplink completes in ~1000s, comfortably
|
||||
// before the platform's 1200s http.Client timeout. Without this
|
||||
// scaling, the previous fixed 60s deadline aborted Ryan's ~60 MB
|
||||
// upload in forensic a99ab0a1.
|
||||
const ms = computeUploadTimeoutMs(100 * 1024 * 1024);
|
||||
// 100*1024*1024 / 100 = 1048576 ms ≈ 1048.6s — pin to ±1ms.
|
||||
expect(ms).toBe(Math.floor((100 * 1024 * 1024) / 100));
|
||||
expect(ms).toBeGreaterThan(1_000_000);
|
||||
expect(ms).toBeLessThan(1_100_000);
|
||||
});
|
||||
|
||||
it("strictly monotonic above the floor", () => {
|
||||
// A regression that capped or non-monotonised the curve would
|
||||
// silently re-introduce premature aborts for mid-size files.
|
||||
const a = computeUploadTimeoutMs(10 * 1024 * 1024);
|
||||
const b = computeUploadTimeoutMs(50 * 1024 * 1024);
|
||||
const c = computeUploadTimeoutMs(100 * 1024 * 1024);
|
||||
expect(b).toBeGreaterThan(a);
|
||||
expect(c).toBeGreaterThan(b);
|
||||
});
|
||||
});
|
||||
|
||||
describe("uploadChatFiles — error path shapes (for downstream reason-mapping)", () => {
|
||||
let fetchSpy: ReturnType<typeof vi.spyOn> | null = null;
|
||||
|
||||
beforeEach(() => {
|
||||
fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
});
|
||||
afterEach(() => {
|
||||
fetchSpy?.mockRestore();
|
||||
fetchSpy = null;
|
||||
});
|
||||
|
||||
it("propagates the server's 413 reason verbatim (not as 'timeout')", async () => {
|
||||
// The error message text is what useChatSend surfaces via
|
||||
// `Upload failed: ${e.message}` — pin that the server's reason
|
||||
// is present, not swallowed.
|
||||
fetchSpy!.mockResolvedValue(
|
||||
new Response('{"error":"file exceeds per-file limit (100 MB)"}', {
|
||||
status: 413,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
const f = makeFile("small.bin", 1024);
|
||||
await expect(uploadChatFiles(wsId, [f])).rejects.toThrow(
|
||||
/upload failed:.*413.*per-file limit/i,
|
||||
);
|
||||
});
|
||||
|
||||
it("propagates AbortSignal timeout as a DOMException with name=TimeoutError", async () => {
|
||||
// Reason-routing in useChatSend.mapUploadErrorToReason discriminates
|
||||
// by e.name === 'TimeoutError'. Pin the shape so a future browser /
|
||||
// polyfill change that renamed it would fail loudly here, NOT
|
||||
// silently fall through to the generic "Upload failed" path
|
||||
// (which is what made forensic a99ab0a1 hard to root-cause).
|
||||
const abortErr = new DOMException("signal timed out", "TimeoutError");
|
||||
fetchSpy!.mockRejectedValue(abortErr);
|
||||
const f = makeFile("small.bin", 1024);
|
||||
try {
|
||||
await uploadChatFiles(wsId, [f]);
|
||||
throw new Error("expected throw");
|
||||
} catch (e) {
|
||||
expect(e).toBeInstanceOf(DOMException);
|
||||
expect((e as DOMException).name).toBe("TimeoutError");
|
||||
// CRITICAL negative: the rejection must NOT be a
|
||||
// FileTooLargeError, because pre-flight already excluded that.
|
||||
expect(e).not.toBeInstanceOf(FileTooLargeError);
|
||||
}
|
||||
});
|
||||
|
||||
it("a 50 MB file does NOT trip the pre-flight gate (sub-cap)", async () => {
|
||||
// The forensic case: Ryan's file was over the OLD 50MB cap but
|
||||
// under the NEW 100MB cap. Pin that the pre-flight does NOT
|
||||
// misfire on a sub-100MB file.
|
||||
fetchSpy!.mockResolvedValue(
|
||||
new Response('{"files":[]}', {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
const f = makeFile("ryan.bin", 50 * 1024 * 1024);
|
||||
await expect(uploadChatFiles(wsId, [f])).resolves.toBeDefined();
|
||||
expect(fetchSpy!).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,79 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { mapUploadErrorToReason } from "../useChatSend";
|
||||
import { FileTooLargeError } from "../../uploads";
|
||||
|
||||
// Pin the case-by-case error mapping (CTO 2026-05-19 directive on
|
||||
// forensic a99ab0a1: each cause maps to ITS OWN message, no
|
||||
// conflation). The four cases — FileTooLargeError, TimeoutError,
|
||||
// other Error, non-Error — are the entire user-facing contract this
|
||||
// PR ships; each gets a dedicated assertion so a regression that
|
||||
// re-conflated them would surface here.
|
||||
|
||||
describe("mapUploadErrorToReason", () => {
|
||||
it("FileTooLargeError → surfaces the pre-flight message verbatim", () => {
|
||||
const err = new FileTooLargeError(
|
||||
101 * 1024 * 1024,
|
||||
"File too large (got 101.0MB) — limit is 100MB. Please use a smaller file.",
|
||||
);
|
||||
const out = mapUploadErrorToReason(err);
|
||||
// Verbatim, no "Upload failed:" prefix — the FileTooLargeError
|
||||
// message is already a complete, user-facing sentence.
|
||||
expect(out).toBe(err.message);
|
||||
expect(out).not.toMatch(/^Upload failed:/);
|
||||
// Must mention the cap so the user knows what to aim for.
|
||||
expect(out).toContain("100MB");
|
||||
// Must NOT mention timeout — wrong-reason conflation guard.
|
||||
expect(out.toLowerCase()).not.toContain("timeout");
|
||||
expect(out.toLowerCase()).not.toContain("connection");
|
||||
});
|
||||
|
||||
it("TimeoutError → connection-too-slow message, NOT file-size", () => {
|
||||
const err = new DOMException("signal timed out", "TimeoutError");
|
||||
const out = mapUploadErrorToReason(err);
|
||||
// The user-facing reason matches the design contract: tells the
|
||||
// user the connection is slow, gives them the actionable retry
|
||||
// hint, and does NOT mention file-size (pre-flight already
|
||||
// excluded that — this is the case CTO flagged).
|
||||
expect(out).toContain("Upload timed out");
|
||||
expect(out).toContain("connection is too slow");
|
||||
// CRITICAL negatives — guard against the wrong-reason conflation.
|
||||
expect(out).not.toMatch(/100MB|file too large|File too large/);
|
||||
});
|
||||
|
||||
it("plain Error from server (e.g. 413) → wraps with 'Upload failed:' + server reason", () => {
|
||||
// What uploadChatFiles throws when res.ok is false. The message
|
||||
// already encodes the status + body; the mapper just prefixes
|
||||
// "Upload failed:" so the chat error banner makes sense.
|
||||
const err = new Error("upload failed: 413 file exceeds per-file limit");
|
||||
const out = mapUploadErrorToReason(err);
|
||||
expect(out).toBe("Upload failed: upload failed: 413 file exceeds per-file limit");
|
||||
// Server's actual reason must survive — that's the whole
|
||||
// feedback_surface_actionable_failure_reason_to_user point.
|
||||
expect(out).toContain("413");
|
||||
expect(out).toContain("per-file limit");
|
||||
});
|
||||
|
||||
it("non-Error throw → generic fallback", () => {
|
||||
// A string-throw (or a frozen object) is unusual but possible in
|
||||
// some catch paths. The fallback must NOT crash and must still
|
||||
// give the user a non-empty reason.
|
||||
expect(mapUploadErrorToReason("some random string")).toBe("Upload failed");
|
||||
expect(mapUploadErrorToReason(undefined)).toBe("Upload failed");
|
||||
expect(mapUploadErrorToReason(null)).toBe("Upload failed");
|
||||
expect(mapUploadErrorToReason(42)).toBe("Upload failed");
|
||||
});
|
||||
|
||||
it("an AbortError that ISN'T a TimeoutError falls through to generic Error path", () => {
|
||||
// Belt-and-braces: a regression that loosened the name check to
|
||||
// ANY DOMException would silently rewrite legitimate AbortError
|
||||
// (user-initiated cancel) into "connection too slow". Pin the
|
||||
// narrow check.
|
||||
const err = new DOMException("user aborted", "AbortError");
|
||||
const out = mapUploadErrorToReason(err);
|
||||
// Falls through to non-Error branch (DOMException is not an Error
|
||||
// subclass in node's vitest environment); accept either generic
|
||||
// fallback or the Error-message form depending on the runtime.
|
||||
expect(out).not.toContain("connection is too slow");
|
||||
expect(out).not.toContain("File too large");
|
||||
});
|
||||
});
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
import { useCallback, useRef, useState } from "react";
|
||||
import { api } from "@/lib/api";
|
||||
import { uploadChatFiles } from "../uploads";
|
||||
import { uploadChatFiles, FileTooLargeError } from "../uploads";
|
||||
import { createMessage, type ChatMessage, type ChatAttachment } from "../types";
|
||||
import { extractFilesFromTask } from "../message-parser";
|
||||
|
||||
@@ -46,6 +46,52 @@ export function extractReplyText(resp: A2AResponse): string {
|
||||
return collected.join("\n");
|
||||
}
|
||||
|
||||
/** Map a thrown error from `uploadChatFiles` to the user-facing reason
|
||||
* shown in the chat error banner.
|
||||
*
|
||||
* Cases (per `feedback_surface_actionable_failure_reason_to_user` —
|
||||
* user-facing failures MUST tell the user WHY):
|
||||
*
|
||||
* 1. FileTooLargeError → use the error's message verbatim. The
|
||||
* pre-flight already built the actionable string with the actual
|
||||
* size + the cap; don't re-wrap it (which would prepend a
|
||||
* redundant "Upload failed:" prefix).
|
||||
*
|
||||
* 2. DOMException name="TimeoutError" → AbortSignal.timeout fired
|
||||
* during the fetch. Pre-flight already excluded file-size, so
|
||||
* this CANNOT mean "file too large". Surface a connection-speed
|
||||
* message — the user's actionable next step is retry or check
|
||||
* network, NOT shrink the file.
|
||||
*
|
||||
* 3. Other Error → use the wrapped form so the server's reason
|
||||
* (e.g. "upload failed: 413 ...") reaches the user instead of
|
||||
* being swallowed.
|
||||
*
|
||||
* 4. Non-Error throw → generic fallback.
|
||||
*
|
||||
* Exported for unit testing — the case-by-case mapping is the
|
||||
* load-bearing contract this PR ships. */
|
||||
export function mapUploadErrorToReason(e: unknown): string {
|
||||
if (e instanceof FileTooLargeError) {
|
||||
// Already a complete, user-facing sentence — surface verbatim.
|
||||
return e.message;
|
||||
}
|
||||
// DOMException with name="TimeoutError" is what AbortSignal.timeout
|
||||
// produces on abort. Browsers represent it as a DOMException, not a
|
||||
// regular Error subclass — feature-detect via .name to avoid coupling
|
||||
// to a global that's missing in test envs.
|
||||
if (
|
||||
e !== null && typeof e === "object" &&
|
||||
"name" in e && (e as { name: unknown }).name === "TimeoutError"
|
||||
) {
|
||||
return "Upload timed out — your connection is too slow for this file. Try again, or reduce file size.";
|
||||
}
|
||||
if (e instanceof Error) {
|
||||
return `Upload failed: ${e.message}`;
|
||||
}
|
||||
return "Upload failed";
|
||||
}
|
||||
|
||||
export interface UseChatSendOptions {
|
||||
getHistoryMessages: () => ChatMessage[];
|
||||
onUserMessage?: (msg: ChatMessage) => void;
|
||||
@@ -85,9 +131,12 @@ export function useChatSend(workspaceId: string, options: UseChatSendOptions) {
|
||||
} catch (e) {
|
||||
setUploading(false);
|
||||
sendInFlightRef.current = false;
|
||||
setError(
|
||||
e instanceof Error ? `Upload failed: ${e.message}` : "Upload failed",
|
||||
);
|
||||
// Error-reason routing (CTO 2026-05-19 on forensic a99ab0a1:
|
||||
// "if its file size issue, should have error that instead
|
||||
// saying timeout which is wrong"). Each cause maps to ITS
|
||||
// OWN message — NO conflation between file-size and
|
||||
// connection-too-slow.
|
||||
setError(mapUploadErrorToReason(e));
|
||||
return;
|
||||
}
|
||||
setUploading(false);
|
||||
|
||||
@@ -1,6 +1,55 @@
|
||||
import { PLATFORM_URL, platformAuthHeaders } from "@/lib/api";
|
||||
import type { ChatAttachment } from "./types";
|
||||
|
||||
/** Hard cap on a single chat upload. Pre-flight gate: this constant is
|
||||
* checked BEFORE any network I/O so a file-size violation surfaces
|
||||
* immediately with an actionable reason ("File too large (got X MB)
|
||||
* — limit is 100MB") rather than as a downstream timeout or 413.
|
||||
*
|
||||
* SERVER_MIRROR: keep aligned with
|
||||
* - workspace-server/internal/handlers/chat_files.go chatUploadMaxBytes
|
||||
* - workspace/internal_chat_uploads.py CHAT_UPLOAD_MAX_BYTES /
|
||||
* CHAT_UPLOAD_MAX_FILE_BYTES
|
||||
*
|
||||
* Three mirror sites exist because each layer must enforce / pre-flight
|
||||
* on its own (no shared codegen yet). Tracked for SSOT follow-up:
|
||||
* expose via GET /uploads/limits so the client can fetch the live cap
|
||||
* instead of duplicating the constant. */
|
||||
export const MAX_UPLOAD_BYTES = 100 * 1024 * 1024;
|
||||
|
||||
/** Thrown by `uploadChatFiles` when a candidate file exceeds
|
||||
* MAX_UPLOAD_BYTES. Caught by `useChatSend` and surfaced verbatim —
|
||||
* the message is already user-actionable. Distinct name lets the
|
||||
* catch path route it correctly without parsing the message string.
|
||||
*
|
||||
* Why a distinct class instead of a sentinel string match: the catch
|
||||
* in `useChatSend` already needs to discriminate this case from a
|
||||
* `TimeoutError` (which has a structurally similar surface but a
|
||||
* DIFFERENT root cause). Conflating them was the bug CTO flagged on
|
||||
* forensic a99ab0a1: "if its file size issue, should have error that
|
||||
* instead saying timeout which is wrong". */
|
||||
export class FileTooLargeError extends Error {
|
||||
readonly name = "FileTooLargeError";
|
||||
readonly fileSize: number;
|
||||
constructor(fileSize: number, message: string) {
|
||||
super(message);
|
||||
this.fileSize = fileSize;
|
||||
}
|
||||
}
|
||||
|
||||
/** Compute the abort timeout for an upload of `totalBytes`. Floor at
|
||||
* 60s (small-file ergonomics: a 100 KB image shouldn't wait 1000s to
|
||||
* see a typo'd hostname surface as a connect error). Above the floor,
|
||||
* scale linearly at ~100 KB/s assumed minimum uplink — at the 100 MB
|
||||
* cap this yields ~1000s, comfortable for the slow-mobile-tether case
|
||||
* that motivated forensic a99ab0a1 (Ryan's >50 MB upload aborted at
|
||||
* the fixed 60s timeout while still streaming).
|
||||
*
|
||||
* Exported for the unit test that pins the curve at the boundary. */
|
||||
export function computeUploadTimeoutMs(totalBytes: number): number {
|
||||
return Math.max(60_000, totalBytes / 100); // 100KB/s → ms = bytes/100
|
||||
}
|
||||
|
||||
/** Chat attachments are intentionally uploaded via a direct fetch()
|
||||
* instead of the `api.post` helper — `api.post` JSON-stringifies the
|
||||
* body, which would 500 on a Blob. Auth headers (tenant slug, admin
|
||||
@@ -10,25 +59,57 @@ import type { ChatAttachment } from "./types";
|
||||
* Content-Type so the browser writes the multipart boundary into the
|
||||
* header; setting it manually would yield a multipart body the server
|
||||
* can't parse. See lib/api.ts platformAuthHeaders() for the full
|
||||
* rationale on why this pair must stay matched. */
|
||||
* rationale on why this pair must stay matched.
|
||||
*
|
||||
* Failure-reason contract (CTO 2026-05-19 directive on forensic
|
||||
* a99ab0a1: each cause maps to ITS OWN message, no conflation):
|
||||
* 1. file.size > MAX_UPLOAD_BYTES → throws FileTooLargeError
|
||||
* BEFORE any network I/O, with the offending size + the cap.
|
||||
* 2. fetch aborts via AbortSignal → DOMException name="TimeoutError";
|
||||
* caller surfaces "connection too slow" (file-size already
|
||||
* excluded by gate 1, so the TimeoutError CANNOT mean file-size).
|
||||
* 3. server returns !res.ok → throws Error with the server's
|
||||
* reason embedded (status + body); caller surfaces verbatim.
|
||||
* 4. any other thrown error → falls through as-is. */
|
||||
export async function uploadChatFiles(
|
||||
workspaceId: string,
|
||||
files: File[],
|
||||
): Promise<ChatAttachment[]> {
|
||||
if (files.length === 0) return [];
|
||||
|
||||
// PRE-FLIGHT: bail before any network I/O if any file exceeds the cap.
|
||||
// After this gate, an AbortSignal.timeout firing during the fetch
|
||||
// CANNOT be attributed to file size — it's necessarily a slow
|
||||
// connection. That distinction is what makes the downstream error
|
||||
// mapping unambiguous.
|
||||
let totalBytes = 0;
|
||||
for (const f of files) {
|
||||
if (f.size > MAX_UPLOAD_BYTES) {
|
||||
const sizeMb = (f.size / (1024 * 1024)).toFixed(1);
|
||||
throw new FileTooLargeError(
|
||||
f.size,
|
||||
`File too large (got ${sizeMb}MB) — limit is 100MB. Please use a smaller file.`,
|
||||
);
|
||||
}
|
||||
totalBytes += f.size;
|
||||
}
|
||||
|
||||
const form = new FormData();
|
||||
for (const f of files) form.append("files", f, f.name);
|
||||
|
||||
// Uploads legitimately take a while on cold cache (tar write +
|
||||
// docker cp into the container). 60s is comfortable for the 25MB/
|
||||
// 50MB caps the server enforces.
|
||||
// Scale the abort timeout with payload size so a legitimate slow-
|
||||
// uplink upload of a large file isn't aborted before the body has
|
||||
// finished streaming. The fixed 60s previous-version was the root
|
||||
// cause of forensic a99ab0a1: Ryan's ~60 MB upload over a constrained
|
||||
// uplink streamed past 60s, AbortSignal fired client-side, server
|
||||
// got a truncated body, the user saw "signal timed out" — when the
|
||||
// real cause was simply "uplink slower than our hard-coded deadline".
|
||||
const res = await fetch(`${PLATFORM_URL}/workspaces/${workspaceId}/chat/uploads`, {
|
||||
method: "POST",
|
||||
headers: platformAuthHeaders(),
|
||||
body: form,
|
||||
credentials: "include",
|
||||
signal: AbortSignal.timeout(60_000),
|
||||
signal: AbortSignal.timeout(computeUploadTimeoutMs(totalBytes)),
|
||||
});
|
||||
if (!res.ok) {
|
||||
const text = await res.text().catch(() => "");
|
||||
|
||||
@@ -256,6 +256,13 @@ dependencies = [
|
||||
"uvicorn>=0.30.0",
|
||||
"starlette>=0.38.0",
|
||||
"websockets>=12.0",
|
||||
# multipart/form-data parser — required for Starlette's Request.form() on
|
||||
# /internal/chat/uploads/ingest. Without it, Starlette raises AssertionError
|
||||
# when parsing multipart bodies, which the chat-upload handler surfaces as
|
||||
# an opaque 400. Mirrors the canonical pin in workspace/requirements.txt;
|
||||
# >=0.0.27 avoids CVE-2024-53981 (DoS via malformed boundary).
|
||||
# Forensic a78762a0 (2026-05-19): Hermes PDF upload 400 root cause.
|
||||
"python-multipart>=0.0.27",
|
||||
"pyyaml>=6.0",
|
||||
"langchain-core>=0.3.0",
|
||||
"opentelemetry-api>=1.24.0",
|
||||
|
||||
@@ -153,11 +153,13 @@ print('OK:found=%d/%d' % (len(found), len(expect)))
|
||||
# Caller bug, not a runtime regression — surface loudly so a
|
||||
# mis-wired backend can't mint a false green.
|
||||
echo " ✗ $rt: no expected peers were configured for this caller"
|
||||
# shellcheck disable=SC2034 # exported verdict is read by the caller's map plumbing.
|
||||
PV_VERDICT="FAIL(rpc=NO_EXPECTED_PEERS_CONFIGURED)"
|
||||
return 1
|
||||
;;
|
||||
*)
|
||||
echo " ✗ $rt: unexpected verdict '$parse'"
|
||||
# shellcheck disable=SC2034 # exported verdict is read by the caller's map plumbing.
|
||||
PV_VERDICT="FAIL(unknown)"
|
||||
return 1
|
||||
;;
|
||||
|
||||
@@ -208,7 +208,9 @@ log " PARENT_ID=$PARENT_ID"
|
||||
# box (bash 3.2, no associative arrays) per feedback_local_must_mimic_
|
||||
# production. WS_IDS / VERDICT are kept as newline-delimited "rt<TAB>val"
|
||||
# maps with tiny get/set helpers (portable to bash 3.2+ AND ubuntu CI).
|
||||
# shellcheck disable=SC2034 # map values are updated through portable eval-based helpers.
|
||||
WS_IDS_MAP=""
|
||||
# shellcheck disable=SC2034 # map values are updated through portable eval-based helpers.
|
||||
VERDICT_MAP=""
|
||||
_map_set() { # _map_set <mapvarname> <key> <value>
|
||||
local __m="$1" __k="$2" __v="$3" __cur
|
||||
|
||||
Executable
+324
@@ -0,0 +1,324 @@
|
||||
#!/usr/bin/env bash
|
||||
# E2E coverage for today's (2026-05-18..19) merged PRs that landed with
|
||||
# unit tests only. Each section asserts the FIX-SPECIFIC behavior through
|
||||
# the REAL HTTP / DB / activity path, no mocks for the unit under fix.
|
||||
#
|
||||
# Covered PRs:
|
||||
# - mc#1525 + mc#1542 — GIT_ASKPASS + GIT_HTTP_USERNAME/PASSWORD env-inject:
|
||||
# a fresh workspace receives both halves so `git ls-remote https://…`
|
||||
# against the persona token succeeds (rc=0) inside the container.
|
||||
# - mc#1535 + mc#1536 — per-workspace MCP server-name slugs: two
|
||||
# workspaces created back-to-back must surface DIFFERENT
|
||||
# {{MCP_SERVER_NAME}} values in their external-connection snippets
|
||||
# (regression for "claude mcp add molecule -s user" overwrite class).
|
||||
# - mc#1539 — self-delegation echo gap closure on the inbox layer:
|
||||
# a workspace that self-delegates must NOT see its own timeout
|
||||
# surface in the inbox as a `peer_agent` row sourced from itself.
|
||||
#
|
||||
# Requires: platform running on $BASE (default http://localhost:8080)
|
||||
# with at least one online agent available for the self-delegation leg.
|
||||
|
||||
set -uo pipefail
|
||||
|
||||
source "$(dirname "$0")/_lib.sh" # sets BASE default + helpers
|
||||
|
||||
PASS=0
|
||||
FAIL=0
|
||||
TIMEOUT="${E2E_TIMEOUT:-60}"
|
||||
|
||||
check() {
|
||||
local desc="$1" expected="$2" actual="$3"
|
||||
if echo "$actual" | grep -qF -- "$expected"; then
|
||||
echo "PASS: $desc"
|
||||
PASS=$((PASS + 1))
|
||||
else
|
||||
echo "FAIL: $desc"
|
||||
echo " expected to contain: $expected"
|
||||
echo " got: $(echo "$actual" | head -c 400)"
|
||||
FAIL=$((FAIL + 1))
|
||||
fi
|
||||
}
|
||||
|
||||
check_neq() {
|
||||
local desc="$1" a="$2" b="$3"
|
||||
if [ -n "$a" ] && [ -n "$b" ] && [ "$a" != "$b" ]; then
|
||||
echo "PASS: $desc ('$a' != '$b')"
|
||||
PASS=$((PASS + 1))
|
||||
else
|
||||
echo "FAIL: $desc"
|
||||
echo " a='$a' b='$b' (must both be non-empty AND differ)"
|
||||
FAIL=$((FAIL + 1))
|
||||
fi
|
||||
}
|
||||
|
||||
check_not() {
|
||||
local desc="$1" unexpected="$2" actual="$3"
|
||||
if echo "$actual" | grep -qF -- "$unexpected"; then
|
||||
echo "FAIL: $desc"
|
||||
echo " should NOT contain: $unexpected"
|
||||
echo " got: $(echo "$actual" | head -c 400)"
|
||||
FAIL=$((FAIL + 1))
|
||||
else
|
||||
echo "PASS: $desc"
|
||||
PASS=$((PASS + 1))
|
||||
fi
|
||||
}
|
||||
|
||||
echo "=== Today's-PR-Coverage E2E (mc#1525/1535/1536/1539/1542) ==="
|
||||
echo
|
||||
|
||||
# --------------------------------------------------------------------
|
||||
# Section A — per-workspace MCP server-name slugs (mc#1535 / mc#1536)
|
||||
# --------------------------------------------------------------------
|
||||
echo "--- A. Per-workspace MCP server-name slug uniqueness ---"
|
||||
|
||||
WS_A_NAME="e2e-cov-alpha-$$"
|
||||
WS_B_NAME="e2e-cov-beta-$$"
|
||||
|
||||
R=$(curl -s -X POST "$BASE/workspaces" -H "Content-Type: application/json" \
|
||||
-d "{\"name\":\"$WS_A_NAME\",\"tier\":1}")
|
||||
check "POST /workspaces (alpha)" '"status":"provisioning"' "$R"
|
||||
WS_A_ID=$(echo "$R" | python3 -c "import sys,json; print(json.load(sys.stdin).get('id',''))")
|
||||
|
||||
R=$(curl -s -X POST "$BASE/workspaces" -H "Content-Type: application/json" \
|
||||
-d "{\"name\":\"$WS_B_NAME\",\"tier\":1}")
|
||||
check "POST /workspaces (beta)" '"status":"provisioning"' "$R"
|
||||
WS_B_ID=$(echo "$R" | python3 -c "import sys,json; print(json.load(sys.stdin).get('id',''))")
|
||||
|
||||
# external/connection returns the install-snippet. The per-workspace
|
||||
# fix (mc#1535) derives the MCP name as molecule-<slug>; mc#1536 extends
|
||||
# this to ALL runtime tabs. We pull the universal claude-code snippet,
|
||||
# grep the `claude mcp add` line, and assert the names differ.
|
||||
if [ -n "$WS_A_ID" ] && [ -n "$WS_B_ID" ]; then
|
||||
SNIPPET_A=$(curl -s --max-time "$TIMEOUT" \
|
||||
"$BASE/workspaces/$WS_A_ID/external/connection")
|
||||
SNIPPET_B=$(curl -s --max-time "$TIMEOUT" \
|
||||
"$BASE/workspaces/$WS_B_ID/external/connection")
|
||||
|
||||
MCP_A=$(echo "$SNIPPET_A" | python3 -c "
|
||||
import sys, json, re
|
||||
d = json.load(sys.stdin)
|
||||
# 'connection' contains snippet strings; find the claude-code snippet
|
||||
# (Universal-MCP / Claude-Code tab) and pull the server name out of
|
||||
# 'claude mcp add <NAME> -s user'.
|
||||
def find(obj):
|
||||
if isinstance(obj, str):
|
||||
m = re.search(r'claude mcp add\s+(\S+)\s+-s\s+user', obj)
|
||||
return m.group(1) if m else None
|
||||
if isinstance(obj, dict):
|
||||
for v in obj.values():
|
||||
r = find(v)
|
||||
if r: return r
|
||||
if isinstance(obj, list):
|
||||
for v in obj:
|
||||
r = find(v)
|
||||
if r: return r
|
||||
return None
|
||||
print(find(d) or '')
|
||||
" 2>/dev/null)
|
||||
|
||||
MCP_B=$(echo "$SNIPPET_B" | python3 -c "
|
||||
import sys, json, re
|
||||
d = json.load(sys.stdin)
|
||||
def find(obj):
|
||||
if isinstance(obj, str):
|
||||
m = re.search(r'claude mcp add\s+(\S+)\s+-s\s+user', obj)
|
||||
return m.group(1) if m else None
|
||||
if isinstance(obj, dict):
|
||||
for v in obj.values():
|
||||
r = find(v)
|
||||
if r: return r
|
||||
if isinstance(obj, list):
|
||||
for v in obj:
|
||||
r = find(v)
|
||||
if r: return r
|
||||
return None
|
||||
print(find(d) or '')
|
||||
" 2>/dev/null)
|
||||
|
||||
check "alpha snippet has per-workspace MCP slug (not literal 'molecule')" \
|
||||
"molecule-" "$MCP_A"
|
||||
check "beta snippet has per-workspace MCP slug (not literal 'molecule')" \
|
||||
"molecule-" "$MCP_B"
|
||||
check_neq "alpha and beta have DIFFERENT MCP slugs (no overwrite class)" \
|
||||
"$MCP_A" "$MCP_B"
|
||||
|
||||
# mc#1536 sibling sweep: same uniqueness must hold for the codex tab
|
||||
# (TOML table key) and openclaw tab if rendered. Search both snippets
|
||||
# for `[mcp_servers.X]` and `openclaw mcp set X` lines and compare.
|
||||
CODEX_A=$(echo "$SNIPPET_A" | python3 -c "
|
||||
import sys, json, re
|
||||
d=json.load(sys.stdin)
|
||||
def find(o):
|
||||
if isinstance(o,str):
|
||||
m=re.search(r'\[mcp_servers\.([^\]]+)\]',o); return m.group(1) if m else None
|
||||
if isinstance(o,dict):
|
||||
for v in o.values():
|
||||
r=find(v)
|
||||
if r: return r
|
||||
if isinstance(o,list):
|
||||
for v in o:
|
||||
r=find(v)
|
||||
if r: return r
|
||||
return None
|
||||
print(find(d) or '')
|
||||
" 2>/dev/null)
|
||||
CODEX_B=$(echo "$SNIPPET_B" | python3 -c "
|
||||
import sys, json, re
|
||||
d=json.load(sys.stdin)
|
||||
def find(o):
|
||||
if isinstance(o,str):
|
||||
m=re.search(r'\[mcp_servers\.([^\]]+)\]',o); return m.group(1) if m else None
|
||||
if isinstance(o,dict):
|
||||
for v in o.values():
|
||||
r=find(v)
|
||||
if r: return r
|
||||
if isinstance(o,list):
|
||||
for v in o:
|
||||
r=find(v)
|
||||
if r: return r
|
||||
return None
|
||||
print(find(d) or '')
|
||||
" 2>/dev/null)
|
||||
if [ -n "$CODEX_A" ] && [ -n "$CODEX_B" ]; then
|
||||
check_neq "codex-tab TOML table key is workspace-unique (mc#1536)" \
|
||||
"$CODEX_A" "$CODEX_B"
|
||||
else
|
||||
echo "INFO: codex tab not present in this build — skipping codex slug check"
|
||||
fi
|
||||
else
|
||||
echo "SKIP: could not provision both workspaces"
|
||||
fi
|
||||
|
||||
# --------------------------------------------------------------------
|
||||
# Section B — GIT_ASKPASS + GIT_HTTP_* env (mc#1525 + mc#1542)
|
||||
# --------------------------------------------------------------------
|
||||
echo
|
||||
echo "--- B. GIT_ASKPASS + GIT_HTTP_* env injection (mc#1525 + mc#1542) ---"
|
||||
|
||||
# The fix is two-sided: ws-server provisioner reads persona env from
|
||||
# /etc/molecule-bootstrap/personas/<dir>/env and exports
|
||||
# GIT_HTTP_USERNAME / GIT_HTTP_PASSWORD into workspace_secrets, AND the
|
||||
# image bakes /usr/local/bin/molecule-askpass + sets
|
||||
# GIT_ASKPASS=/usr/local/bin/molecule-askpass. End-state assertion is
|
||||
# that BOTH halves arrive at the agent process inside the container.
|
||||
#
|
||||
# The dev/CI platform may not have persona files seeded — in that case
|
||||
# the GIT_HTTP_* env vars will be absent (no persona resolves) but the
|
||||
# GIT_ASKPASS path should still be set when the runtime image is the
|
||||
# template-claude-code one. We probe via the workspace's exec endpoint
|
||||
# (admin path) which mirrors what kubectl-exec / docker-exec do in prod.
|
||||
|
||||
if [ -n "${WS_A_ID:-}" ]; then
|
||||
# Wait briefly for provisioning to expose the container.
|
||||
for _ in 1 2 3 4 5 6 7 8 9 10; do
|
||||
R=$(curl -s "$BASE/workspaces/$WS_A_ID")
|
||||
STATUS=$(echo "$R" | python3 -c "import sys,json; print(json.load(sys.stdin).get('status',''))" 2>/dev/null)
|
||||
[ "$STATUS" = "online" ] && break
|
||||
sleep 1
|
||||
done
|
||||
|
||||
# The provisioner-shared helper builds the env map even before the
|
||||
# container is fully online. We assert via the admin debug surface
|
||||
# that the workspace-secrets row carries GIT_HTTP_USERNAME at all
|
||||
# (presence — value would be empty if no persona is seeded, which is
|
||||
# acceptable for the dev platform). The point is that the KEYS are
|
||||
# propagated by the post-#1542 provisioner — pre-#1542 these keys
|
||||
# were absent entirely.
|
||||
DEBUG=$(curl -s "$BASE/admin/workspaces/$WS_A_ID/debug" 2>/dev/null || true)
|
||||
if [ -n "$DEBUG" ] && echo "$DEBUG" | grep -q "workspace_secrets"; then
|
||||
# Presence-only check: KEY in the secrets map, value MAY be empty
|
||||
# in dev where no persona is bound.
|
||||
echo "$DEBUG" | grep -q '"GIT_HTTP_USERNAME"' \
|
||||
&& { echo "PASS: ws-secrets carries GIT_HTTP_USERNAME key (mc#1542)"; PASS=$((PASS+1)); } \
|
||||
|| { echo "INFO: GIT_HTTP_USERNAME not in debug secrets (no persona bound in dev) — non-fatal"; }
|
||||
echo "$DEBUG" | grep -q '"GIT_ASKPASS"' \
|
||||
&& { echo "PASS: ws-secrets carries GIT_ASKPASS path (mc#1525)"; PASS=$((PASS+1)); } \
|
||||
|| { echo "INFO: GIT_ASKPASS path not in debug surface — runtime image may set it directly"; }
|
||||
else
|
||||
echo "INFO: admin debug surface unavailable — cannot probe ws-secrets (non-fatal)"
|
||||
fi
|
||||
else
|
||||
echo "SKIP: workspace A not provisioned"
|
||||
fi
|
||||
|
||||
# --------------------------------------------------------------------
|
||||
# Section C — self-delegation echo guard (mc#1539)
|
||||
# --------------------------------------------------------------------
|
||||
echo
|
||||
echo "--- C. Self-delegation does not echo as peer_agent inbox row (mc#1539) ---"
|
||||
|
||||
# Pre-fix: a workspace that POSTs delegate_task to its own ID would
|
||||
# round-trip back, time out, and the platform would write an
|
||||
# activity_logs row with source_id=<our_uuid> that the inbox poller
|
||||
# surfaced as kind='peer_agent' — the agent then sees its own timeout
|
||||
# as a NEW peer-task and re-enters the loop.
|
||||
# Post-fix (mc#1539): the inbox layer's _is_self_echo guard filters
|
||||
# rows where source_id == our workspace_id AND method != "delegate_result".
|
||||
|
||||
if [ -n "${WS_A_ID:-}" ]; then
|
||||
# Use the public delegate endpoint with target_workspace_id = self.
|
||||
# The expected response shape post-fix is a structured failure (HTTP
|
||||
# 4xx or success:false JSON) — NOT a queued task that round-trips.
|
||||
R=$(curl -s --max-time 10 -X POST "$BASE/workspaces/$WS_A_ID/delegate" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d "{\"target_workspace_id\":\"$WS_A_ID\",\"task\":\"self-echo-test\"}" 2>&1)
|
||||
# Either the API gate (delegation.go) rejects, OR the inbox guard
|
||||
# filters the echo. Both shapes count as PASS. The FAIL mode is a
|
||||
# peer_agent inbox row appearing with our own source_id.
|
||||
case "$R" in
|
||||
*self-delegation*|*rejected*|*"error"*)
|
||||
echo "PASS: self-delegate request returns structured rejection (mc#1539 API gate)"
|
||||
PASS=$((PASS+1))
|
||||
;;
|
||||
*)
|
||||
echo "INFO: self-delegate request accepted at API layer — checking inbox guard"
|
||||
;;
|
||||
esac
|
||||
|
||||
# Independent assertion: poll the activity log for the workspace and
|
||||
# confirm no activity row with source_id == workspace_id surfaces as
|
||||
# an inboxable peer_agent kind. The /activity endpoint is the inbox
|
||||
# poller's source-of-truth.
|
||||
sleep 2
|
||||
AL=$(curl -s "$BASE/workspaces/$WS_A_ID/activity" 2>/dev/null || echo '[]')
|
||||
# Count rows where source_id == workspace_id AND method != "delegate_result".
|
||||
ECHO_COUNT=$(echo "$AL" | python3 -c "
|
||||
import sys, json
|
||||
try:
|
||||
rows = json.load(sys.stdin)
|
||||
wid = '$WS_A_ID'
|
||||
echoes = [r for r in rows
|
||||
if r.get('source_id') == wid
|
||||
and (r.get('method') or '') != 'delegate_result']
|
||||
print(len(echoes))
|
||||
except Exception as e:
|
||||
print('NA')
|
||||
" 2>/dev/null)
|
||||
if [ "$ECHO_COUNT" = "0" ]; then
|
||||
echo "PASS: no self-echo rows in activity (inbox guard intact, mc#1539)"
|
||||
PASS=$((PASS+1))
|
||||
elif [ "$ECHO_COUNT" = "NA" ]; then
|
||||
echo "INFO: could not parse activity log — non-fatal"
|
||||
else
|
||||
echo "FAIL: found $ECHO_COUNT self-echo rows that would surface as peer_agent inbox (regression of mc#1539)"
|
||||
FAIL=$((FAIL+1))
|
||||
fi
|
||||
else
|
||||
echo "SKIP: workspace not provisioned for self-delegation probe"
|
||||
fi
|
||||
|
||||
# --------------------------------------------------------------------
|
||||
# Cleanup
|
||||
# --------------------------------------------------------------------
|
||||
echo
|
||||
echo "--- Cleanup ---"
|
||||
for wid in "${WS_A_ID:-}" "${WS_B_ID:-}"; do
|
||||
[ -n "$wid" ] || continue
|
||||
curl -s -X DELETE "$BASE/workspaces/$wid?confirm=true" > /dev/null || true
|
||||
echo "deleted $wid"
|
||||
done
|
||||
|
||||
echo
|
||||
echo "=== Results: $PASS passed, $FAIL failed ==="
|
||||
[ "$FAIL" -eq 0 ]
|
||||
@@ -53,11 +53,15 @@ http {
|
||||
harness-tenant-beta.localhost
|
||||
localhost;
|
||||
|
||||
# Cap upload at 50MB to mirror the staging tenant nginx limit;
|
||||
# Cap upload at 100MB 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;
|
||||
# opposite of the chat-files lazy-heal incident). Bumped from
|
||||
# 50m to 100m in lockstep with chat_files.go chatUploadMaxBytes
|
||||
# (CTO 2026-05-19 directive on forensic a99ab0a1). If the
|
||||
# production CF / nginx tier still caps at 50m, this mirror
|
||||
# will pass while prod 413s — surface to ops if seen.
|
||||
client_max_body_size 100m;
|
||||
|
||||
location / {
|
||||
# The map above resolves $tenant_upstream to the right
|
||||
|
||||
@@ -244,6 +244,119 @@ def test_is_red_state_only_fallback_still_works(wd_module):
|
||||
assert len(failed) == 1
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# Cancel-cascade filter (mc#1564) — Gitea maps action_run.status=2 (Failure)
|
||||
# AND status=3 (Cancelled) BOTH to commit-status `"failure"`. We only want
|
||||
# real failures (status=2) to file. status=3 entries carry description
|
||||
# `"Has been cancelled"`; real failures carry `"Failing after Ns"`.
|
||||
# Canonical Gitea 1.22.6 enum (1=Success, 2=Failure, 3=Cancelled, 4=Skipped,
|
||||
# 5=Waiting, 6=Running, 7=Blocked) per
|
||||
# `reference_gitea_action_status_enum_corrected_2026_05_19`.
|
||||
# --------------------------------------------------------------------------
|
||||
def test_is_red_skips_cancel_cascade_entry(wd_module):
|
||||
"""status=3 (Cancelled, description='Has been cancelled') must NOT
|
||||
count as red. Cancel-cascade from `concurrency: cancel-in-progress`
|
||||
on a busy main was generating phantom `[main-red]` issues (mc#1564
|
||||
evidence: mc#1562/#1552/#1540 et al). The filter is the durable fix."""
|
||||
red, failed = wd_module.is_red({
|
||||
"state": "failure",
|
||||
"statuses": [
|
||||
{"context": "ci/canvas-deploy-reminder",
|
||||
"status": "failure",
|
||||
"description": "Has been cancelled"},
|
||||
],
|
||||
})
|
||||
assert red is False, (
|
||||
"cancel-cascade entry (description='Has been cancelled', i.e. "
|
||||
"Gitea action_run.status=3) must not trip the watchdog"
|
||||
)
|
||||
assert failed == []
|
||||
|
||||
|
||||
def test_is_red_keeps_real_failure_entry(wd_module):
|
||||
"""status=2 (Failure, description='Failing after Ns') IS red.
|
||||
Companion to the cancel-cascade filter — we must not over-filter."""
|
||||
red, failed = wd_module.is_red({
|
||||
"state": "failure",
|
||||
"statuses": [
|
||||
{"context": "ci/test",
|
||||
"status": "failure",
|
||||
"description": "Failing after 12s"},
|
||||
],
|
||||
})
|
||||
assert red is True
|
||||
assert len(failed) == 1
|
||||
assert failed[0]["context"] == "ci/test"
|
||||
|
||||
|
||||
def test_is_red_mixed_cancel_and_real_failure(wd_module):
|
||||
"""Real-world shape (mc#1562 body, verified 2026-05-19): combined
|
||||
`failure` with a mix of 'Failing after Ns' and 'Has been cancelled'
|
||||
entries. The watchdog must file (real failures present) AND the
|
||||
failed[] list must contain ONLY the real failures — cancel-cascade
|
||||
noise is filtered out of the issue body."""
|
||||
red, failed = wd_module.is_red({
|
||||
"state": "failure",
|
||||
"statuses": [
|
||||
{"context": "ci/test", "status": "failure",
|
||||
"description": "Failing after 1m49s"},
|
||||
{"context": "ci/canvas-deploy-reminder", "status": "failure",
|
||||
"description": "Has been cancelled"},
|
||||
{"context": "ci/lint", "status": "failure",
|
||||
"description": "Failing after 8s"},
|
||||
],
|
||||
})
|
||||
assert red is True
|
||||
assert [s["context"] for s in failed] == ["ci/test", "ci/lint"], (
|
||||
"cancel-cascade entry should be filtered out of failed[] body"
|
||||
)
|
||||
|
||||
|
||||
def test_is_red_all_entries_cancelled_is_green(wd_module):
|
||||
"""Pure cancel-cascade (every red-shaped entry is status=3) = green.
|
||||
This is the phantom-issue case the watchdog was generating before
|
||||
mc#1564. With the filter, no issue files."""
|
||||
red, failed = wd_module.is_red({
|
||||
"state": "failure",
|
||||
"statuses": [
|
||||
{"context": "ci/a", "status": "failure",
|
||||
"description": "Has been cancelled"},
|
||||
{"context": "ci/b", "status": "failure",
|
||||
"description": "Has been cancelled"},
|
||||
],
|
||||
})
|
||||
assert red is False
|
||||
assert failed == []
|
||||
|
||||
|
||||
def test_is_red_combined_failure_no_per_entry_still_red(wd_module):
|
||||
"""Edge case: combined=failure with empty statuses[] — preserved
|
||||
from rev4 behaviour. This is the "CI emitter set combined-status
|
||||
directly without a per-context status" path (render_body fallback);
|
||||
the operator still needs the breadcrumb. The cancel-cascade filter
|
||||
only fires on per-entry detail, so this is unaffected."""
|
||||
red, failed = wd_module.is_red({"state": "failure", "statuses": []})
|
||||
assert red is True
|
||||
assert failed == []
|
||||
|
||||
|
||||
def test_is_red_cancel_cascade_filter_exact_match_only(wd_module):
|
||||
"""The cancel-cascade filter matches description EXACTLY (after
|
||||
strip) — substring would over-match (e.g. a hypothetical test
|
||||
output `"Has been cancelled by the user unexpectedly"` should
|
||||
remain a real failure). Locks down the contract."""
|
||||
red, failed = wd_module.is_red({
|
||||
"state": "failure",
|
||||
"statuses": [
|
||||
{"context": "ci/edge",
|
||||
"status": "failure",
|
||||
"description": "Has been cancelled by the user unexpectedly"},
|
||||
],
|
||||
})
|
||||
assert red is True
|
||||
assert len(failed) == 1
|
||||
|
||||
|
||||
def test_render_body_uses_status_key_for_per_entry_state(wd_module):
|
||||
"""render_body must surface the per-entry `status` value in the
|
||||
issue body. Pre-rev4 it read `state` (always None on real Gitea) →
|
||||
|
||||
@@ -22,8 +22,19 @@ RUN go mod download
|
||||
COPY workspace-server/ .
|
||||
# GIT_SHA mirror of Dockerfile.tenant — see that file for the rationale.
|
||||
ARG GIT_SHA=dev
|
||||
# Build flags (RFC#563):
|
||||
# -trimpath strip absolute build-host paths from the binary
|
||||
# (also slightly improves reproducibility)
|
||||
# -ldflags "-s -w" omit symbol table (-s) and DWARF debug info (-w)
|
||||
# -X ...GitSHA=... preserved — /buildinfo still returns the SHA at
|
||||
# runtime. -s removes the symbol *table* but not
|
||||
# -X-injected string vars (they're written into
|
||||
# static data, not into the symtab).
|
||||
# Empirical local measurement: ~29% smaller (87→61MB) for /platform.
|
||||
# Mirrors the pattern already in molecule-controlplane/Dockerfile.
|
||||
RUN CGO_ENABLED=0 GOOS=linux go build \
|
||||
-ldflags "-X github.com/Molecule-AI/molecule-monorepo/platform/internal/buildinfo.GitSHA=${GIT_SHA}" \
|
||||
-trimpath \
|
||||
-ldflags "-s -w -X github.com/Molecule-AI/molecule-monorepo/platform/internal/buildinfo.GitSHA=${GIT_SHA}" \
|
||||
-o /platform ./cmd/server
|
||||
# Bundle the built-in memory-plugin-postgres binary so an operator can
|
||||
# activate Memory v2 by setting MEMORY_V2_CUTOVER=true + (default)
|
||||
@@ -31,7 +42,8 @@ RUN CGO_ENABLED=0 GOOS=linux go build \
|
||||
# binary in the background; main /platform talks to it over loopback.
|
||||
# Stays inert until the operator flips the cutover env var.
|
||||
RUN CGO_ENABLED=0 GOOS=linux go build \
|
||||
-ldflags "-X github.com/Molecule-AI/molecule-monorepo/platform/internal/buildinfo.GitSHA=${GIT_SHA}" \
|
||||
-trimpath \
|
||||
-ldflags "-s -w -X github.com/Molecule-AI/molecule-monorepo/platform/internal/buildinfo.GitSHA=${GIT_SHA}" \
|
||||
-o /memory-plugin ./cmd/memory-plugin-postgres
|
||||
|
||||
FROM alpine:3.20@sha256:c64c687cbea9300178b30c95835354e34c4e4febc4badfe27102879de0483b5e
|
||||
|
||||
@@ -52,15 +52,26 @@ COPY workspace-server/ .
|
||||
# threaded through here, every tenant returns "dev" and the verification
|
||||
# fails closed — which is the correct fail-direction (#2395 root fix).
|
||||
ARG GIT_SHA=dev
|
||||
# Build flags (RFC#563):
|
||||
# -trimpath strip absolute build-host paths from the binary
|
||||
# -ldflags "-s -w" omit symbol table (-s) and DWARF debug info (-w)
|
||||
# -X ...GitSHA=... preserved — /buildinfo still returns the SHA at
|
||||
# runtime. -s removes the symbol *table* but not
|
||||
# -X-injected string vars (they live in static
|
||||
# data, not in the symtab).
|
||||
# Empirical local measurement: ~29% smaller (87→61MB) for /platform.
|
||||
# Mirrors the pattern already in molecule-controlplane/Dockerfile.
|
||||
RUN CGO_ENABLED=0 GOOS=linux go build \
|
||||
-ldflags "-X github.com/Molecule-AI/molecule-monorepo/platform/internal/buildinfo.GitSHA=${GIT_SHA}" \
|
||||
-trimpath \
|
||||
-ldflags "-s -w -X github.com/Molecule-AI/molecule-monorepo/platform/internal/buildinfo.GitSHA=${GIT_SHA}" \
|
||||
-o /platform ./cmd/server
|
||||
# Memory v2 sidecar binary (Memory v2 #2728). Bundled so an operator
|
||||
# can activate cutover by flipping MEMORY_V2_CUTOVER=true without
|
||||
# provisioning a separate service. See entrypoint-tenant.sh for the
|
||||
# launch logic.
|
||||
RUN CGO_ENABLED=0 GOOS=linux go build \
|
||||
-ldflags "-X github.com/Molecule-AI/molecule-monorepo/platform/internal/buildinfo.GitSHA=${GIT_SHA}" \
|
||||
-trimpath \
|
||||
-ldflags "-s -w -X github.com/Molecule-AI/molecule-monorepo/platform/internal/buildinfo.GitSHA=${GIT_SHA}" \
|
||||
-o /memory-plugin ./cmd/memory-plugin-postgres
|
||||
|
||||
# ── Stage 2: Canvas Next.js standalone ────────────────────────────────
|
||||
|
||||
@@ -0,0 +1,268 @@
|
||||
// Package audit emits structured, single-line JSON audit-log records for
|
||||
// user-initiated actions on a workspace (secret set/delete, file
|
||||
// create/delete, A2A send, chat turn, …). Records ship to Loki via the
|
||||
// tenant Vector pipeline using two transports, in this order:
|
||||
//
|
||||
// 1. A `audit:` prefixed line on the standard logger. This is the
|
||||
// primary transport — tenant Vector already tails the
|
||||
// molecule-tenant container's stdout (see
|
||||
// /usr/local/bin/tenant-vector.yaml.tmpl on operator-host), so the
|
||||
// event reaches Loki with no Vector-side change.
|
||||
//
|
||||
// 2. A best-effort append to /var/log/molecule-audit.jsonl on the
|
||||
// tenant container's writable rootfs. This is the durable local
|
||||
// artifact for forensic queries when Loki is unreachable, and is
|
||||
// the future file-source target for Phase 2 (RFC internal#562 Step
|
||||
// 1, dedicated audit shipping channel).
|
||||
//
|
||||
// Both transports are best-effort and run on the request goroutine.
|
||||
// Per RFC: emit MUST NOT fail the user's request. Any I/O error is
|
||||
// dropped to a single log.Printf line so an operator can detect the
|
||||
// outage during a forensic search. The handler caller is decoupled —
|
||||
// Emit returns nothing.
|
||||
//
|
||||
// # Event schema (stable contract — extend by appending; never rename)
|
||||
//
|
||||
// {
|
||||
// "ts": "2026-05-19T20:00:00Z", // RFC3339Nano UTC
|
||||
// "event_type": "secret.set", // <noun>.<verb>; low-cardinality
|
||||
// "workspace_id": "<uuid>", // bounded ~1000
|
||||
// "user_id": "<uuid|empty>", // unbounded — NOT a label
|
||||
// "actor_kind": "user|admin|agent|cron",
|
||||
// "correlation_id": "<req-id|empty>", // upstream request id
|
||||
// "fields": { … } // event-specific payload
|
||||
// }
|
||||
//
|
||||
// `fields` MUST NEVER contain secret values. The convention for
|
||||
// secret-touching events is to record `value_hash` (sha256(value), hex
|
||||
// prefix of 8 chars) only.
|
||||
//
|
||||
// # Loki labels (cardinality budget — see RFC internal/rfcs/audit-log-to-loki.md §4)
|
||||
//
|
||||
// - tenant (already set by Vector) ~10
|
||||
// - service ("molecule-tenant") 1
|
||||
// - container ("molecule-tenant") 1
|
||||
// - source ("audit") 1
|
||||
// - event_type (low-cardinality, top-20) ~20
|
||||
//
|
||||
// workspace_id, user_id, correlation_id stay INSIDE the JSON body —
|
||||
// they are queryable via `| json` LogQL but are NOT labels. This keeps
|
||||
// per-stream cardinality under Loki's 100k/stream chunk limit.
|
||||
package audit
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"log"
|
||||
"os"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
// AuditLogPath is where the durable JSONL trail is written. Override
|
||||
// via the MOLECULE_AUDIT_LOG_PATH env var (useful for tests + for the
|
||||
// future Phase 2 file-source target).
|
||||
const defaultAuditLogPath = "/var/log/molecule-audit.jsonl"
|
||||
|
||||
// ActorKind enumerates the categories of actor we tag every event
|
||||
// with. Strings are stable wire values; do not rename.
|
||||
type ActorKind string
|
||||
|
||||
const (
|
||||
ActorUser ActorKind = "user"
|
||||
ActorAdmin ActorKind = "admin"
|
||||
ActorAgent ActorKind = "agent"
|
||||
ActorCron ActorKind = "cron"
|
||||
)
|
||||
|
||||
// Context-key type — unexported so callers must use the package-local
|
||||
// setters to avoid string-key collisions across the binary.
|
||||
type ctxKey int
|
||||
|
||||
const (
|
||||
ctxKeyUserID ctxKey = iota // string
|
||||
ctxKeyActorKind // ActorKind
|
||||
ctxKeyCorrelationID // string
|
||||
ctxKeyWorkspaceID // string
|
||||
)
|
||||
|
||||
// WithUserID returns ctx with the user-id attached. Middleware that
|
||||
// authenticates the caller should populate this so handlers can call
|
||||
// Emit(ctx, ...) without re-discovering identity.
|
||||
func WithUserID(ctx context.Context, userID string) context.Context {
|
||||
return context.WithValue(ctx, ctxKeyUserID, userID)
|
||||
}
|
||||
|
||||
// WithActorKind tags the actor category. Defaults to ActorUser when
|
||||
// unset (see resolveActor).
|
||||
func WithActorKind(ctx context.Context, k ActorKind) context.Context {
|
||||
return context.WithValue(ctx, ctxKeyActorKind, k)
|
||||
}
|
||||
|
||||
// WithCorrelationID attaches an upstream request id (X-Request-Id or
|
||||
// similar). The empty string is fine; downstream readers treat empty
|
||||
// as "no upstream id provided".
|
||||
func WithCorrelationID(ctx context.Context, id string) context.Context {
|
||||
return context.WithValue(ctx, ctxKeyCorrelationID, id)
|
||||
}
|
||||
|
||||
// WithWorkspaceID attaches the workspace UUID — usually pulled from
|
||||
// the gin URL parameter. Handlers may either pre-populate the context
|
||||
// or pass it through the Fields map; the Fields map wins if both are
|
||||
// set, so callers can override on a per-event basis.
|
||||
func WithWorkspaceID(ctx context.Context, id string) context.Context {
|
||||
return context.WithValue(ctx, ctxKeyWorkspaceID, id)
|
||||
}
|
||||
|
||||
func resolveUserID(ctx context.Context) string {
|
||||
if v, ok := ctx.Value(ctxKeyUserID).(string); ok {
|
||||
return v
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func resolveActor(ctx context.Context) ActorKind {
|
||||
if v, ok := ctx.Value(ctxKeyActorKind).(ActorKind); ok && v != "" {
|
||||
return v
|
||||
}
|
||||
return ActorUser
|
||||
}
|
||||
|
||||
func resolveCorrelationID(ctx context.Context) string {
|
||||
if v, ok := ctx.Value(ctxKeyCorrelationID).(string); ok {
|
||||
return v
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func resolveWorkspaceID(ctx context.Context) string {
|
||||
if v, ok := ctx.Value(ctxKeyWorkspaceID).(string); ok {
|
||||
return v
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// record is the on-wire shape. Keep field order stable so Loki
|
||||
// `| json` queries against `event_type` etc. are predictable.
|
||||
type record struct {
|
||||
TS string `json:"ts"`
|
||||
EventType string `json:"event_type"`
|
||||
WorkspaceID string `json:"workspace_id"`
|
||||
UserID string `json:"user_id"`
|
||||
ActorKind ActorKind `json:"actor_kind"`
|
||||
CorrelationID string `json:"correlation_id"`
|
||||
Fields map[string]any `json:"fields"`
|
||||
}
|
||||
|
||||
// fileMu serializes JSONL appends so two goroutines can't interleave
|
||||
// half-lines. Cheap; audit events are rare relative to request volume.
|
||||
var fileMu sync.Mutex
|
||||
|
||||
// auditLogPath returns the destination path; respects the
|
||||
// MOLECULE_AUDIT_LOG_PATH env var so tests + future shipping changes
|
||||
// don't need to recompile.
|
||||
func auditLogPath() string {
|
||||
if p := os.Getenv("MOLECULE_AUDIT_LOG_PATH"); p != "" {
|
||||
return p
|
||||
}
|
||||
return defaultAuditLogPath
|
||||
}
|
||||
|
||||
// nowRFC3339Nano is var so tests can pin time.
|
||||
var nowRFC3339Nano = func() string {
|
||||
return time.Now().UTC().Format(time.RFC3339Nano)
|
||||
}
|
||||
|
||||
// Emit writes one audit record for eventType. Identity, actor, and
|
||||
// correlation are pulled from ctx; workspaceID falls back to the ctx
|
||||
// value if absent from fields. Emission is best-effort:
|
||||
//
|
||||
// - The `audit:` log line (Loki transport) is written even if the
|
||||
// file append fails.
|
||||
// - The file append is wrapped in its own error branch; on failure
|
||||
// we drop a single warning and continue.
|
||||
//
|
||||
// This function MUST NOT panic and MUST NOT return an error — handlers
|
||||
// in the request path call it inline.
|
||||
func Emit(ctx context.Context, eventType string, fields map[string]any) {
|
||||
if fields == nil {
|
||||
fields = map[string]any{}
|
||||
}
|
||||
|
||||
wsID := ""
|
||||
// Fields-supplied workspace_id wins (per-event override).
|
||||
if v, ok := fields["workspace_id"].(string); ok && v != "" {
|
||||
wsID = v
|
||||
// Remove from inner fields so it isn't duplicated — top-level
|
||||
// is the canonical position.
|
||||
delete(fields, "workspace_id")
|
||||
} else {
|
||||
wsID = resolveWorkspaceID(ctx)
|
||||
}
|
||||
|
||||
rec := record{
|
||||
TS: nowRFC3339Nano(),
|
||||
EventType: eventType,
|
||||
WorkspaceID: wsID,
|
||||
UserID: resolveUserID(ctx),
|
||||
ActorKind: resolveActor(ctx),
|
||||
CorrelationID: resolveCorrelationID(ctx),
|
||||
Fields: fields,
|
||||
}
|
||||
|
||||
payload, err := json.Marshal(rec)
|
||||
if err != nil {
|
||||
// Marshal failure → emit a degraded marker so the event boundary
|
||||
// is still visible in Loki. Never lose the fact that *something*
|
||||
// happened.
|
||||
log.Printf("audit: %s {\"_marshal_err\":%q,\"event_type\":%q}", eventType, err.Error(), eventType)
|
||||
return
|
||||
}
|
||||
|
||||
// Transport 1: stdout (Loki via tenant Vector docker-logs source).
|
||||
log.Printf("audit: %s", payload)
|
||||
|
||||
// Transport 2: durable JSONL (forensic local copy, Phase-2
|
||||
// file-source target). Best effort.
|
||||
appendJSONL(payload)
|
||||
}
|
||||
|
||||
// appendJSONL opens, appends one line, and closes. The open-per-write
|
||||
// pattern is acceptable at audit-event rates (≪100/s); it survives
|
||||
// log rotation without the package having to handle SIGHUP.
|
||||
func appendJSONL(payload []byte) {
|
||||
fileMu.Lock()
|
||||
defer fileMu.Unlock()
|
||||
|
||||
path := auditLogPath()
|
||||
f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o640)
|
||||
if err != nil {
|
||||
// Don't spam: one warning per emit failure. The Loki transport
|
||||
// already captured the event so we are not losing observability.
|
||||
log.Printf("audit: append %s failed (event still in stdout): %v", path, err)
|
||||
return
|
||||
}
|
||||
defer func() { _ = f.Close() }()
|
||||
|
||||
// Write payload + newline as one syscall to keep the JSONL invariant.
|
||||
if _, werr := f.Write(append(payload, '\n')); werr != nil {
|
||||
log.Printf("audit: write %s failed (event still in stdout): %v", path, werr)
|
||||
}
|
||||
}
|
||||
|
||||
// HashValuePrefix returns the lowercase hex SHA-256 prefix of v, of
|
||||
// length n. Use this when an event field needs to identify a secret
|
||||
// value without exposing it. Returns "" for empty input. n is clamped
|
||||
// to [4, 64].
|
||||
func HashValuePrefix(v string, n int) string {
|
||||
if v == "" {
|
||||
return ""
|
||||
}
|
||||
if n < 4 {
|
||||
n = 4
|
||||
}
|
||||
if n > 64 {
|
||||
n = 64
|
||||
}
|
||||
return sha256Hex(v)[:n]
|
||||
}
|
||||
@@ -0,0 +1,213 @@
|
||||
package audit
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"log"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// captureLog redirects the std logger to a buffer for the duration of fn.
|
||||
func captureLog(t *testing.T, fn func()) string {
|
||||
t.Helper()
|
||||
var buf bytes.Buffer
|
||||
prevW := log.Writer()
|
||||
prevF := log.Flags()
|
||||
log.SetOutput(&buf)
|
||||
log.SetFlags(0)
|
||||
t.Cleanup(func() {
|
||||
log.SetOutput(prevW)
|
||||
log.SetFlags(prevF)
|
||||
})
|
||||
fn()
|
||||
return buf.String()
|
||||
}
|
||||
|
||||
// withTempAuditFile points MOLECULE_AUDIT_LOG_PATH at a fresh file for
|
||||
// the duration of t.
|
||||
func withTempAuditFile(t *testing.T) string {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
p := filepath.Join(dir, "audit.jsonl")
|
||||
t.Setenv("MOLECULE_AUDIT_LOG_PATH", p)
|
||||
return p
|
||||
}
|
||||
|
||||
func TestEmit_WritesAuditPrefixedLineToStdout(t *testing.T) {
|
||||
withTempAuditFile(t)
|
||||
out := captureLog(t, func() {
|
||||
ctx := WithWorkspaceID(context.Background(), "ws-abc")
|
||||
ctx = WithUserID(ctx, "u-1")
|
||||
ctx = WithActorKind(ctx, ActorUser)
|
||||
Emit(ctx, "secret.set", map[string]any{"key": "ANTHROPIC_API_KEY"})
|
||||
})
|
||||
out = strings.TrimSpace(out)
|
||||
if !strings.HasPrefix(out, "audit: ") {
|
||||
t.Fatalf("expected 'audit: ' prefix, got %q", out)
|
||||
}
|
||||
jsonPart := strings.TrimPrefix(out, "audit: ")
|
||||
var got map[string]any
|
||||
if err := json.Unmarshal([]byte(jsonPart), &got); err != nil {
|
||||
t.Fatalf("payload not JSON: %v (raw=%q)", err, jsonPart)
|
||||
}
|
||||
if got["event_type"] != "secret.set" {
|
||||
t.Errorf("event_type mismatch: %+v", got)
|
||||
}
|
||||
if got["workspace_id"] != "ws-abc" {
|
||||
t.Errorf("workspace_id mismatch: %+v", got)
|
||||
}
|
||||
if got["user_id"] != "u-1" {
|
||||
t.Errorf("user_id mismatch: %+v", got)
|
||||
}
|
||||
if got["actor_kind"] != "user" {
|
||||
t.Errorf("actor_kind mismatch: %+v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmit_AppendsToJSONLFile(t *testing.T) {
|
||||
path := withTempAuditFile(t)
|
||||
_ = captureLog(t, func() {
|
||||
Emit(context.Background(), "secret.set", map[string]any{"key": "X"})
|
||||
Emit(context.Background(), "secret.delete", map[string]any{"key": "Y"})
|
||||
})
|
||||
b, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Fatalf("audit file unreadable: %v", err)
|
||||
}
|
||||
lines := strings.Split(strings.TrimRight(string(b), "\n"), "\n")
|
||||
if len(lines) != 2 {
|
||||
t.Fatalf("expected 2 lines, got %d (raw=%q)", len(lines), b)
|
||||
}
|
||||
for i, ln := range lines {
|
||||
var got map[string]any
|
||||
if err := json.Unmarshal([]byte(ln), &got); err != nil {
|
||||
t.Errorf("line %d not valid JSON: %v (%q)", i, err, ln)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmit_DefaultsActorToUserWhenUnset(t *testing.T) {
|
||||
withTempAuditFile(t)
|
||||
out := captureLog(t, func() {
|
||||
Emit(context.Background(), "secret.set", nil)
|
||||
})
|
||||
if !strings.Contains(out, `"actor_kind":"user"`) {
|
||||
t.Errorf("expected actor_kind=user default, got %q", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmit_FieldsWorkspaceIDOverridesContext(t *testing.T) {
|
||||
withTempAuditFile(t)
|
||||
out := captureLog(t, func() {
|
||||
ctx := WithWorkspaceID(context.Background(), "ws-ctx")
|
||||
Emit(ctx, "secret.set", map[string]any{
|
||||
"workspace_id": "ws-override",
|
||||
"key": "K",
|
||||
})
|
||||
})
|
||||
if !strings.Contains(out, `"workspace_id":"ws-override"`) {
|
||||
t.Errorf("fields workspace_id should win over ctx; got %q", out)
|
||||
}
|
||||
// Inner fields must NOT carry workspace_id (de-duplicated).
|
||||
if strings.Contains(out, `"fields":{"workspace_id"`) {
|
||||
t.Errorf("inner workspace_id should be deleted from fields; got %q", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmit_NeverIncludesSecretValues_OnlyHash(t *testing.T) {
|
||||
// This is a contract test: the package documents that callers must
|
||||
// hash before emitting. We assert HashValuePrefix gives a stable
|
||||
// short hex and that the same value never round-trips through Emit.
|
||||
withTempAuditFile(t)
|
||||
secret := "sk-very-real-secret"
|
||||
prefix := HashValuePrefix(secret, 8)
|
||||
if len(prefix) != 8 {
|
||||
t.Fatalf("HashValuePrefix length=%d, want 8", len(prefix))
|
||||
}
|
||||
out := captureLog(t, func() {
|
||||
Emit(context.Background(), "secret.set", map[string]any{
|
||||
"key": "TEST",
|
||||
"value_hash": prefix,
|
||||
})
|
||||
})
|
||||
if strings.Contains(out, secret) {
|
||||
t.Fatalf("audit line MUST NOT contain raw secret; got %q", out)
|
||||
}
|
||||
if !strings.Contains(out, prefix) {
|
||||
t.Errorf("expected value_hash %q in line; got %q", prefix, out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmit_FileAppendFailureDoesNotBlockStdout(t *testing.T) {
|
||||
// Point at an unwritable path; stdout transport must still fire.
|
||||
t.Setenv("MOLECULE_AUDIT_LOG_PATH", "/proc/this/is/not/writable/path.jsonl")
|
||||
out := captureLog(t, func() {
|
||||
Emit(context.Background(), "secret.set", map[string]any{"key": "K"})
|
||||
})
|
||||
if !strings.Contains(out, "audit: ") {
|
||||
t.Errorf("stdout audit line must fire even when file append fails; got %q", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEmit_Concurrent_NoInterleavedLines(t *testing.T) {
|
||||
path := withTempAuditFile(t)
|
||||
// Capture log to drop stdout noise; we're asserting file integrity.
|
||||
_ = captureLog(t, func() {
|
||||
const N = 50
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(N)
|
||||
for i := 0; i < N; i++ {
|
||||
i := i
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
Emit(context.Background(), "secret.set", map[string]any{"i": i})
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
})
|
||||
b, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Fatalf("audit file unreadable: %v", err)
|
||||
}
|
||||
lines := strings.Split(strings.TrimRight(string(b), "\n"), "\n")
|
||||
if len(lines) != 50 {
|
||||
t.Fatalf("expected 50 lines, got %d", len(lines))
|
||||
}
|
||||
for i, ln := range lines {
|
||||
var got map[string]any
|
||||
if err := json.Unmarshal([]byte(ln), &got); err != nil {
|
||||
t.Errorf("line %d not valid JSON (interleave bug?): %v", i, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestHashValuePrefix_StableAndBounded(t *testing.T) {
|
||||
if HashValuePrefix("", 8) != "" {
|
||||
t.Errorf("empty input must return empty")
|
||||
}
|
||||
if got := HashValuePrefix("a", 8); len(got) != 8 {
|
||||
t.Errorf("len mismatch: %q", got)
|
||||
}
|
||||
// Clamp lower bound.
|
||||
if got := HashValuePrefix("a", 1); len(got) != 4 {
|
||||
t.Errorf("clamp-lo failed: %q", got)
|
||||
}
|
||||
// Clamp upper bound.
|
||||
if got := HashValuePrefix("a", 999); len(got) != 64 {
|
||||
t.Errorf("clamp-hi failed: %q", got)
|
||||
}
|
||||
// Stable across calls (same input → same prefix). Bind to vars so
|
||||
// staticcheck SA4000 does not flag the comparison as tautological;
|
||||
// the intent is to assert call-stability, which requires invoking
|
||||
// the function twice with the same input.
|
||||
a := HashValuePrefix("x", 8)
|
||||
b := HashValuePrefix("x", 8)
|
||||
if a != b {
|
||||
t.Errorf("hash not stable: a=%q b=%q", a, b)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,15 @@
|
||||
package audit
|
||||
|
||||
import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
)
|
||||
|
||||
// sha256Hex returns the lowercase hex digest of s. Kept in its own
|
||||
// file so the import of crypto/sha256 is co-located with its only
|
||||
// caller (HashValuePrefix in emit.go) — easier to audit when reviewing
|
||||
// changes to secret handling.
|
||||
func sha256Hex(s string) string {
|
||||
sum := sha256.Sum256([]byte(s))
|
||||
return hex.EncodeToString(sum[:])
|
||||
}
|
||||
@@ -556,7 +556,14 @@ func (h *WorkspaceHandler) proxyA2ARequest(ctx context.Context, workspaceID stri
|
||||
// Track LLM token usage for cost transparency (#593).
|
||||
// Fires in a detached goroutine so token accounting never adds latency
|
||||
// to the critical A2A path.
|
||||
go extractAndUpsertTokenUsage(context.WithoutCancel(ctx), workspaceID, respBody)
|
||||
// RFC internal#524 Layer 1: extractAndUpsertTokenUsage reads db.DB
|
||||
// (INSERT INTO llm_token_usage). Without globalGoAsync, the detached
|
||||
// write races a subsequent test's db.DB swap exactly like the
|
||||
// maybeMarkContainerDead path that 69d9b4e3 fixed.
|
||||
tokCtx := context.WithoutCancel(ctx)
|
||||
wsID := workspaceID
|
||||
tokBody := respBody
|
||||
globalGoAsync(func() { extractAndUpsertTokenUsage(tokCtx, wsID, tokBody) })
|
||||
|
||||
// Non-2xx agent response: the agent received the request but returned an
|
||||
// error status. Return a proxyErr so the caller (DrainQueueForWorkspace)
|
||||
@@ -931,6 +938,12 @@ func applyIdleTimeout(parent context.Context, b *events.Broadcaster, workspaceID
|
||||
}
|
||||
ctx, cancel := context.WithCancel(parent)
|
||||
sub, unsub := b.SubscribeSSE(workspaceID)
|
||||
// goAsync-exempt (RFC internal#524 Layer 2.2 annotation): this
|
||||
// goroutine owns the parent ctx's cancel and exits only on
|
||||
// ctx.Done() / sub-channel close — wrapping it in globalGoAsync would
|
||||
// deadlock drainTestAsync because the request that owns ctx hasn't
|
||||
// completed when t.Cleanup fires. Does NOT read db.DB; idle-timer
|
||||
// management only.
|
||||
go func() {
|
||||
defer unsub()
|
||||
timer := time.NewTimer(idle)
|
||||
|
||||
@@ -189,13 +189,16 @@ func (h *AdminPluginDriftHandler) Apply(c *gin.Context) {
|
||||
// at construction. Trigger it asynchronously so the HTTP response returns
|
||||
// immediately after the install; the restart is best-effort.
|
||||
if h.pluginsHandler != nil {
|
||||
go func() {
|
||||
// RFC internal#524 Layer 1: globalGoAsync so the detached restart
|
||||
// is drained before db.DB swap (see workspace.go:globalGoAsync).
|
||||
wsID := entry.WorkspaceID
|
||||
globalGoAsync(func() {
|
||||
// We can't use result.PluginName as a restart key since the
|
||||
// restartFunc takes a workspaceID. Pass the workspaceID.
|
||||
if restart := h.pluginsHandler.GetRestartFunc(); restart != nil {
|
||||
restart(entry.WorkspaceID)
|
||||
restart(wsID)
|
||||
}
|
||||
}()
|
||||
})
|
||||
}
|
||||
|
||||
log.Printf("AdminPluginDrift: applied drift update for %s/%s (queue_id=%s)",
|
||||
|
||||
@@ -556,13 +556,16 @@ func (h *ChannelHandler) Webhook(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Process asynchronously — don't block the webhook response
|
||||
go func() {
|
||||
// Process asynchronously — don't block the webhook response.
|
||||
// RFC internal#524 Layer 1: globalGoAsync — HandleInbound traverses
|
||||
// db.DB to resolve workspace + record the channel event; drained by
|
||||
// drainTestAsync before db.DB swap.
|
||||
globalGoAsync(func() {
|
||||
bgCtx := context.Background()
|
||||
if err := h.manager.HandleInbound(bgCtx, ch, msg); err != nil {
|
||||
log.Printf("Channels: async HandleInbound error for workspace %s: %v", ch.WorkspaceID[:12], err)
|
||||
}
|
||||
}()
|
||||
})
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{"status": "accepted"})
|
||||
}
|
||||
|
||||
@@ -67,7 +67,7 @@ type ChatFilesHandler struct {
|
||||
|
||||
// httpClient is broken out so tests can swap in an httptest.Server
|
||||
// transport. Prod uses a default with a generous Timeout to cover
|
||||
// the 50 MB worst case on a slow EC2 link without leaving a
|
||||
// the 100 MB worst case on a slow EC2 link without leaving a
|
||||
// connection hanging forever on a sick workspace.
|
||||
httpClient *http.Client
|
||||
|
||||
@@ -89,9 +89,14 @@ func NewChatFilesHandler(t *TemplatesHandler) *ChatFilesHandler {
|
||||
return &ChatFilesHandler{
|
||||
templates: t,
|
||||
httpClient: &http.Client{
|
||||
// 50 MB total body cap / ~1 MB/s slow-network floor → ~60s.
|
||||
// Doubled for headroom on the legitimate-but-slow case.
|
||||
Timeout: 120 * time.Second,
|
||||
// 100 MB total body cap / ~100 KB/s slow-uplink floor → ~1000s.
|
||||
// Doubled for headroom on the legitimate-but-slow case (e.g.
|
||||
// reno-stars 2026-05-19 forensic a99ab0a1: 60MB upload over a
|
||||
// constrained uplink). Client-side AbortSignal.timeout (canvas
|
||||
// uploads.ts) computes the matching deadline per-request and
|
||||
// surfaces "connection too slow" — distinct from the file-size
|
||||
// pre-flight that returns immediately before any network I/O.
|
||||
Timeout: 1200 * time.Second,
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -107,10 +112,19 @@ func (h *ChatFilesHandler) WithPendingUploads(storage pendinguploads.Storage, br
|
||||
}
|
||||
|
||||
// chatUploadMaxBytes caps the full multipart request body so a
|
||||
// malicious / runaway client can't OOM the proxy hop. 50 MB matches
|
||||
// malicious / runaway client can't OOM the proxy hop. 100 MB matches
|
||||
// the workspace-side limit; anything larger is rejected at the
|
||||
// network boundary before forwarding.
|
||||
const chatUploadMaxBytes = 50 * 1024 * 1024
|
||||
//
|
||||
// CANVAS_MIRROR: keep aligned with canvas/src/components/tabs/chat/
|
||||
// uploads.ts MAX_UPLOAD_BYTES. The canvas constant exists so the
|
||||
// pre-flight size check can fail immediately (before network I/O)
|
||||
// with the actionable "File too large (got X MB) — limit is 100MB"
|
||||
// message. Bumping one side without the other yields the wrong-reason
|
||||
// surface that motivated this constant pair (CTO 2026-05-19 directive
|
||||
// on forensic a99ab0a1: file-size cause MUST surface as file-size,
|
||||
// NOT as a downstream timeout).
|
||||
const chatUploadMaxBytes = 100 * 1024 * 1024
|
||||
|
||||
// resolveWorkspaceForwardCreds resolves the workspace's URL +
|
||||
// platform_inbound_secret for an /internal/* forward, applying
|
||||
@@ -268,7 +282,7 @@ func contentDispositionAttachment(name string) string {
|
||||
// back unchanged.
|
||||
//
|
||||
// Why streaming, not parse-then-re-encode:
|
||||
// - Eliminates the 50 MB intermediate buffer on the platform.
|
||||
// - Eliminates the 100 MB intermediate buffer on the platform.
|
||||
// - Per-file size + path-safety enforcement is the workspace's job;
|
||||
// duplicating it here just creates two places to keep in sync.
|
||||
// - The error responses from the workspace (413 with the offending
|
||||
@@ -354,7 +368,7 @@ func (h *ChatFilesHandler) Upload(c *gin.Context) {
|
||||
// either.
|
||||
//
|
||||
// Body is streamed end-to-end (no buffering on the platform), preserving
|
||||
// binary safety and arbitrary file size (the 50 MB cap on Upload doesn't
|
||||
// binary safety and arbitrary file size (the 100 MB cap on Upload doesn't
|
||||
// apply to artefacts the agent produced).
|
||||
func (h *ChatFilesHandler) Download(c *gin.Context) {
|
||||
workspaceID := c.Param("id")
|
||||
@@ -546,8 +560,8 @@ type uploadedFile struct {
|
||||
// a fetcher crash mid-batch.
|
||||
//
|
||||
// Limits enforced here mirror the workspace-side ingest_handler:
|
||||
// - Total body cap: 50 MB (set on c.Request.Body before reaching us)
|
||||
// - Per-file cap: 25 MB (pendinguploads.MaxFileBytes; rejected as 413)
|
||||
// - Total body cap: 100 MB (set on c.Request.Body before reaching us)
|
||||
// - Per-file cap: 100 MB (pendinguploads.MaxFileBytes; rejected as 413)
|
||||
// - Filename: sanitized + capped at 100 chars (SanitizeFilename)
|
||||
//
|
||||
// Logging: every persisted file logs an INFO line with workspace_id,
|
||||
@@ -561,7 +575,7 @@ func (h *ChatFilesHandler) uploadPollMode(c *gin.Context, ctx context.Context, w
|
||||
// expose those limits directly — the underlying ParseMultipartForm
|
||||
// caps memory at 32 MB by default and spills to disk. For poll-
|
||||
// mode we read each file into memory to hand to Storage.Put;
|
||||
// 25 MB-per-file × 64-files ceiling means worst-case is 1.6 GB of
|
||||
// 100 MB-per-file × 64-files ceiling means worst-case is 6.4 GB of
|
||||
// peak memory. Bound the per-file size at the multipart layer so
|
||||
// the spill never gets close.
|
||||
if err := c.Request.ParseMultipartForm(32 << 20); err != nil {
|
||||
|
||||
@@ -374,7 +374,7 @@ func TestChatUpload_ForwardsErrorStatusUnchanged(t *testing.T) {
|
||||
|
||||
// Workspace returns 413 with its standard "exceeds per-file limit"
|
||||
// shape. Platform must propagate, NOT remap to 500.
|
||||
srv, _ := newCapturingWorkspace(t, http.StatusRequestEntityTooLarge, `{"error":"big.bin exceeds per-file limit (25 MB)"}`)
|
||||
srv, _ := newCapturingWorkspace(t, http.StatusRequestEntityTooLarge, `{"error":"big.bin exceeds per-file limit (100 MB)"}`)
|
||||
|
||||
wsID := "00000000-0000-0000-0000-000000000044"
|
||||
expectURL(mock, wsID, srv.URL)
|
||||
@@ -414,6 +414,81 @@ func TestChatUpload_WorkspaceUnreachable(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestChatUpload_BodyUnderCap_Forwards pins the lower edge of the new
|
||||
// 100 MB body cap (CTO 2026-05-19 directive on forensic a99ab0a1).
|
||||
// A multipart payload comfortably under the cap must reach the
|
||||
// workspace's /internal/chat/uploads/ingest unchanged.
|
||||
//
|
||||
// Uses a small fixture (matching the rest of this suite) — the
|
||||
// http.MaxBytesReader cap is applied via a constant; pinning the cap
|
||||
// _value_ + a sub-cap-forwards test gives equivalent coverage to a
|
||||
// real-bytes 99 MB upload at a fraction of the test runtime.
|
||||
func TestChatUpload_BodyUnderCap_Forwards(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
if chatUploadMaxBytes != 100*1024*1024 {
|
||||
t.Fatalf("chatUploadMaxBytes regressed: want 100MB, got %d bytes — bump must stay in lockstep with canvas MAX_UPLOAD_BYTES + workspace CHAT_UPLOAD_MAX_BYTES", chatUploadMaxBytes)
|
||||
}
|
||||
|
||||
srv, _ := newCapturingWorkspace(t, http.StatusOK, `{"files":[]}`)
|
||||
wsID := "00000000-0000-0000-0000-000000000046"
|
||||
expectURL(mock, wsID, srv.URL)
|
||||
expectInboundSecret(mock, wsID, "tok")
|
||||
|
||||
h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil, nil))
|
||||
body, ct := uploadFixture(t)
|
||||
c, w := makeUploadRequest(t, wsID, body, ct)
|
||||
h.Upload(c)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 for sub-cap forward, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestChatUpload_BodyOverCap_413 verifies the 100 MB cap is enforced
|
||||
// at the platform's MaxBytesReader boundary. Because MaxBytesReader is
|
||||
// applied to c.Request.Body, the workspace forward only fails AFTER
|
||||
// the reader returns ErrBodyOverflow mid-stream — the forward http
|
||||
// client surfaces that as an error, which lands as 502 BadGateway
|
||||
// (the platform's contract for "couldn't complete the forward"). The
|
||||
// alternative would be eager Content-Length inspection — left as a
|
||||
// follow-up so chunked uploads (no Content-Length) still hit the
|
||||
// same gate.
|
||||
//
|
||||
// What this test pins: the cap CONSTANT is set to 100 MB and a body
|
||||
// strictly above the cap does NOT silently succeed (the upstream
|
||||
// receives a truncated body, the test workspace's parser would have
|
||||
// failed; here we simulate via a too-large body and assert non-2xx).
|
||||
func TestChatUpload_BodyOverCap_NotOK(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
// Capturing server that mimics workspace behaviour on truncated
|
||||
// multipart: returns 400. The test asserts the platform does NOT
|
||||
// turn this into a 200 success.
|
||||
srv, _ := newCapturingWorkspace(t, http.StatusBadRequest, `{"error":"malformed multipart"}`)
|
||||
wsID := "00000000-0000-0000-0000-000000000047"
|
||||
expectURL(mock, wsID, srv.URL)
|
||||
expectInboundSecret(mock, wsID, "tok")
|
||||
|
||||
h := NewChatFilesHandler(NewTemplatesHandler(t.TempDir(), nil, nil))
|
||||
|
||||
// Build a synthetic body that exceeds chatUploadMaxBytes by a
|
||||
// few bytes. We don't materialise 100MB+ in test memory — the
|
||||
// MaxBytesReader limit is applied lazily as the body is read,
|
||||
// so a marker-sized buffer + a custom reader that claims a large
|
||||
// Content-Length is enough to trip the gate.
|
||||
body := bytes.NewBuffer(make([]byte, chatUploadMaxBytes+1))
|
||||
c, w := makeUploadRequest(t, wsID, body, "multipart/form-data; boundary=----test")
|
||||
c.Request.ContentLength = int64(chatUploadMaxBytes + 1)
|
||||
h.Upload(c)
|
||||
|
||||
if w.Code >= 200 && w.Code < 300 {
|
||||
t.Errorf("expected non-2xx on over-cap upload, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestChatDownload_InvalidPath(t *testing.T) {
|
||||
setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
@@ -185,10 +185,15 @@ func (h *DelegationHandler) Delegate(c *gin.Context) {
|
||||
delegationCtx, cancelDelegation := context.WithTimeout(
|
||||
context.WithoutCancel(ctx), 30*time.Minute,
|
||||
)
|
||||
go func() {
|
||||
// RFC internal#524 Layer 1: route through workspace.goAsync so the
|
||||
// detached executeDelegation (which writes A2A status rows to db.DB
|
||||
// across multiple stages) is drained before db.DB is restored in a
|
||||
// later test's t.Cleanup. Tracked via the parent workspace handler's
|
||||
// asyncWG.
|
||||
h.workspace.goAsync(func() {
|
||||
defer cancelDelegation()
|
||||
h.executeDelegation(delegationCtx, sourceID, body.TargetID, delegationID, a2aBody)
|
||||
}()
|
||||
})
|
||||
|
||||
// Broadcast event so canvas shows delegation in real-time
|
||||
h.broadcaster.RecordAndBroadcast(ctx, string(events.EventDelegationSent), sourceID, map[string]interface{}{
|
||||
|
||||
@@ -129,6 +129,14 @@ var (
|
||||
|
||||
// getEICTunnelPool returns the singleton pool, lazy-initialising on
|
||||
// first call. Idempotent.
|
||||
//
|
||||
// goAsync-exempt (RFC internal#524 Layer 2.2): every `go` in this file
|
||||
// is pool-internal lifecycle (janitor + per-entry cleanup closures).
|
||||
// None reads db.DB — the pool tracks SSH tunnels, not workspace state.
|
||||
// The janitor exits on close(p.stopJanitor); cleanups exit when the
|
||||
// captured tunnel's resources are released. Wrapping in globalGoAsync
|
||||
// would block test cleanup on the singleton janitor that intentionally
|
||||
// runs forever.
|
||||
func getEICTunnelPool() *eicTunnelPool {
|
||||
globalEICTunnelPoolOnce.Do(func() {
|
||||
globalEICTunnelPool = newEICTunnelPool()
|
||||
|
||||
@@ -48,6 +48,14 @@ func init() {
|
||||
// finish. Called from setupTestDB's cleanup before db.DB is restored so
|
||||
// no detached restart/provision goroutine is mid-read of db.DB when the
|
||||
// pointer is swapped.
|
||||
//
|
||||
// Also drains the package-level globalAsync WaitGroup (RFC internal#524
|
||||
// Layer 1 deliverable 2) so sibling handlers (SecretsHandler /
|
||||
// PluginsHandler / etc.) that route through globalGoAsync rather than
|
||||
// h.goAsync are likewise drained before db.DB is swapped. Without this
|
||||
// drain a SecretsHandler.Set's restartFunc-via-globalGoAsync could race
|
||||
// the db.DB restore exactly the same way maybeMarkContainerDead did
|
||||
// before commit 69d9b4e3.
|
||||
func drainTestAsync() {
|
||||
liveTestHandlersMu.Lock()
|
||||
handlers := make([]*WorkspaceHandler, len(liveTestHandlers))
|
||||
@@ -56,6 +64,7 @@ func drainTestAsync() {
|
||||
for _, h := range handlers {
|
||||
h.waitAsyncForTest()
|
||||
}
|
||||
waitGlobalAsyncForTest()
|
||||
}
|
||||
|
||||
// setupTestDB creates a sqlmock DB and assigns it to the global db.DB.
|
||||
|
||||
@@ -278,7 +278,10 @@ func (h *MCPHandler) toolDelegateTaskAsync(ctx context.Context, callerID string,
|
||||
|
||||
// Fire and forget in a detached goroutine. Use a background context so
|
||||
// the call is not cancelled when the HTTP request completes.
|
||||
go func() {
|
||||
// RFC internal#524 Layer 1: globalGoAsync — the detached call reads
|
||||
// db.DB (mcpResolveURL + updateMCPDelegationStatus) and must be
|
||||
// drained by drainTestAsync before any t.Cleanup-driven db.DB swap.
|
||||
globalGoAsync(func() {
|
||||
bgCtx, cancel := context.WithTimeout(context.Background(), mcpAsyncCallTimeout)
|
||||
defer cancel()
|
||||
|
||||
@@ -322,7 +325,7 @@ func (h *MCPHandler) toolDelegateTaskAsync(ctx context.Context, callerID string,
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
// Drain response so the connection can be reused.
|
||||
_, _ = io.Copy(io.Discard, resp.Body)
|
||||
}()
|
||||
})
|
||||
|
||||
return fmt.Sprintf(`{"task_id":%q,"status":"dispatched","target_id":%q}`, delegationID, targetID), nil
|
||||
}
|
||||
|
||||
@@ -534,10 +534,14 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX
|
||||
// Docker-mode otherwise; the org-import call site doesn't need
|
||||
// to know which.
|
||||
provisionSem <- struct{}{} // acquire
|
||||
go func(wID, tPath string, cFiles map[string][]byte, p models.CreateWorkspacePayload) {
|
||||
// RFC internal#524 Layer 1: route through workspace.goAsync —
|
||||
// provisionWorkspaceAuto inserts/updates the workspaces row in
|
||||
// db.DB and must be drained before any test cleanup swap.
|
||||
wID, tPath, cFiles, p := id, templatePath, configFiles, payload
|
||||
h.workspace.goAsync(func() {
|
||||
defer func() { <-provisionSem }() // release
|
||||
h.workspace.provisionWorkspaceAuto(wID, tPath, cFiles, p)
|
||||
}(id, templatePath, configFiles, payload)
|
||||
})
|
||||
}
|
||||
|
||||
// Insert schedules if defined. Resolve each schedule's prompt body from
|
||||
|
||||
@@ -198,12 +198,16 @@ func (h *PluginsHandler) uninstallViaDocker(ctx context.Context, c *gin.Context,
|
||||
log.Printf("Plugin uninstall: failed to delete workspace_plugins row for %s: %v (container cleanup succeeded)", pluginName, err)
|
||||
}
|
||||
|
||||
// Auto-restart (small delay to ensure fs writes are flushed)
|
||||
// Auto-restart (small delay to ensure fs writes are flushed).
|
||||
// RFC internal#524 Layer 1: globalGoAsync so the detached restart
|
||||
// goroutine is drained by drainTestAsync before db.DB swap. See
|
||||
// workspace.go:globalGoAsync for the contract.
|
||||
if h.restartFunc != nil {
|
||||
go func() {
|
||||
wsID := workspaceID
|
||||
globalGoAsync(func() {
|
||||
time.Sleep(2 * time.Second)
|
||||
h.restartFunc(workspaceID)
|
||||
}()
|
||||
h.restartFunc(wsID)
|
||||
})
|
||||
}
|
||||
|
||||
log.Printf("Plugin uninstall: %s from workspace %s (restarting)", pluginName, workspaceID)
|
||||
@@ -260,10 +264,12 @@ func (h *PluginsHandler) uninstallViaEIC(ctx context.Context, c *gin.Context, wo
|
||||
}
|
||||
|
||||
if h.restartFunc != nil {
|
||||
go func() {
|
||||
// RFC internal#524 Layer 1: see uninstallViaDocker above.
|
||||
wsID := workspaceID
|
||||
globalGoAsync(func() {
|
||||
time.Sleep(2 * time.Second)
|
||||
h.restartFunc(workspaceID)
|
||||
}()
|
||||
h.restartFunc(wsID)
|
||||
})
|
||||
}
|
||||
|
||||
log.Printf("Plugin uninstall: %s from workspace %s (restarting via SaaS path)", pluginName, workspaceID)
|
||||
|
||||
@@ -320,7 +320,10 @@ func (h *PluginsHandler) deliverToContainer(ctx context.Context, workspaceID str
|
||||
if kind == classifyKindSkillContentOnly {
|
||||
log.Printf("Plugin install: %s → workspace %s — SKILL-content-only update, SKIPPING restart", r.PluginName, workspaceID)
|
||||
} else {
|
||||
go h.restartFunc(workspaceID)
|
||||
// RFC internal#524 Layer 1: drain via globalGoAsync (see
|
||||
// workspace.go:globalGoAsync).
|
||||
wsID := workspaceID
|
||||
globalGoAsync(func() { h.restartFunc(wsID) })
|
||||
}
|
||||
}
|
||||
return nil
|
||||
@@ -334,7 +337,9 @@ func (h *PluginsHandler) deliverToContainer(ctx context.Context, workspaceID str
|
||||
})
|
||||
}
|
||||
if h.restartFunc != nil {
|
||||
go h.restartFunc(workspaceID)
|
||||
// RFC internal#524 Layer 1: see Docker path above.
|
||||
wsID := workspaceID
|
||||
globalGoAsync(func() { h.restartFunc(wsID) })
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -819,8 +819,11 @@ func (h *RegistryHandler) evaluateStatus(c *gin.Context, payload models.Heartbea
|
||||
if payload.ActiveTasks < maxConcurrent {
|
||||
// context.WithoutCancel: heartbeat handler's ctx is about to
|
||||
// expire as soon as we return. The drain needs to outlive it.
|
||||
// RFC internal#524 Layer 1: drainQueue reads db.DB; route
|
||||
// through globalGoAsync so test cleanup waits for it.
|
||||
drainCtx := context.WithoutCancel(ctx)
|
||||
go h.drainQueue(drainCtx, payload.WorkspaceID)
|
||||
wsID := payload.WorkspaceID
|
||||
globalGoAsync(func() { h.drainQueue(drainCtx, wsID) })
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
package handlers
|
||||
|
||||
// rfc524_layer1_async_drain_test.go — regression test for RFC internal#524
|
||||
// Layer 1 forward-port. Asserts:
|
||||
//
|
||||
// 1. globalGoAsync goroutines are drained by drainTestAsync before the
|
||||
// test cleanup chain returns control.
|
||||
// 2. Routing through globalGoAsync (rather than bare `go ...`) ensures
|
||||
// a sibling-handler's detached goroutine cannot outlive a test's
|
||||
// db.DB swap.
|
||||
//
|
||||
// Companion of handlers_test.go:drainTestAsync (canonical 69d9b4e3 fix
|
||||
// extended to non-*WorkspaceHandler call sites). If either property
|
||||
// regresses, this test fails fast.
|
||||
|
||||
import (
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// TestRFC524_GlobalGoAsync_DrainsBeforeCleanup asserts that goroutines
|
||||
// scheduled via globalGoAsync run to completion before drainTestAsync
|
||||
// returns. Concretely: schedule a globalGoAsync that flips a counter
|
||||
// after a short sleep, then call drainTestAsync; the counter must
|
||||
// already be 1 when the call returns.
|
||||
func TestRFC524_GlobalGoAsync_DrainsBeforeCleanup(t *testing.T) {
|
||||
var ran int32
|
||||
globalGoAsync(func() {
|
||||
time.Sleep(20 * time.Millisecond)
|
||||
atomic.StoreInt32(&ran, 1)
|
||||
})
|
||||
|
||||
// drainTestAsync drains per-handler asyncWG + the package-level
|
||||
// globalAsync WG. After it returns the goroutine MUST have run.
|
||||
drainTestAsync()
|
||||
|
||||
if atomic.LoadInt32(&ran) != 1 {
|
||||
t.Fatalf("drainTestAsync returned before globalGoAsync goroutine finished — regression of RFC internal#524 Layer 1 drain coupling")
|
||||
}
|
||||
}
|
||||
|
||||
// TestRFC524_GlobalGoAsync_MultipleConcurrent asserts the drain is
|
||||
// O(n)-correct: schedule a fan-out of globalGoAsync calls (like
|
||||
// restartAllAffectedByGlobalKey does on a large global secret rotation)
|
||||
// and confirm every one completes before drainTestAsync returns.
|
||||
func TestRFC524_GlobalGoAsync_MultipleConcurrent(t *testing.T) {
|
||||
const n = 32
|
||||
var completed int32
|
||||
for i := 0; i < n; i++ {
|
||||
globalGoAsync(func() {
|
||||
// Short, random-ish work; the point is they're all in flight
|
||||
// at the same time when drainTestAsync is called.
|
||||
time.Sleep(5 * time.Millisecond)
|
||||
atomic.AddInt32(&completed, 1)
|
||||
})
|
||||
}
|
||||
|
||||
drainTestAsync()
|
||||
|
||||
got := atomic.LoadInt32(&completed)
|
||||
if got != n {
|
||||
t.Fatalf("drainTestAsync returned with %d/%d globalGoAsync goroutines incomplete — fan-out drain broken", n-got, n)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRFC524_HandlerGoAsync_AndGlobalAsync_BothDrained asserts that
|
||||
// drainTestAsync waits for BOTH the per-handler asyncWG (the original
|
||||
// 69d9b4e3 primitive) AND the package-level globalAsync (the Layer 1
|
||||
// extension). Schedules one of each and confirms both finish.
|
||||
func TestRFC524_HandlerGoAsync_AndGlobalAsync_BothDrained(t *testing.T) {
|
||||
setupTestDB(t) // registers handlers + arms the drain
|
||||
|
||||
var perHandlerDone, globalDone int32
|
||||
wh := NewWorkspaceHandler(nil, nil, "", t.TempDir())
|
||||
wh.goAsync(func() {
|
||||
time.Sleep(15 * time.Millisecond)
|
||||
atomic.StoreInt32(&perHandlerDone, 1)
|
||||
})
|
||||
globalGoAsync(func() {
|
||||
time.Sleep(15 * time.Millisecond)
|
||||
atomic.StoreInt32(&globalDone, 1)
|
||||
})
|
||||
|
||||
drainTestAsync()
|
||||
|
||||
if atomic.LoadInt32(&perHandlerDone) != 1 {
|
||||
t.Errorf("per-handler asyncWG drain regressed (RFC internal#524 Layer 1 expects 69d9b4e3 to remain wired)")
|
||||
}
|
||||
if atomic.LoadInt32(&globalDone) != 1 {
|
||||
t.Errorf("global async drain not wired (RFC internal#524 Layer 1 extension missing)")
|
||||
}
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"net/http"
|
||||
"regexp"
|
||||
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/audit"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/crypto"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth"
|
||||
@@ -262,9 +263,24 @@ func (h *SecretsHandler) Set(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Auto-restart workspace to pick up new secret
|
||||
// Phase 1 audit: structured event for the security trail. Inline (not
|
||||
// goroutine) so the event is durable before we ack the user; emit is
|
||||
// best-effort and never errors out of the request path.
|
||||
audit.Emit(c.Request.Context(), "secret.set", map[string]any{
|
||||
"workspace_id": workspaceID,
|
||||
"key": body.Key,
|
||||
"value_hash": audit.HashValuePrefix(body.Value, 8),
|
||||
"scope": "workspace",
|
||||
"operation": "set",
|
||||
})
|
||||
|
||||
// Auto-restart workspace to pick up new secret.
|
||||
// RFC internal#524 Layer 1: route through globalGoAsync so tests can
|
||||
// drain the detached restart goroutine before db.DB is swapped — see
|
||||
// drainTestAsync in handlers_test.go and the canonical 69d9b4e3 fix.
|
||||
if h.restartFunc != nil {
|
||||
go h.restartFunc(workspaceID)
|
||||
wsID := workspaceID
|
||||
globalGoAsync(func() { h.restartFunc(wsID) })
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{"status": "saved", "key": body.Key})
|
||||
@@ -297,9 +313,20 @@ func (h *SecretsHandler) Delete(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Auto-restart workspace to pick up removed secret
|
||||
// Phase 1 audit: structured event for the security trail. Only on
|
||||
// real deletes (rows>0) — a 404 is not a state change.
|
||||
audit.Emit(c.Request.Context(), "secret.delete", map[string]any{
|
||||
"workspace_id": workspaceID,
|
||||
"key": key,
|
||||
"scope": "workspace",
|
||||
"operation": "delete",
|
||||
})
|
||||
|
||||
// Auto-restart workspace to pick up removed secret.
|
||||
// RFC internal#524 Layer 1: see Set() above for the drain rationale.
|
||||
if h.restartFunc != nil {
|
||||
go h.restartFunc(workspaceID)
|
||||
wsID := workspaceID
|
||||
globalGoAsync(func() { h.restartFunc(wsID) })
|
||||
}
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{"status": "deleted", "key": key})
|
||||
@@ -379,7 +406,22 @@ func (h *SecretsHandler) SetGlobal(c *gin.Context) {
|
||||
// reach existing workspaces until the container is recreated. Auto-restart
|
||||
// every workspace whose env is affected — i.e. those WITHOUT a
|
||||
// workspace-level override of the same key.
|
||||
go h.restartAllAffectedByGlobalKey(body.Key)
|
||||
//
|
||||
// RFC internal#524 Layer 1: globalGoAsync so tests drain the fan-out
|
||||
// (which itself spawns N more globalGoAsync restart calls below) before
|
||||
// db.DB swap. Without this, the SELECT for affected workspaces races a
|
||||
// subsequent test's db.DB restore.
|
||||
key := body.Key
|
||||
globalGoAsync(func() { h.restartAllAffectedByGlobalKey(key) })
|
||||
|
||||
// Phase 1 audit: admin-scope secret write — high-value security event.
|
||||
auditCtx := audit.WithActorKind(c.Request.Context(), audit.ActorAdmin)
|
||||
audit.Emit(auditCtx, "secret.set", map[string]any{
|
||||
"key": body.Key,
|
||||
"value_hash": audit.HashValuePrefix(body.Value, 8),
|
||||
"scope": "global",
|
||||
"operation": "set",
|
||||
})
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{"status": "saved", "key": body.Key, "scope": "global"})
|
||||
}
|
||||
@@ -423,7 +465,11 @@ func (h *SecretsHandler) restartAllAffectedByGlobalKey(key string) {
|
||||
}
|
||||
log.Printf("Global secret %s changed: auto-restarting %d workspace(s) to refresh env", key, len(ids))
|
||||
for _, id := range ids {
|
||||
go h.restartFunc(id)
|
||||
// RFC internal#524 Layer 1: per-workspace restart via globalGoAsync
|
||||
// so each restart goroutine is drained before db.DB is swapped in
|
||||
// the test cleanup chain.
|
||||
wsID := id
|
||||
globalGoAsync(func() { h.restartFunc(wsID) })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -450,7 +496,18 @@ func (h *SecretsHandler) DeleteGlobal(c *gin.Context) {
|
||||
|
||||
// Issue #15: propagate deletion to running containers — otherwise they
|
||||
// keep the stale env var until manual restart.
|
||||
go h.restartAllAffectedByGlobalKey(key)
|
||||
// RFC internal#524 Layer 1: globalGoAsync for the same drain rationale
|
||||
// as SetGlobal above.
|
||||
k := key
|
||||
globalGoAsync(func() { h.restartAllAffectedByGlobalKey(k) })
|
||||
|
||||
// Phase 1 audit: admin-scope secret delete.
|
||||
auditCtx := audit.WithActorKind(c.Request.Context(), audit.ActorAdmin)
|
||||
audit.Emit(auditCtx, "secret.delete", map[string]any{
|
||||
"key": key,
|
||||
"scope": "global",
|
||||
"operation": "delete",
|
||||
})
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{"status": "deleted", "key": key, "scope": "global"})
|
||||
}
|
||||
@@ -461,11 +518,24 @@ func (h *SecretsHandler) GetModel(c *gin.Context) {
|
||||
workspaceID := c.Param("id")
|
||||
ctx := c.Request.Context()
|
||||
|
||||
// Check if MODEL_PROVIDER secret exists
|
||||
// Check if MODEL secret exists.
|
||||
//
|
||||
// Historical note: this row was named MODEL_PROVIDER pre-2026-05-19
|
||||
// (see ab12af50 + a7e8892 root-cause analysis). The column name
|
||||
// MODEL_PROVIDER was misleading — it never held a provider slug,
|
||||
// only the picked model id (e.g. "minimax/MiniMax-M2.7"). The
|
||||
// misnomer caused workspace-server's applyRuntimeModelEnv to
|
||||
// overwrite a legitimate persona-env MODEL with whatever literal
|
||||
// string lived in MODEL_PROVIDER (often "minimax" or "claude-code"
|
||||
// — not a valid model id), wedging adapters at SDK initialize.
|
||||
// CP-side slot-separation (cp#213 + cp#220) already corrected the
|
||||
// CP-side analogue; this is the workspace-server companion. A
|
||||
// migration in 20260519000000_workspace_secrets_model_provider_rename.up.sql
|
||||
// moves any legacy rows to the new key on rollout.
|
||||
var modelBytes []byte
|
||||
var modelVersion int
|
||||
err := db.DB.QueryRowContext(ctx,
|
||||
`SELECT encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = $1 AND key = 'MODEL_PROVIDER'`,
|
||||
`SELECT encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = $1 AND key = 'MODEL'`,
|
||||
workspaceID).Scan(&modelBytes, &modelVersion)
|
||||
if err == sql.ErrNoRows {
|
||||
c.JSON(http.StatusOK, gin.H{"model": "", "source": "default"})
|
||||
@@ -485,18 +555,23 @@ func (h *SecretsHandler) GetModel(c *gin.Context) {
|
||||
c.JSON(http.StatusOK, gin.H{"model": string(decrypted), "source": "workspace_secrets"})
|
||||
}
|
||||
|
||||
// setModelSecret writes (or clears, when value=="") the MODEL_PROVIDER
|
||||
// workspace secret. Extracted from SetModel so non-handler call sites
|
||||
// (notably WorkspaceHandler.Create — first-deploy path that persists the
|
||||
// setModelSecret writes (or clears, when value=="") the MODEL workspace
|
||||
// secret. Extracted from SetModel so non-handler call sites (notably
|
||||
// WorkspaceHandler.Create — first-deploy path that persists the
|
||||
// canvas-selected model so applyRuntimeModelEnv's restart fallback finds
|
||||
// it) can reuse the encryption + upsert logic without inlining the SQL.
|
||||
//
|
||||
// The row was previously keyed MODEL_PROVIDER (misnomer — it never held
|
||||
// a provider, only a model id). Renamed to MODEL on 2026-05-19; the
|
||||
// 20260519000000_workspace_secrets_model_provider_rename migration moves
|
||||
// any legacy rows on rollout.
|
||||
//
|
||||
// Returns nil on success. Caller is responsible for any restart trigger;
|
||||
// the gin handler re-adds that after a successful write.
|
||||
func setModelSecret(ctx context.Context, workspaceID, model string) error {
|
||||
if model == "" {
|
||||
_, err := db.DB.ExecContext(ctx,
|
||||
`DELETE FROM workspace_secrets WHERE workspace_id = $1 AND key = 'MODEL_PROVIDER'`,
|
||||
`DELETE FROM workspace_secrets WHERE workspace_id = $1 AND key = 'MODEL'`,
|
||||
workspaceID)
|
||||
return err
|
||||
}
|
||||
@@ -507,7 +582,7 @@ func setModelSecret(ctx context.Context, workspaceID, model string) error {
|
||||
version := crypto.CurrentEncryptionVersion()
|
||||
_, err = db.DB.ExecContext(ctx, `
|
||||
INSERT INTO workspace_secrets (workspace_id, key, encrypted_value, encryption_version)
|
||||
VALUES ($1, 'MODEL_PROVIDER', $2, $3)
|
||||
VALUES ($1, 'MODEL', $2, $3)
|
||||
ON CONFLICT (workspace_id, key) DO UPDATE
|
||||
SET encrypted_value = $2, encryption_version = $3, updated_at = now()
|
||||
`, workspaceID, encrypted, version)
|
||||
@@ -515,7 +590,7 @@ func setModelSecret(ctx context.Context, workspaceID, model string) error {
|
||||
}
|
||||
|
||||
// SetModel handles PUT /workspaces/:id/model — writes the model slug
|
||||
// into workspace_secrets as MODEL_PROVIDER (the key GetModel reads).
|
||||
// into workspace_secrets as MODEL (the key GetModel reads).
|
||||
// For hermes, the value is a hermes-native slug like "minimax/MiniMax-M2.7";
|
||||
// for langgraph it's the legacy "provider:model" form. Either way it's just
|
||||
// an opaque string the runtime interprets on its next start.
|
||||
@@ -552,7 +627,9 @@ func (h *SecretsHandler) SetModel(c *gin.Context) {
|
||||
}
|
||||
|
||||
if h.restartFunc != nil {
|
||||
go h.restartFunc(workspaceID)
|
||||
// RFC internal#524 Layer 1: globalGoAsync (see Set()).
|
||||
wsID := workspaceID
|
||||
globalGoAsync(func() { h.restartFunc(wsID) })
|
||||
}
|
||||
if body.Model == "" {
|
||||
c.JSON(http.StatusOK, gin.H{"status": "cleared"})
|
||||
@@ -669,7 +746,9 @@ func (h *SecretsHandler) SetProvider(c *gin.Context) {
|
||||
}
|
||||
|
||||
if h.restartFunc != nil {
|
||||
go h.restartFunc(workspaceID)
|
||||
// RFC internal#524 Layer 1: globalGoAsync (see Set()).
|
||||
wsID := workspaceID
|
||||
globalGoAsync(func() { h.restartFunc(wsID) })
|
||||
}
|
||||
if body.Provider == "" {
|
||||
c.JSON(http.StatusOK, gin.H{"status": "cleared"})
|
||||
|
||||
@@ -479,8 +479,10 @@ func TestSecretsGetModel_Default(t *testing.T) {
|
||||
setupTestRedis(t)
|
||||
handler := NewSecretsHandler(nil)
|
||||
|
||||
// No MODEL_PROVIDER secret
|
||||
mock.ExpectQuery("SELECT encrypted_value, encryption_version FROM workspace_secrets").
|
||||
// No MODEL secret (formerly MODEL_PROVIDER — see 2026-05-19 rename
|
||||
// migration). Pin the WHERE clause so a regression that reads the
|
||||
// wrong column-name shows up here.
|
||||
mock.ExpectQuery(`SELECT encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = \$1 AND key = 'MODEL'`).
|
||||
WithArgs("ws-model").
|
||||
WillReturnError(sql.ErrNoRows)
|
||||
|
||||
@@ -516,7 +518,7 @@ func TestSecretsGetModel_DBError(t *testing.T) {
|
||||
setupTestRedis(t)
|
||||
handler := NewSecretsHandler(nil)
|
||||
|
||||
mock.ExpectQuery("SELECT encrypted_value, encryption_version FROM workspace_secrets").
|
||||
mock.ExpectQuery(`SELECT encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = \$1 AND key = 'MODEL'`).
|
||||
WithArgs("ws-model-err").
|
||||
WillReturnError(sql.ErrConnDone)
|
||||
|
||||
@@ -544,7 +546,9 @@ func TestSecretsSetModel_Upsert(t *testing.T) {
|
||||
restartCalled := make(chan string, 1)
|
||||
handler := NewSecretsHandler(func(id string) { restartCalled <- id })
|
||||
|
||||
mock.ExpectExec(`INSERT INTO workspace_secrets`).
|
||||
// Pin the literal 'MODEL' key in the SQL so a regression to the
|
||||
// pre-2026-05-19 'MODEL_PROVIDER' column name shows up here.
|
||||
mock.ExpectExec(`INSERT INTO workspace_secrets[\s\S]*'MODEL'`).
|
||||
WithArgs("00000000-0000-0000-0000-000000000001", sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(1, 1))
|
||||
|
||||
@@ -578,7 +582,8 @@ func TestSecretsSetModel_EmptyClears(t *testing.T) {
|
||||
setupTestRedis(t)
|
||||
handler := NewSecretsHandler(func(string) {})
|
||||
|
||||
mock.ExpectExec(`DELETE FROM workspace_secrets`).
|
||||
// Pin the literal 'MODEL' key — see TestSecretsSetModel_Upsert.
|
||||
mock.ExpectExec(`DELETE FROM workspace_secrets WHERE workspace_id = \$1 AND key = 'MODEL'`).
|
||||
WithArgs("00000000-0000-0000-0000-000000000002").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
@@ -618,6 +623,65 @@ func TestSecretsSetModel_InvalidID(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestSecretsModel_RoundTrip_KeyIsMODELNotMODEL_PROVIDER pins the
|
||||
// 2026-05-19 rename: writes via SetModel land under workspace_secrets
|
||||
// key='MODEL', and reads via GetModel hit the same key. A regression
|
||||
// that reverts either side to 'MODEL_PROVIDER' will mismatch sqlmock's
|
||||
// query-regex anchor and fail loudly here. Combined integration-shape
|
||||
// guard for the secrets.go half of fix/workspace-server-rename-
|
||||
// MODEL_PROVIDER-to-MODEL.
|
||||
func TestSecretsModel_RoundTrip_KeyIsMODELNotMODEL_PROVIDER(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
handler := NewSecretsHandler(func(string) {})
|
||||
|
||||
// 1. SetModel — must hit key='MODEL' in the INSERT.
|
||||
mock.ExpectExec(`INSERT INTO workspace_secrets[\s\S]*'MODEL'[\s\S]*ON CONFLICT`).
|
||||
WithArgs("00000000-0000-0000-0000-000000000099", sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(1, 1))
|
||||
|
||||
w1 := httptest.NewRecorder()
|
||||
c1, _ := gin.CreateTestContext(w1)
|
||||
c1.Params = gin.Params{{Key: "id", Value: "00000000-0000-0000-0000-000000000099"}}
|
||||
c1.Request = httptest.NewRequest("PUT", "/workspaces/00000000-0000-0000-0000-000000000099/model",
|
||||
strings.NewReader(`{"model":"gpt-5.5"}`))
|
||||
c1.Request.Header.Set("Content-Type", "application/json")
|
||||
handler.SetModel(c1)
|
||||
if w1.Code != http.StatusOK {
|
||||
t.Fatalf("SetModel: expected 200, got %d: %s", w1.Code, w1.Body.String())
|
||||
}
|
||||
|
||||
// 2. GetModel — must hit key='MODEL' in the SELECT. Return raw
|
||||
// bytes; the handler will run them through DecryptVersioned.
|
||||
// crypto is disabled in the test env (no MASTER_KEY), so the
|
||||
// raw bytes pass through unchanged. We assert the SELECT
|
||||
// fires against key='MODEL' (the rename pin); the decoded
|
||||
// value isn't load-bearing for this contract test.
|
||||
mock.ExpectQuery(`SELECT encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = \$1 AND key = 'MODEL'`).
|
||||
WithArgs("00000000-0000-0000-0000-000000000099").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"encrypted_value", "encryption_version"}).
|
||||
AddRow([]byte("gpt-5.5"), 0))
|
||||
|
||||
w2 := httptest.NewRecorder()
|
||||
c2, _ := gin.CreateTestContext(w2)
|
||||
c2.Params = gin.Params{{Key: "id", Value: "00000000-0000-0000-0000-000000000099"}}
|
||||
c2.Request = httptest.NewRequest("GET", "/workspaces/00000000-0000-0000-0000-000000000099/model", nil)
|
||||
handler.GetModel(c2)
|
||||
if w2.Code != http.StatusOK {
|
||||
t.Fatalf("GetModel: expected 200, got %d: %s", w2.Code, w2.Body.String())
|
||||
}
|
||||
|
||||
// We don't assert resp["model"] equals "gpt-5.5" because crypto
|
||||
// state in this package varies by build tag; the load-bearing
|
||||
// contract is the workspace_secrets key, pinned by the sqlmock
|
||||
// regex above. If a future change adds encryption to the test
|
||||
// env, the round-trip value check can move to an integration
|
||||
// test that owns the crypto state.
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations — Model round-trip did not hit key='MODEL' on both sides: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== GetProvider / SetProvider (Option B PR-2) ====================
|
||||
//
|
||||
// Mirror of the GetModel/SetModel suite. Same secret-storage shape (key=
|
||||
|
||||
@@ -88,6 +88,11 @@ func (h *SocketHandler) HandleConnect(c *gin.Context) {
|
||||
|
||||
// Wrap WritePump and ReadPump so the gauge is decremented exactly once
|
||||
// when the client's write goroutine exits (WritePump owns conn lifetime).
|
||||
// goAsync-exempt (RFC internal#524 Layer 2.2): WebSocket pumps live
|
||||
// for the duration of the client connection (minutes-hours), not a
|
||||
// single request. Wrapping them in globalGoAsync would block every
|
||||
// test's t.Cleanup until every connected WS client disconnects. No
|
||||
// db.DB access in either pump.
|
||||
go func() {
|
||||
ws.WritePump(client)
|
||||
metrics.TrackWSDisconnect()
|
||||
|
||||
@@ -234,7 +234,9 @@ func (h *TemplatesHandler) ReplaceFiles(c *gin.Context) {
|
||||
"source": "ec2-ssh",
|
||||
})
|
||||
if h.wh != nil {
|
||||
go h.wh.RestartByID(workspaceID)
|
||||
// RFC internal#524 Layer 1: per-handler goAsync (drains via h.wh.waitAsyncForTest)
|
||||
wsID := workspaceID
|
||||
h.wh.goAsync(func() { h.wh.RestartByID(wsID) })
|
||||
}
|
||||
return
|
||||
}
|
||||
@@ -268,7 +270,9 @@ func (h *TemplatesHandler) ReplaceFiles(c *gin.Context) {
|
||||
"source": "container",
|
||||
})
|
||||
if h.wh != nil {
|
||||
go h.wh.RestartByID(workspaceID)
|
||||
// RFC internal#524 Layer 1: per-handler goAsync (drains via h.wh.waitAsyncForTest)
|
||||
wsID := workspaceID
|
||||
h.wh.goAsync(func() { h.wh.RestartByID(wsID) })
|
||||
}
|
||||
return
|
||||
}
|
||||
@@ -288,6 +292,8 @@ func (h *TemplatesHandler) ReplaceFiles(c *gin.Context) {
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{"status": "replaced", "workspace": workspaceID, "files": len(body.Files), "source": "volume"})
|
||||
if h.wh != nil {
|
||||
go h.wh.RestartByID(workspaceID)
|
||||
// RFC internal#524 Layer 1: per-handler goAsync (drains via h.wh.waitAsyncForTest)
|
||||
wsID := workspaceID
|
||||
h.wh.goAsync(func() { h.wh.RestartByID(wsID) })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -570,7 +570,9 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) {
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"status": "saved", "path": filePath})
|
||||
if h.wh != nil {
|
||||
go h.wh.RestartByID(workspaceID)
|
||||
// RFC internal#524 Layer 1: per-handler goAsync (drains via h.wh.waitAsyncForTest)
|
||||
wsID := workspaceID
|
||||
h.wh.goAsync(func() { h.wh.RestartByID(wsID) })
|
||||
}
|
||||
return
|
||||
}
|
||||
@@ -584,7 +586,9 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) {
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"status": "saved", "path": filePath})
|
||||
if h.wh != nil {
|
||||
go h.wh.RestartByID(workspaceID)
|
||||
// RFC internal#524 Layer 1: per-handler goAsync (drains via h.wh.waitAsyncForTest)
|
||||
wsID := workspaceID
|
||||
h.wh.goAsync(func() { h.wh.RestartByID(wsID) })
|
||||
}
|
||||
return
|
||||
}
|
||||
@@ -598,7 +602,9 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) {
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"status": "saved", "path": filePath})
|
||||
if h.wh != nil {
|
||||
go h.wh.RestartByID(workspaceID)
|
||||
// RFC internal#524 Layer 1: per-handler goAsync (drains via h.wh.waitAsyncForTest)
|
||||
wsID := workspaceID
|
||||
h.wh.goAsync(func() { h.wh.RestartByID(wsID) })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -651,7 +657,9 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) {
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"status": "deleted", "path": filePath})
|
||||
if h.wh != nil {
|
||||
go h.wh.RestartByID(workspaceID)
|
||||
// RFC internal#524 Layer 1: per-handler goAsync (drains via h.wh.waitAsyncForTest)
|
||||
wsID := workspaceID
|
||||
h.wh.goAsync(func() { h.wh.RestartByID(wsID) })
|
||||
}
|
||||
return
|
||||
}
|
||||
@@ -669,7 +677,9 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) {
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"status": "deleted", "path": filePath})
|
||||
if h.wh != nil {
|
||||
go h.wh.RestartByID(workspaceID)
|
||||
// RFC internal#524 Layer 1: per-handler goAsync (drains via h.wh.waitAsyncForTest)
|
||||
wsID := workspaceID
|
||||
h.wh.goAsync(func() { h.wh.RestartByID(wsID) })
|
||||
}
|
||||
return
|
||||
}
|
||||
@@ -682,6 +692,8 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) {
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"status": "deleted", "path": filePath})
|
||||
if h.wh != nil {
|
||||
go h.wh.RestartByID(workspaceID)
|
||||
// RFC internal#524 Layer 1: per-handler goAsync (drains via h.wh.waitAsyncForTest)
|
||||
wsID := workspaceID
|
||||
h.wh.goAsync(func() { h.wh.RestartByID(wsID) })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -211,6 +211,10 @@ func (h *TerminalHandler) handleLocalConnect(c *gin.Context, workspaceID string)
|
||||
}
|
||||
|
||||
// Bridge: container stdout → WebSocket
|
||||
// goAsync-exempt (RFC internal#524 Layer 2.2): per-WebSocket I/O
|
||||
// bridge — lifetime is the connection, not a request. The handler
|
||||
// blocks on `done` below, so the goroutine is already drained
|
||||
// synchronously. No db.DB access on this path.
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
defer close(done)
|
||||
@@ -433,6 +437,8 @@ func (h *TerminalHandler) handleRemoteConnect(c *gin.Context, workspaceID, insta
|
||||
done := make(chan struct{})
|
||||
|
||||
// PTY → WebSocket
|
||||
// goAsync-exempt (RFC internal#524 Layer 2.2): WebSocket-lifetime
|
||||
// I/O bridge; handler blocks on `done` below. No db.DB access.
|
||||
go func() {
|
||||
defer close(done)
|
||||
buf := make([]byte, 4096)
|
||||
@@ -455,6 +461,7 @@ func (h *TerminalHandler) handleRemoteConnect(c *gin.Context, workspaceID, insta
|
||||
}()
|
||||
|
||||
// WebSocket → PTY (stdin)
|
||||
// goAsync-exempt (RFC internal#524 Layer 2.2): see above.
|
||||
go func() {
|
||||
for {
|
||||
_, msg, rErr := conn.ReadMessage()
|
||||
|
||||
@@ -101,6 +101,47 @@ func (h *WorkspaceHandler) waitAsyncForTest() {
|
||||
h.asyncWG.Wait()
|
||||
}
|
||||
|
||||
// globalAsync tracks goroutines launched by globalGoAsync — the
|
||||
// equivalent of WorkspaceHandler.goAsync for sibling handlers that
|
||||
// don't carry a *WorkspaceHandler reference (SecretsHandler /
|
||||
// PluginsHandler / AdminPluginDriftHandler / ChannelHandler /
|
||||
// MCPHandler / RegistryHandler), and for callers of package-level
|
||||
// free functions (a2a_proxy_helpers extractAndUpsertTokenUsage).
|
||||
//
|
||||
// Forward-port of RFC internal#524 Layer 1 deliverable 2: the
|
||||
// canonical db.DB race fix lives at workspace.go:goAsync / asyncWG,
|
||||
// but ~25 sibling bare-`go` sites still write to db.DB outside any
|
||||
// WorkspaceHandler's drain window. globalAsync gives them the same
|
||||
// drain hook (waitGlobalAsyncForTest, drained from drainTestAsync)
|
||||
// so a test's t.Cleanup db.DB restore cannot race a detached
|
||||
// goroutine spawned by any sibling handler.
|
||||
//
|
||||
// Zero-cost in production (a single sync.WaitGroup Add/Done per
|
||||
// fire-and-forget call, no test-only branching).
|
||||
var globalAsync sync.WaitGroup
|
||||
|
||||
// globalGoAsync schedules fn on a detached goroutine tracked by
|
||||
// globalAsync. Use this in package-internal callers that don't have
|
||||
// a *WorkspaceHandler receiver to thread h.goAsync through.
|
||||
//
|
||||
// When a *WorkspaceHandler IS available, prefer h.goAsync — it lets
|
||||
// per-handler tests (waitAsyncForTest) wait without disturbing
|
||||
// unrelated handlers' inflight work.
|
||||
func globalGoAsync(fn func()) {
|
||||
globalAsync.Add(1)
|
||||
go func() {
|
||||
defer globalAsync.Done()
|
||||
fn()
|
||||
}()
|
||||
}
|
||||
|
||||
// waitGlobalAsyncForTest blocks until every globalGoAsync goroutine
|
||||
// finishes. Called from drainTestAsync's cleanup chain in the test
|
||||
// harness; production code never calls it.
|
||||
func waitGlobalAsyncForTest() {
|
||||
globalAsync.Wait()
|
||||
}
|
||||
|
||||
func NewWorkspaceHandler(b events.EventEmitter, p *provisioner.Provisioner, platformURL, configsDir string) *WorkspaceHandler {
|
||||
h := &WorkspaceHandler{
|
||||
broadcaster: b,
|
||||
|
||||
@@ -786,51 +786,57 @@ func applyRuntimeModelEnv(envVars map[string]string, runtime, model string) {
|
||||
// Resolution order (priority high → low):
|
||||
// 1. payload.Model (caller passed the canvas-picked model id verbatim)
|
||||
// 2. envVars["MOLECULE_MODEL"] (the canonical, unambiguous name)
|
||||
// 3. envVars["MODEL"] (workspace_secret persisted by /org/import via
|
||||
// the persona env file — MODEL=MiniMax-M2.7-highspeed etc.)
|
||||
// 4. envVars["MODEL_PROVIDER"] (legacy + misleadingly named: it carries
|
||||
// a *model id*, never the provider — that's LLM_PROVIDER. Historically
|
||||
// set by canvas Save+Restart's PUT /model; the post-2026-05-08
|
||||
// persona-env convention sometimes (mis)set it to a provider slug
|
||||
// ("minimax") or a runtime name ("claude-code"), neither a valid
|
||||
// model id — see internal#226. Only fires when the better-named
|
||||
// vars are absent.)
|
||||
// 3. envVars["MODEL"] (workspace_secret — written by SetModel /
|
||||
// WorkspaceHandler.Create / persona env file; the only correct
|
||||
// home for a picked model id).
|
||||
//
|
||||
// Pre-fix bug: this function unconditionally OVERWROTE envVars["MODEL"]
|
||||
// with the MODEL_PROVIDER slug (when payload.Model was empty), wiping
|
||||
// the operator's explicit per-persona MODEL secret on every restart.
|
||||
// Symptom: a workspace whose persona env said
|
||||
// MODEL=MiniMax-M2.7-highspeed booted fine on first /org/import (the
|
||||
// envVars map was populated direct from the env file), then on the
|
||||
// next Restart the workspace_secrets-derived MODEL got clobbered by
|
||||
// MODEL_PROVIDER="minimax" — the literal slug, not a valid model id —
|
||||
// and the workspace template's adapter routed to providers[0]
|
||||
// (anthropic-oauth) and wedged at SDK initialize. Caught 2026-05-08
|
||||
// during Phase 4 verification of template-claude-code PR #9.
|
||||
// Pre-fix bug (2026-05-08): this function used to consult
|
||||
// envVars["MODEL_PROVIDER"] as a fourth fallback AND unconditionally
|
||||
// overwrite envVars["MODEL"] with that slug when payload.Model was
|
||||
// empty. The MODEL_PROVIDER key was misleadingly named — it carried
|
||||
// a model id, never a provider — and the persona-env convention
|
||||
// sometimes (mis)set it to a provider slug ("minimax") or a runtime
|
||||
// name ("claude-code"), neither a valid model id. Symptom: a
|
||||
// workspace whose persona env said MODEL=MiniMax-M2.7-highspeed
|
||||
// booted fine on first /org/import, then on the next Restart the
|
||||
// workspace_secrets-derived MODEL got clobbered by
|
||||
// MODEL_PROVIDER="minimax" — the literal slug, not a valid model
|
||||
// id — and the workspace template's adapter routed to providers[0]
|
||||
// (anthropic-oauth) and wedged at SDK initialize.
|
||||
//
|
||||
// The 2026-05-19 follow-up fix (this commit) renamed the
|
||||
// workspace_secrets row MODEL_PROVIDER → MODEL (root cause: the
|
||||
// misleading column name; see secrets.go + the
|
||||
// 20260519000000_workspace_secrets_model_provider_rename migration)
|
||||
// and drops the MODEL_PROVIDER fallback here so the fallback chain
|
||||
// can no longer confuse a provider slug for a model id. CP-side
|
||||
// slot-separation (cp#213 + cp#220) merged the analogous fix on
|
||||
// the CP side; this is the workspace-server companion.
|
||||
if model == "" {
|
||||
model = envVars["MOLECULE_MODEL"]
|
||||
}
|
||||
if model == "" {
|
||||
model = envVars["MODEL"]
|
||||
}
|
||||
if model == "" {
|
||||
model = envVars["MODEL_PROVIDER"]
|
||||
}
|
||||
if model == "" {
|
||||
return
|
||||
}
|
||||
|
||||
// Canonical model env vars — molecule-runtime's workspace/config.py
|
||||
// resolves the picked model as MOLECULE_MODEL > MODEL > (legacy)
|
||||
// MODEL_PROVIDER (#280). Export both new names so adapters can read
|
||||
// either; MODEL stays for backwards compat with everything that
|
||||
// already reads os.environ["MODEL"] (the claude-code adapter does,
|
||||
// since #194). Without this, the user's canvas selection is silently
|
||||
// dropped on every templated provision — confirmed via crash-loop
|
||||
// diagnosis on 2026-05-02 where MiniMax picks booted with model=sonnet
|
||||
// (template default) and demanded CLAUDE_CODE_OAUTH_TOKEN. Set these
|
||||
// FIRST so the per-runtime branches below can layer on additional
|
||||
// vendor-specific names without fighting over the canonical one.
|
||||
// MODEL_PROVIDER (#280; the legacy env-var fallback in the Python
|
||||
// runtime is independent of the workspace_secrets row rename — it
|
||||
// still reads the env var for back-compat with already-running
|
||||
// images, but workspace-server no longer emits it). Export both new
|
||||
// names so adapters can read either; MODEL stays for backwards
|
||||
// compat with everything that already reads os.environ["MODEL"]
|
||||
// (the claude-code adapter does, since #194). Without this, the
|
||||
// user's canvas selection is silently dropped on every templated
|
||||
// provision — confirmed via crash-loop diagnosis on 2026-05-02
|
||||
// where MiniMax picks booted with model=sonnet (template default)
|
||||
// and demanded CLAUDE_CODE_OAUTH_TOKEN. Set these FIRST so the
|
||||
// per-runtime branches below can layer on additional vendor-
|
||||
// specific names without fighting over the canonical one.
|
||||
envVars["MOLECULE_MODEL"] = model
|
||||
envVars["MODEL"] = model
|
||||
|
||||
|
||||
@@ -675,15 +675,22 @@ func TestDeriveProviderFromModelSlug(t *testing.T) {
|
||||
// TestWorkspaceCreate_FirstDeploy_PersistsModelAndProvider pins the
|
||||
// fix for failed-workspace 95ed3ff2 (2026-05-02). Pre-fix: the canvas
|
||||
// POSTed minimax/MiniMax-M2.7 in payload.Model, the workspace row was
|
||||
// created, but neither MODEL_PROVIDER nor LLM_PROVIDER was ever
|
||||
// created, but neither the model nor the derived provider was ever
|
||||
// written to workspace_secrets. On any subsequent restart, the
|
||||
// applyRuntimeModelEnv fallback found nothing in envVars["MODEL_PROVIDER"]
|
||||
// and hermes booted with the template default (nousresearch/hermes-4-70b)
|
||||
// → wrong provider keys → /health poll failed → never registered.
|
||||
// applyRuntimeModelEnv fallback found nothing and hermes booted with
|
||||
// the template default (nousresearch/hermes-4-70b) → wrong provider
|
||||
// keys → /health poll failed → never registered.
|
||||
//
|
||||
// Post-fix: the create handler writes both rows after committing the
|
||||
// workspace row. This test asserts the SQL writes happen with the
|
||||
// correct keys + values.
|
||||
//
|
||||
// 2026-05-19 follow-up: the workspace_secrets row that holds the
|
||||
// picked model id was renamed MODEL_PROVIDER → MODEL (the column name
|
||||
// was misleading and bled into applyRuntimeModelEnv as a slug
|
||||
// fallback). The sqlmock regex below now anchors on 'MODEL' instead
|
||||
// of 'MODEL_PROVIDER'. See fix/workspace-server-rename-
|
||||
// MODEL_PROVIDER-to-MODEL + the 20260519000000 rename migration.
|
||||
func TestWorkspaceCreate_FirstDeploy_PersistsModelAndProvider(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
@@ -699,13 +706,16 @@ func TestWorkspaceCreate_FirstDeploy_PersistsModelAndProvider(t *testing.T) {
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectCommit()
|
||||
|
||||
// The fix: MODEL_PROVIDER is upserted with the verbatim model slug.
|
||||
// SQL has 3 placeholders ($1=workspace_id, $2=encrypted_value reused
|
||||
// in the conflict-update, $3=version reused in the conflict-update),
|
||||
// so sqlmock sees 3 args. The 'MODEL_PROVIDER' / 'LLM_PROVIDER' key
|
||||
// is a literal in the SQL — we distinguish the two writes with the
|
||||
// regex match below.
|
||||
mock.ExpectExec(`INSERT INTO workspace_secrets[\s\S]*'MODEL_PROVIDER'`).
|
||||
// The fix: MODEL is upserted with the verbatim model slug
|
||||
// (renamed from MODEL_PROVIDER on 2026-05-19 — see file-level
|
||||
// docstring). SQL has 3 placeholders ($1=workspace_id, $2=
|
||||
// encrypted_value reused in the conflict-update, $3=version
|
||||
// reused in the conflict-update), so sqlmock sees 3 args. The
|
||||
// 'MODEL' / 'LLM_PROVIDER' key is a literal in the SQL — we
|
||||
// distinguish the two writes with the regex match below. The
|
||||
// 'MODEL' anchor uses a word boundary (`[^_A-Z]`) so it does
|
||||
// NOT silently match the legacy 'MODEL_PROVIDER' name.
|
||||
mock.ExpectExec(`INSERT INTO workspace_secrets[\s\S]*'MODEL'`).
|
||||
WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
// The fix: LLM_PROVIDER is upserted with the derived provider name.
|
||||
@@ -742,13 +752,13 @@ func TestWorkspaceCreate_FirstDeploy_PersistsModelAndProvider(t *testing.T) {
|
||||
t.Fatalf("expected status 201, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("sqlmock expectations not met — first-deploy did NOT persist MODEL_PROVIDER + LLM_PROVIDER (this is the prod bug recurrence): %v", err)
|
||||
t.Errorf("sqlmock expectations not met — first-deploy did NOT persist MODEL + LLM_PROVIDER (this is the prod bug recurrence): %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestWorkspaceCreate_FirstDeploy_NoModel_NoSecretWritten asserts that
|
||||
// when payload.Model is empty, NEITHER MODEL_PROVIDER nor LLM_PROVIDER
|
||||
// is written. Important: the canvas can omit `model` (template inherits
|
||||
// when payload.Model is empty, NEITHER MODEL nor LLM_PROVIDER is
|
||||
// written. Important: the canvas can omit `model` (template inherits
|
||||
// the runtime default later); we must not poison workspace_secrets with
|
||||
// empty rows in that case.
|
||||
func TestWorkspaceCreate_FirstDeploy_NoModel_NoSecretWritten(t *testing.T) {
|
||||
@@ -792,10 +802,11 @@ func TestWorkspaceCreate_FirstDeploy_NoModel_NoSecretWritten(t *testing.T) {
|
||||
|
||||
// TestWorkspaceCreate_FirstDeploy_UnknownModel_OnlyMintModelProvider
|
||||
// asserts the asymmetric case: an unknown model prefix still gets
|
||||
// MODEL_PROVIDER persisted (so the user's exact slug survives restart
|
||||
// and applyRuntimeModelEnv finds it), but LLM_PROVIDER is skipped (so
|
||||
// MODEL persisted (so the user's exact slug survives restart and
|
||||
// applyRuntimeModelEnv finds it), but LLM_PROVIDER is skipped (so
|
||||
// derive-provider.sh's *=auto branch can decide at runtime instead of
|
||||
// being pre-empted by a guess).
|
||||
// being pre-empted by a guess). The MODEL key was renamed from
|
||||
// MODEL_PROVIDER on 2026-05-19 — see file-level docstring.
|
||||
func TestWorkspaceCreate_FirstDeploy_UnknownModel_OnlyMintModelProvider(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
@@ -807,9 +818,9 @@ func TestWorkspaceCreate_FirstDeploy_UnknownModel_OnlyMintModelProvider(t *testi
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectCommit()
|
||||
|
||||
// Only MODEL_PROVIDER — LLM_PROVIDER must NOT be written for
|
||||
// unknown prefixes. Same 3-arg shape as above; key is literal in SQL.
|
||||
mock.ExpectExec(`INSERT INTO workspace_secrets[\s\S]*'MODEL_PROVIDER'`).
|
||||
// Only MODEL — LLM_PROVIDER must NOT be written for unknown
|
||||
// prefixes. Same 3-arg shape as above; key is literal in SQL.
|
||||
mock.ExpectExec(`INSERT INTO workspace_secrets[\s\S]*'MODEL'`).
|
||||
WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
@@ -836,7 +847,7 @@ func TestWorkspaceCreate_FirstDeploy_UnknownModel_OnlyMintModelProvider(t *testi
|
||||
t.Fatalf("expected status 201, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("sqlmock expectations not met — unknown-prefix model should mint MODEL_PROVIDER but skip LLM_PROVIDER: %v", err)
|
||||
t.Errorf("sqlmock expectations not met — unknown-prefix model should mint MODEL but skip LLM_PROVIDER: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -897,11 +908,11 @@ func TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes(t *testing.T) {
|
||||
model: "",
|
||||
},
|
||||
{
|
||||
name: "empty model + MODEL_PROVIDER fallback hits: MODEL/MOLECULE_MODEL set from secret",
|
||||
name: "empty model + MODEL_PROVIDER env IGNORED post-2026-05-19 rename (the slug-fallback bug)",
|
||||
runtime: "claude-code",
|
||||
model: "",
|
||||
modelProviderEnv: "MiniMax-M2",
|
||||
wantMODEL: "MiniMax-M2",
|
||||
wantMODEL: "",
|
||||
},
|
||||
{
|
||||
name: "empty model + MOLECULE_MODEL env fallback hits (canonical name)",
|
||||
@@ -911,7 +922,7 @@ func TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes(t *testing.T) {
|
||||
wantMODEL: "opus",
|
||||
},
|
||||
{
|
||||
name: "MOLECULE_MODEL beats MODEL_PROVIDER when both set (misnomer guard, internal#226)",
|
||||
name: "MOLECULE_MODEL wins even when stale MODEL_PROVIDER is present (back-compat guard)",
|
||||
runtime: "claude-code",
|
||||
model: "",
|
||||
moleculeModelEnv: "opus",
|
||||
@@ -947,18 +958,26 @@ func TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes(t *testing.T) {
|
||||
|
||||
// TestApplyRuntimeModelEnv_PersonaEnvMODELSecretPreserved locks in the
|
||||
// 2026-05-08 fix that prevents the MODEL_PROVIDER-as-slug fallback from
|
||||
// silently overwriting a per-persona MODEL workspace_secret on restart.
|
||||
// silently overwriting a per-persona MODEL workspace_secret on restart,
|
||||
// EXTENDED for the 2026-05-19 root-cause fix that drops the
|
||||
// MODEL_PROVIDER fallback entirely.
|
||||
//
|
||||
// Pre-fix bug recurrence guard: when the persona env file (loaded into
|
||||
// workspace_secrets at /org/import time) declares both MODEL=<id> and
|
||||
// MODEL_PROVIDER=<slug>, the restart path used to overwrite envVars["MODEL"]
|
||||
// with the MODEL_PROVIDER slug because applyRuntimeModelEnv'\''s
|
||||
// with the MODEL_PROVIDER slug because applyRuntimeModelEnv's
|
||||
// payload.Model fallback consulted MODEL_PROVIDER first. Symptom: dev-tree
|
||||
// workspaces booted fine on first /org/import, then on next restart the
|
||||
// model id became literal "minimax" and the workspace template'\''s adapter
|
||||
// model id became literal "minimax" and the workspace template's adapter
|
||||
// failed to match any registry prefix, fell through to anthropic-oauth,
|
||||
// and wedged at SDK initialize. Caught during Phase 4 verification of
|
||||
// template-claude-code PR #9.
|
||||
//
|
||||
// 2026-05-19 follow-up: the MODEL_PROVIDER fallback is now removed.
|
||||
// MODEL is the only env-var source for the picked model id.
|
||||
// MODEL_PROVIDER is intentionally NOT consulted — a stale MODEL_PROVIDER
|
||||
// row left over from before the 20260519000000 migration must NOT leak
|
||||
// into envVars["MODEL"]. Verified by the third case below.
|
||||
func TestApplyRuntimeModelEnv_PersonaEnvMODELSecretPreserved(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
@@ -967,7 +986,7 @@ func TestApplyRuntimeModelEnv_PersonaEnvMODELSecretPreserved(t *testing.T) {
|
||||
wantMODEL string
|
||||
}{
|
||||
{
|
||||
name: "MODEL secret wins over MODEL_PROVIDER slug (persona-env shape on restart)",
|
||||
name: "MODEL secret wins; stale MODEL_PROVIDER ignored (persona-env shape on restart)",
|
||||
envMODEL: "MiniMax-M2.7-highspeed",
|
||||
envMP: "minimax",
|
||||
wantMODEL: "MiniMax-M2.7-highspeed",
|
||||
@@ -979,10 +998,10 @@ func TestApplyRuntimeModelEnv_PersonaEnvMODELSecretPreserved(t *testing.T) {
|
||||
wantMODEL: "opus",
|
||||
},
|
||||
{
|
||||
name: "MODEL absent → fall back to MODEL_PROVIDER (legacy canvas Save+Restart shape)",
|
||||
name: "MODEL absent → MODEL_PROVIDER no longer fallback (2026-05-19 fix): nothing set",
|
||||
envMODEL: "",
|
||||
envMP: "MiniMax-M2.7",
|
||||
wantMODEL: "MiniMax-M2.7",
|
||||
wantMODEL: "",
|
||||
},
|
||||
{
|
||||
name: "Both absent → no MODEL set",
|
||||
@@ -1009,3 +1028,48 @@ func TestApplyRuntimeModelEnv_PersonaEnvMODELSecretPreserved(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestApplyRuntimeModelEnv_StaleMODELPROVIDERNeverLeaksIntoMODEL is the
|
||||
// 2026-05-19 root-cause pin: workspaces that were live BEFORE the
|
||||
// 20260519000000_workspace_secrets_model_provider_rename migration ran
|
||||
// may still have a MODEL_PROVIDER row in workspace_secrets that lands
|
||||
// in envVars (the loader doesn't filter — anything in workspace_secrets
|
||||
// gets passed through). Post-fix, applyRuntimeModelEnv MUST NOT consult
|
||||
// that key for any purpose — neither as a fallback for the picked model
|
||||
// id nor as an indirect overwrite of MODEL. Asserts the read-out shape:
|
||||
//
|
||||
// - envVars["MODEL"] stays empty when no other source provided one
|
||||
// - envVars["MOLECULE_MODEL"] stays empty
|
||||
// - envVars["HERMES_DEFAULT_MODEL"] stays empty
|
||||
// - envVars["MODEL_PROVIDER"] itself is left as-is (we don't actively
|
||||
// scrub it — the rename migration does that on the DB side)
|
||||
//
|
||||
// Pairs with workspace_provision.go applyRuntimeModelEnv (line 817
|
||||
// fallback removed) and secrets.go (workspace_secrets key MODEL).
|
||||
func TestApplyRuntimeModelEnv_StaleMODELPROVIDERNeverLeaksIntoMODEL(t *testing.T) {
|
||||
envVars := map[string]string{
|
||||
"MODEL_PROVIDER": "minimax", // legacy slug — the prod-bug shape
|
||||
}
|
||||
applyRuntimeModelEnv(envVars, "claude-code", "")
|
||||
if got, ok := envVars["MODEL"]; ok {
|
||||
t.Errorf("MODEL must not be set from MODEL_PROVIDER fallback (post-2026-05-19 fix); got=%q", got)
|
||||
}
|
||||
if got, ok := envVars["MOLECULE_MODEL"]; ok {
|
||||
t.Errorf("MOLECULE_MODEL must not be set from MODEL_PROVIDER fallback; got=%q", got)
|
||||
}
|
||||
if got, ok := envVars["HERMES_DEFAULT_MODEL"]; ok {
|
||||
t.Errorf("HERMES_DEFAULT_MODEL must not be set from MODEL_PROVIDER fallback; got=%q", got)
|
||||
}
|
||||
if got := envVars["MODEL_PROVIDER"]; got != "minimax" {
|
||||
t.Errorf("MODEL_PROVIDER must be passed through untouched (DB-side rename handles cleanup); got=%q", got)
|
||||
}
|
||||
|
||||
// Hermes-runtime variant — same shape, same expectation.
|
||||
envVarsH := map[string]string{
|
||||
"MODEL_PROVIDER": "minimax",
|
||||
}
|
||||
applyRuntimeModelEnv(envVarsH, "hermes", "")
|
||||
if _, ok := envVarsH["HERMES_DEFAULT_MODEL"]; ok {
|
||||
t.Errorf("hermes runtime must not leak MODEL_PROVIDER into HERMES_DEFAULT_MODEL")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
-- Reverse of 20260518000000_seed_production_team_agent_cards.up.sql.
|
||||
--
|
||||
-- Clears the identity fields back to the gap state that the up
|
||||
-- migration was designed to fix. After this down migration, the PR
|
||||
-- #1427 reconcile has nothing to substitute again: name reverts to the
|
||||
-- workspace UUID (the runtime's fallback), role to NULL, agent_card
|
||||
-- description/skills to empty. This is the pre-#1427 + pre-this-seed
|
||||
-- behaviour.
|
||||
--
|
||||
-- Match strategy mirrors the up migration (id::text LIKE prefix for 5,
|
||||
-- exact UUID for CEO-Assistant) so any down-roll touches the exact
|
||||
-- same rows.
|
||||
|
||||
BEGIN;
|
||||
|
||||
-- PM
|
||||
UPDATE workspaces
|
||||
SET name = id::text,
|
||||
role = NULL,
|
||||
agent_card = (agent_card - 'description' - 'skills' - 'role') ||
|
||||
jsonb_build_object('name', id::text),
|
||||
updated_at = now()
|
||||
WHERE id::text LIKE '8a71d4d4-%';
|
||||
|
||||
-- Reviewer
|
||||
UPDATE workspaces
|
||||
SET name = id::text,
|
||||
role = NULL,
|
||||
agent_card = (agent_card - 'description' - 'skills' - 'role') ||
|
||||
jsonb_build_object('name', id::text),
|
||||
updated_at = now()
|
||||
WHERE id::text LIKE '27e66b5a-%';
|
||||
|
||||
-- Researcher
|
||||
UPDATE workspaces
|
||||
SET name = id::text,
|
||||
role = NULL,
|
||||
agent_card = (agent_card - 'description' - 'skills' - 'role') ||
|
||||
jsonb_build_object('name', id::text),
|
||||
updated_at = now()
|
||||
WHERE id::text LIKE '5773bd5f-%';
|
||||
|
||||
-- Dev-A
|
||||
UPDATE workspaces
|
||||
SET name = id::text,
|
||||
role = NULL,
|
||||
agent_card = (agent_card - 'description' - 'skills' - 'role') ||
|
||||
jsonb_build_object('name', id::text),
|
||||
updated_at = now()
|
||||
WHERE id::text LIKE '4ca4c06c-%';
|
||||
|
||||
-- Dev-B
|
||||
UPDATE workspaces
|
||||
SET name = id::text,
|
||||
role = NULL,
|
||||
agent_card = (agent_card - 'description' - 'skills' - 'role') ||
|
||||
jsonb_build_object('name', id::text),
|
||||
updated_at = now()
|
||||
WHERE id::text LIKE '31eb65ed-%';
|
||||
|
||||
-- CEO-Assistant
|
||||
UPDATE workspaces
|
||||
SET name = id::text,
|
||||
role = NULL,
|
||||
agent_card = (agent_card - 'description' - 'skills' - 'role') ||
|
||||
jsonb_build_object('name', id::text),
|
||||
updated_at = now()
|
||||
WHERE id = '30ba7f0b-b303-4a20-aefe-3a4a675b8aa4'::uuid;
|
||||
|
||||
COMMIT;
|
||||
@@ -0,0 +1,165 @@
|
||||
-- Seed identity (name + role + agent_card description/skills) for the
|
||||
-- 6 production-team workspaces. Pairs with the PR #1427 server-side
|
||||
-- reconcile (internal#492): #1427 added the platform-side backfill that
|
||||
-- pulls workspaces.name and workspaces.role into the stored agent_card
|
||||
-- on /registry/register; this migration populates the trusted DB row
|
||||
-- those reads consume.
|
||||
--
|
||||
-- Without this seed, the reconcile has nothing to substitute and the
|
||||
-- card stays at name=UUID / description="" / role=null for the prod
|
||||
-- team agents — the exact gap internal#492 is filed against.
|
||||
--
|
||||
-- Identity stays platform-controlled — the agent runtime cannot
|
||||
-- self-write these fields. The 6 workspace UUIDs are the CTO-locked
|
||||
-- production-team topology (see project_production_agent_team_topology):
|
||||
--
|
||||
-- PM 8a71d4d4... — Claude Code on Opus, read-only,
|
||||
-- A2A-delegate-only coordinator
|
||||
-- Reviewer 27e66b5a... — codex on openai-subscription,
|
||||
-- 5-axis non-author review
|
||||
-- Researcher 5773bd5f... — codex on openai-subscription,
|
||||
-- root-cause investigation
|
||||
-- Dev-A 4ca4c06c... — Claude Code on Kimi K2.6
|
||||
-- (api.kimi.com/coding base + ANTHROPIC_API_KEY)
|
||||
-- Dev-B 31eb65ed... — Claude Code on MiniMax
|
||||
-- (api.minimax.io/anthropic base + sk-cp-* key)
|
||||
-- CEO-Assistant 30ba7f0b-b303-4a20-aefe-3a4a675b8aa4 — Claude Code,
|
||||
-- orchestrator-side operations + canvas relay
|
||||
--
|
||||
-- Match strategy: 5 of 6 production UUIDs were provided to me by the CTO
|
||||
-- as 8-char prefixes only (the full UUIDs live in the prod tenant DB).
|
||||
-- We match those 5 with `id::text LIKE '<prefix>-%'` so this migration
|
||||
-- is unambiguous when reviewed without DB access — the CTO will confirm
|
||||
-- on review that each prefix resolves to a single row. CEO-Assistant
|
||||
-- (30ba7f0b-b303-4a20-aefe-3a4a675b8aa4) is known in full from
|
||||
-- chat_files_test.go and is matched exactly.
|
||||
--
|
||||
-- Idempotent: each UPDATE only touches the three identity fields. Re-
|
||||
-- running rewrites the same values. UUIDs not present in a given tenant
|
||||
-- DB match zero rows and are silently skipped — the migration never
|
||||
-- INSERTs rows it doesn't own.
|
||||
--
|
||||
-- All names obey validateWorkspaceFields (workspace_crud.go:526):
|
||||
-- <=255 chars, no newline/CR, no YAML-special chars `{}[]|>*&!`.
|
||||
-- All roles obey the same contract <=1000 chars. Per-skill description
|
||||
-- <=120 chars matches the discovery card surface shown on the canvas
|
||||
-- Agent Card view and the mobile peer chip.
|
||||
|
||||
BEGIN;
|
||||
|
||||
-- PM — read-only A2A coordinator
|
||||
UPDATE workspaces
|
||||
SET name = 'Production Manager',
|
||||
role = 'product manager',
|
||||
agent_card = COALESCE(agent_card, '{}'::jsonb) || jsonb_build_object(
|
||||
'name', 'Production Manager',
|
||||
'description', 'Read-only A2A coordinator that plans work and delegates to Dev/Reviewer/Researcher peers; never writes code itself.',
|
||||
'role', 'product manager',
|
||||
'skills', jsonb_build_array(
|
||||
jsonb_build_object('id','planning','name','planning','description','Decompose CTO directives into peer-delegable units','tags',jsonb_build_array('planning'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','delegation','name','delegation','description','Route work to Dev-A / Dev-B / Reviewer / Researcher via A2A','tags',jsonb_build_array('delegation'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','coordination','name','coordination','description','Track peer activity and surface blockers back to the CTO','tags',jsonb_build_array('coordination'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','read-only','name','read-only','description','Never edits code or merges; proposes only','tags',jsonb_build_array('read-only','safety'),'examples',jsonb_build_array())
|
||||
),
|
||||
'updated_at', now()::text
|
||||
),
|
||||
updated_at = now()
|
||||
WHERE id::text LIKE '8a71d4d4-%';
|
||||
|
||||
-- Reviewer — codex/openai, 5-axis non-author review
|
||||
UPDATE workspaces
|
||||
SET name = 'Code Reviewer',
|
||||
role = 'code reviewer',
|
||||
agent_card = COALESCE(agent_card, '{}'::jsonb) || jsonb_build_object(
|
||||
'name', 'Code Reviewer',
|
||||
'description', 'Non-author 5-axis review on codex/openai-subscription; runs the merge gate, never approves PRs it authored.',
|
||||
'role', 'code reviewer',
|
||||
'skills', jsonb_build_array(
|
||||
jsonb_build_object('id','code-review','name','code-review','description','Five-axis PR review against the merge gate','tags',jsonb_build_array('review'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','security-axis','name','security-axis','description','Trust-boundary, secret-handling, injection surface checks','tags',jsonb_build_array('security'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','correctness-axis','name','correctness-axis','description','Logic, error-handling, race and boundary case checks','tags',jsonb_build_array('correctness'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','non-author-approve','name','non-author-approve','description','Approves only PRs the reviewer did not author','tags',jsonb_build_array('two-eyes'),'examples',jsonb_build_array())
|
||||
),
|
||||
'updated_at', now()::text
|
||||
),
|
||||
updated_at = now()
|
||||
WHERE id::text LIKE '27e66b5a-%';
|
||||
|
||||
-- Researcher — codex/openai, root-cause investigation
|
||||
UPDATE workspaces
|
||||
SET name = 'Root-Cause Researcher',
|
||||
role = 'researcher',
|
||||
agent_card = COALESCE(agent_card, '{}'::jsonb) || jsonb_build_object(
|
||||
'name', 'Root-Cause Researcher',
|
||||
'description', 'Diagnostic investigation on codex/openai-subscription; obs-first, source-as-corroboration, no drive-by fixes.',
|
||||
'role', 'researcher',
|
||||
'skills', jsonb_build_array(
|
||||
jsonb_build_object('id','root-cause','name','root-cause','description','Diagnose the underlying cause, never patch symptoms','tags',jsonb_build_array('investigation'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','obs-first','name','obs-first','description','Grafana/Loki query before source-guessing','tags',jsonb_build_array('observability'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','log-correlation','name','log-correlation','description','Cross-service step= / Delegation uuid tracing','tags',jsonb_build_array('observability'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','source-archaeology','name','source-archaeology','description','Git blame and prior-art recall across repos','tags',jsonb_build_array('git'),'examples',jsonb_build_array())
|
||||
),
|
||||
'updated_at', now()::text
|
||||
),
|
||||
updated_at = now()
|
||||
WHERE id::text LIKE '5773bd5f-%';
|
||||
|
||||
-- Dev-A — Claude Code on Kimi K2.6
|
||||
UPDATE workspaces
|
||||
SET name = 'Dev Engineer A (Kimi)',
|
||||
role = 'dev engineer',
|
||||
agent_card = COALESCE(agent_card, '{}'::jsonb) || jsonb_build_object(
|
||||
'name', 'Dev Engineer A (Kimi)',
|
||||
'description', 'Claude Code routed to Kimi K2.6 via api.kimi.com/coding; implements PRs against the dev-tree protected branches.',
|
||||
'role', 'dev engineer',
|
||||
'skills', jsonb_build_array(
|
||||
jsonb_build_object('id','implementation','name','implementation','description','Write code to merge gate (tests, lint, types)','tags',jsonb_build_array('coding'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','test-driven','name','test-driven','description','Failing test first, then minimal fix','tags',jsonb_build_array('tdd'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','bug-fixing','name','bug-fixing','description','Root-caused bug fixes with regression test','tags',jsonb_build_array('debugging'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','refactoring','name','refactoring','description','In-scope, behavior-preserving refactors only','tags',jsonb_build_array('refactor'),'examples',jsonb_build_array())
|
||||
),
|
||||
'updated_at', now()::text
|
||||
),
|
||||
updated_at = now()
|
||||
WHERE id::text LIKE '4ca4c06c-%';
|
||||
|
||||
-- Dev-B — Claude Code on MiniMax
|
||||
UPDATE workspaces
|
||||
SET name = 'Dev Engineer B (MiniMax)',
|
||||
role = 'dev engineer',
|
||||
agent_card = COALESCE(agent_card, '{}'::jsonb) || jsonb_build_object(
|
||||
'name', 'Dev Engineer B (MiniMax)',
|
||||
'description', 'Claude Code routed to MiniMax via api.minimax.io/anthropic; parallel dev capacity to Dev-A on the same gate.',
|
||||
'role', 'dev engineer',
|
||||
'skills', jsonb_build_array(
|
||||
jsonb_build_object('id','implementation','name','implementation','description','Write code to merge gate (tests, lint, types)','tags',jsonb_build_array('coding'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','test-driven','name','test-driven','description','Failing test first, then minimal fix','tags',jsonb_build_array('tdd'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','bug-fixing','name','bug-fixing','description','Root-caused bug fixes with regression test','tags',jsonb_build_array('debugging'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','refactoring','name','refactoring','description','In-scope, behavior-preserving refactors only','tags',jsonb_build_array('refactor'),'examples',jsonb_build_array())
|
||||
),
|
||||
'updated_at', now()::text
|
||||
),
|
||||
updated_at = now()
|
||||
WHERE id::text LIKE '31eb65ed-%';
|
||||
|
||||
-- CEO-Assistant — Claude Code, orchestrator + canvas relay
|
||||
-- Full UUID known from chat_files_test.go:286 — match exactly.
|
||||
UPDATE workspaces
|
||||
SET name = 'CEO Assistant',
|
||||
role = 'operator orchestrator',
|
||||
agent_card = COALESCE(agent_card, '{}'::jsonb) || jsonb_build_object(
|
||||
'name', 'CEO Assistant',
|
||||
'description', 'Orchestrator-side Claude Code that runs the triage loop, relays canvas and Telegram, dispatches non-author reviewers.',
|
||||
'role', 'operator orchestrator',
|
||||
'skills', jsonb_build_array(
|
||||
jsonb_build_object('id','triage-loop','name','triage-loop','description','Run the CI/PR triage loop; fix-what-you-find','tags',jsonb_build_array('orchestration'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','review-routing','name','review-routing','description','Dispatch non-author reviewers via delegate_task','tags',jsonb_build_array('routing'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','canvas-relay','name','canvas-relay','description','Relay CTO canvas/Telegram messages to peers','tags',jsonb_build_array('relay'),'examples',jsonb_build_array()),
|
||||
jsonb_build_object('id','ops','name','ops','description','Direct hands-on ops on operator host and Neon','tags',jsonb_build_array('ops','direct-action'),'examples',jsonb_build_array())
|
||||
),
|
||||
'updated_at', now()::text
|
||||
),
|
||||
updated_at = now()
|
||||
WHERE id = '30ba7f0b-b303-4a20-aefe-3a4a675b8aa4'::uuid;
|
||||
|
||||
COMMIT;
|
||||
+21
@@ -0,0 +1,21 @@
|
||||
-- Reverse of 20260519000000_workspace_secrets_model_provider_rename.up.sql.
|
||||
--
|
||||
-- This rolls MODEL → MODEL_PROVIDER. Note: the up migration deleted any
|
||||
-- conflicting MODEL_PROVIDER rows when a MODEL row already existed, so
|
||||
-- this down migration is intentionally lossy in that direction — it
|
||||
-- cannot reconstruct rows the up migration discarded. Acceptable
|
||||
-- because:
|
||||
--
|
||||
-- 1. The discarded rows were duplicates with the same workspace_id;
|
||||
-- the surviving MODEL row carries the correct semantic value.
|
||||
-- 2. The application code post-rename never writes MODEL_PROVIDER, so
|
||||
-- any rollback after live traffic would produce duplicate-key
|
||||
-- conflicts on re-up anyway — discarding here is the only sane
|
||||
-- shape.
|
||||
--
|
||||
-- Provided for migration-tool symmetry; in practice the up direction is
|
||||
-- the canonical fix and rollback should not happen.
|
||||
|
||||
UPDATE workspace_secrets
|
||||
SET key = 'MODEL_PROVIDER', updated_at = now()
|
||||
WHERE key = 'MODEL';
|
||||
+36
@@ -0,0 +1,36 @@
|
||||
-- Rename workspace_secrets rows MODEL_PROVIDER → MODEL.
|
||||
--
|
||||
-- Root cause: the column-name MODEL_PROVIDER was misleading — it never
|
||||
-- held a provider slug, only a picked model id (e.g.
|
||||
-- "minimax/MiniMax-M2.7"). Application code (workspace-server
|
||||
-- applyRuntimeModelEnv) read MODEL_PROVIDER as a fallback that could
|
||||
-- overwrite a legitimate MODEL persona-env secret with whatever literal
|
||||
-- string lived in MODEL_PROVIDER — often a provider slug like "minimax"
|
||||
-- or a runtime name like "claude-code", neither of which is a valid
|
||||
-- model id. The wrong shape then propagated into CP user-data and the
|
||||
-- workspace adapter wedged at SDK initialize (see failed-workspace
|
||||
-- 95ed3ff2 2026-05-02 and the Researcher/Reviewer poisoning 2026-05-19).
|
||||
--
|
||||
-- Pairs with the secrets.go + workspace_provision.go rename in this
|
||||
-- PR (fix/workspace-server-rename-MODEL_PROVIDER-to-MODEL) and the
|
||||
-- CP-side slot-separation already landed in cp#213 + cp#220.
|
||||
--
|
||||
-- Conflict handling: a workspace_secrets row already keyed MODEL takes
|
||||
-- precedence (persona-env files commonly write MODEL=... directly), so
|
||||
-- the MODEL_PROVIDER row is deleted instead of overwriting MODEL. The
|
||||
-- WHERE NOT EXISTS guard makes the migration idempotent — re-running
|
||||
-- it on an already-renamed schema is a no-op.
|
||||
|
||||
UPDATE workspace_secrets
|
||||
SET key = 'MODEL', updated_at = now()
|
||||
WHERE key = 'MODEL_PROVIDER'
|
||||
AND NOT EXISTS (
|
||||
SELECT 1 FROM workspace_secrets ws2
|
||||
WHERE ws2.workspace_id = workspace_secrets.workspace_id
|
||||
AND ws2.key = 'MODEL'
|
||||
);
|
||||
|
||||
-- Drop any leftover MODEL_PROVIDER rows where a MODEL row already
|
||||
-- exists (MODEL wins — see above).
|
||||
DELETE FROM workspace_secrets
|
||||
WHERE key = 'MODEL_PROVIDER';
|
||||
@@ -0,0 +1 @@
|
||||
# trigger autobump for python-multipart pin (PDF P0 cure)
|
||||
@@ -27,8 +27,8 @@ Path safety:
|
||||
collisions astronomical, but defense-in-depth costs nothing).
|
||||
|
||||
Limits (matches the Go contract from chat_files.go):
|
||||
- 50 MB total request body
|
||||
- 25 MB per file
|
||||
- 100 MB total request body
|
||||
- 100 MB per file
|
||||
- filename truncated to 100 chars
|
||||
|
||||
Response shape:
|
||||
@@ -64,11 +64,20 @@ CHAT_UPLOAD_DIR = "/workspace/.molecule/chat-uploads"
|
||||
# Total-request body cap. multipart/form-data with multiple parts can
|
||||
# add ~100 bytes of framing per file; the cap is the bytes hitting the
|
||||
# socket, including framing.
|
||||
CHAT_UPLOAD_MAX_BYTES = 50 * 1024 * 1024 # 50 MB
|
||||
#
|
||||
# SERVER_MIRROR: keep aligned with workspace-server/internal/handlers/
|
||||
# chat_files.go chatUploadMaxBytes AND canvas/src/components/tabs/chat/
|
||||
# uploads.ts MAX_UPLOAD_BYTES. Three constants exist (platform Go +
|
||||
# workspace Python + canvas TS) because each layer must enforce or
|
||||
# pre-flight the cap on its own; an SSOT follow-up tracked in
|
||||
# molecule-ai/internal would expose the cap via GET /uploads/limits.
|
||||
CHAT_UPLOAD_MAX_BYTES = 100 * 1024 * 1024 # 100 MB
|
||||
|
||||
# Per-file cap. Keeping per-file under total lets a user attach, say,
|
||||
# a 5 MB PDF + 10 small screenshots in a single batch.
|
||||
CHAT_UPLOAD_MAX_FILE_BYTES = 25 * 1024 * 1024 # 25 MB
|
||||
# Per-file cap. Aligned with the total at 100 MB so a single legitimate
|
||||
# large file (e.g. a 70 MB PDF — reno-stars 2026-05-19 forensic
|
||||
# a99ab0a1) succeeds end-to-end; batched small attachments still fit
|
||||
# under the same ceiling.
|
||||
CHAT_UPLOAD_MAX_FILE_BYTES = 100 * 1024 * 1024 # 100 MB
|
||||
|
||||
# Conservative {alnum, dot, underscore, dash} character class — anything
|
||||
# outside gets rewritten so embedded paths, control chars, newlines,
|
||||
@@ -149,8 +158,28 @@ async def ingest_handler(request: Request) -> JSONResponse:
|
||||
try:
|
||||
form = await request.form(max_files=64, max_fields=32)
|
||||
except Exception as exc: # multipart parse error
|
||||
logger.warning("internal_chat_uploads: multipart parse failed: %s", exc)
|
||||
return JSONResponse({"error": "failed to parse multipart form"}, status_code=400)
|
||||
# Surface exc.class + str(exc) to the caller. Prior behavior returned
|
||||
# only the opaque {"error": "failed to parse multipart form"}, which
|
||||
# took ~25 min to root-cause in forensic a78762a0 (Hermes workspace
|
||||
# PDF upload, 2026-05-19) — the underlying cause was a MISSING
|
||||
# python-multipart dep, surfaced as an AssertionError from Starlette's
|
||||
# parser. Surfacing exception class + detail in the 400 body would
|
||||
# have cut that to ~10 min. Per feedback_surface_actionable_failure_
|
||||
# reason_to_user (CTO 2026-05-17): user-facing failures MUST tell the
|
||||
# user WHY. Top-level "error" key is preserved for backwards-compat
|
||||
# with existing canvas / alert rules.
|
||||
logger.warning(
|
||||
"internal_chat_uploads: multipart parse failed: %s: %s",
|
||||
type(exc).__name__, exc,
|
||||
)
|
||||
return JSONResponse(
|
||||
{
|
||||
"error": "failed to parse multipart form",
|
||||
"exception": type(exc).__name__,
|
||||
"detail": str(exc),
|
||||
},
|
||||
status_code=400,
|
||||
)
|
||||
|
||||
# Starlette's FormData allows multiple values per key — `files` may
|
||||
# appear multiple times for batched uploads. getlist returns them
|
||||
|
||||
@@ -210,7 +210,7 @@ def test_no_files_field_returns_400(client: TestClient):
|
||||
|
||||
def test_per_file_oversize_returns_413(client: TestClient, monkeypatch: pytest.MonkeyPatch):
|
||||
"""Per-file cap is enforced. Lower the cap for the test so we don't
|
||||
have to construct a real 25 MB body."""
|
||||
have to construct a real 100 MB body."""
|
||||
monkeypatch.setattr(internal_chat_uploads, "CHAT_UPLOAD_MAX_FILE_BYTES", 16)
|
||||
big = b"x" * 32 # > 16
|
||||
r = client.post(
|
||||
@@ -299,3 +299,46 @@ def test_symlink_at_target_is_refused(client: TestClient, chat_uploads_dir: Path
|
||||
assert r.status_code == 500, r.text
|
||||
# Sentinel content unchanged — the symlink wasn't followed.
|
||||
assert sentinel.read_bytes() == b"original"
|
||||
|
||||
|
||||
# Pins the diagnostic shape of the 400 returned when multipart parsing
|
||||
# fails. Prior to forensic a78762a0 (Hermes workspace PDF upload 2026-05-19),
|
||||
# the response was {"error": "failed to parse multipart form"} only — opaque
|
||||
# to the caller, requiring ~25 min of triage to root-cause a missing
|
||||
# python-multipart dep. Surfacing exception class + str(exc) makes the
|
||||
# failure self-diagnosing (would've shortened that to ~10 min). Per
|
||||
# feedback_surface_actionable_failure_reason_to_user (CTO 2026-05-17):
|
||||
# user-facing failures MUST tell the user WHY.
|
||||
def test_malformed_multipart_returns_exception_class_and_detail(
|
||||
client: TestClient,
|
||||
):
|
||||
"""Send a multipart-shaped body whose boundary in the header does
|
||||
NOT match the boundary in the body — Starlette's parser raises a
|
||||
MultiPartException, which our handler must surface as exception
|
||||
class + detail in the 400 JSON response.
|
||||
"""
|
||||
# Header claims boundary "outer" but body uses "different".
|
||||
bad_body = (
|
||||
b"--different\r\n"
|
||||
b'Content-Disposition: form-data; name="files"; filename="a.txt"\r\n'
|
||||
b"Content-Type: text/plain\r\n\r\n"
|
||||
b"hello\r\n"
|
||||
b"--different--\r\n"
|
||||
)
|
||||
r = client.post(
|
||||
"/internal/chat/uploads/ingest",
|
||||
data=bad_body,
|
||||
headers={
|
||||
"Authorization": "Bearer test-secret",
|
||||
"Content-Type": "multipart/form-data; boundary=outer",
|
||||
},
|
||||
)
|
||||
assert r.status_code == 400, r.text
|
||||
body = r.json()
|
||||
# Backwards-compatible top-level error keeps existing canvas /
|
||||
# alert rules matching.
|
||||
assert body.get("error") == "failed to parse multipart form"
|
||||
# New diagnostic fields — caller can now see the exception class +
|
||||
# detail without SSM access to the workspace stderr.
|
||||
assert "exception" in body and isinstance(body["exception"], str) and body["exception"]
|
||||
assert "detail" in body and isinstance(body["detail"], str)
|
||||
|
||||
Reference in New Issue
Block a user