Compare commits

..

No commits in common. "main" and "feat/plugin-version-subscription" have entirely different histories.

85 changed files with 450 additions and 4185 deletions

View File

@ -1,118 +0,0 @@
#!/usr/bin/env bash
# audit-force-merge — detect a §SOP-6 force-merge after PR close, emit
# `incident.force_merge` to stdout as structured JSON.
#
# Vector's docker_logs source picks up runner stdout; the JSON gets
# shipped to Loki on molecule-canonical-obs, indexable by event_type.
# Query example:
#
# {host="operator"} |= "event_type" |= "incident.force_merge" | json
#
# A force-merge is detected when a PR closed-with-merged=true had at
# least one of the repo's required-status-check contexts in a state
# other than "success" at the merge commit's SHA. That's exactly what
# the Gitea force_merge:true API call lets through, so it's a faithful
# detector of the override path.
#
# Triggers on `pull_request_target: closed` (loaded from base branch
# per §SOP-6 security model). No-op when merged=false.
#
# Required env (set by the workflow):
# GITEA_TOKEN, GITEA_HOST, REPO, PR_NUMBER, REQUIRED_CHECKS
#
# REQUIRED_CHECKS is a newline-separated list of status-check context
# names that branch protection requires. Declared in the workflow YAML
# rather than fetched from /branch_protections (which needs admin
# scope — sop-tier-bot has read-only). Trade dynamism for simplicity:
# when the required-check set changes, update both branch protection
# AND this env. Keeping them in sync is less complexity than granting
# the audit bot admin perms on every repo.
set -euo pipefail
: "${GITEA_TOKEN:?required}"
: "${GITEA_HOST:?required}"
: "${REPO:?required}"
: "${PR_NUMBER:?required}"
: "${REQUIRED_CHECKS:?required (newline-separated context names)}"
OWNER="${REPO%%/*}"
NAME="${REPO##*/}"
API="https://${GITEA_HOST}/api/v1"
AUTH="Authorization: token ${GITEA_TOKEN}"
# 1. Fetch the PR. If not merged, no-op.
PR=$(curl -sS -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}")
MERGED=$(echo "$PR" | jq -r '.merged // false')
if [ "$MERGED" != "true" ]; then
echo "::notice::PR #${PR_NUMBER} closed without merge — no audit emission."
exit 0
fi
MERGE_SHA=$(echo "$PR" | jq -r '.merge_commit_sha // empty')
MERGED_BY=$(echo "$PR" | jq -r '.merged_by.login // "unknown"')
TITLE=$(echo "$PR" | jq -r '.title // ""')
BASE_BRANCH=$(echo "$PR" | jq -r '.base.ref // "main"')
HEAD_SHA=$(echo "$PR" | jq -r '.head.sha // empty')
if [ -z "$MERGE_SHA" ]; then
echo "::warning::PR #${PR_NUMBER} merged=true but no merge_commit_sha — cannot evaluate force-merge."
exit 0
fi
# 2. Required status checks declared in the workflow env.
REQUIRED="$REQUIRED_CHECKS"
if [ -z "${REQUIRED//[[:space:]]/}" ]; then
echo "::notice::REQUIRED_CHECKS empty — force-merge not applicable."
exit 0
fi
# 3. Status-check state at the PR HEAD (where checks ran). The merge
# commit doesn't get its own checks; we evaluate the PR's last
# commit, which is what branch protection compared against.
STATUS=$(curl -sS -H "$AUTH" \
"${API}/repos/${OWNER}/${NAME}/commits/${HEAD_SHA}/status")
declare -A CHECK_STATE
while IFS=$'\t' read -r ctx state; do
[ -n "$ctx" ] && CHECK_STATE[$ctx]="$state"
done < <(echo "$STATUS" | jq -r '.statuses // [] | .[] | "\(.context)\t\(.status)"')
# 4. For each required check, was it green at merge? YAML block scalars
# (`|`) leave a trailing newline; skip blank/whitespace-only lines.
FAILED_CHECKS=()
while IFS= read -r req; do
trimmed="${req#"${req%%[![:space:]]*}"}" # ltrim
trimmed="${trimmed%"${trimmed##*[![:space:]]}"}" # rtrim
[ -z "$trimmed" ] && continue
state="${CHECK_STATE[$trimmed]:-missing}"
if [ "$state" != "success" ]; then
FAILED_CHECKS+=("${trimmed}=${state}")
fi
done <<< "$REQUIRED"
if [ "${#FAILED_CHECKS[@]}" -eq 0 ]; then
echo "::notice::PR #${PR_NUMBER} merged with all required checks green — not a force-merge."
exit 0
fi
# 5. Emit structured audit event.
NOW=$(date -u +%Y-%m-%dT%H:%M:%SZ)
FAILED_JSON=$(printf '%s\n' "${FAILED_CHECKS[@]}" | jq -R . | jq -s .)
# Print as a single-line JSON so Vector's parse_json transform can pick
# it up cleanly from docker_logs.
jq -nc \
--arg event_type "incident.force_merge" \
--arg ts "$NOW" \
--arg repo "$REPO" \
--argjson pr "$PR_NUMBER" \
--arg title "$TITLE" \
--arg base "$BASE_BRANCH" \
--arg merged_by "$MERGED_BY" \
--arg merge_sha "$MERGE_SHA" \
--argjson failed_checks "$FAILED_JSON" \
'{event_type: $event_type, ts: $ts, repo: $repo, pr: $pr, title: $title,
base_branch: $base, merged_by: $merged_by, merge_sha: $merge_sha,
failed_checks: $failed_checks}'
echo "::warning::FORCE-MERGE detected on PR #${PR_NUMBER} by ${MERGED_BY}: ${#FAILED_CHECKS[@]} required check(s) not green at merge time."

View File

@ -1,149 +0,0 @@
#!/usr/bin/env bash
# sop-tier-check — verify a Gitea PR satisfies the §SOP-6 approval gate.
#
# Reads the PR's tier label, walks approving reviewers, and checks each
# approver's Gitea team membership against the tier's eligible-team set.
# Marks pass only when at least one non-author approver is in an eligible
# team.
#
# Invoked from `.gitea/workflows/sop-tier-check.yml`. The workflow sets
# the env vars below; this script does no IO outside of stdout/stderr +
# the Gitea API.
#
# Required env:
# GITEA_TOKEN — bot PAT with read:organization,read:user,
# read:issue,read:repository scopes
# GITEA_HOST — e.g. git.moleculesai.app
# REPO — owner/name (from github.repository)
# PR_NUMBER — int (from github.event.pull_request.number)
# PR_AUTHOR — login (from github.event.pull_request.user.login)
#
# Optional:
# SOP_DEBUG=1 — print per-API-call diagnostic lines (HTTP codes,
# raw response bodies). Default: off.
#
# Stale-status caveat: Gitea Actions does not always re-fire workflows
# on `labeled` / `pull_request_review:submitted` events. If the
# sop-tier-check status is stale (e.g. red after labels/approvals were
# added), push an empty commit to the PR branch to force a synchronize
# event, OR re-request reviews. Tracked: internal#46.
set -euo pipefail
debug() {
if [ "${SOP_DEBUG:-}" = "1" ]; then
echo " [debug] $*" >&2
fi
}
# Validate env
: "${GITEA_TOKEN:?GITEA_TOKEN required}"
: "${GITEA_HOST:?GITEA_HOST required}"
: "${REPO:?REPO required (owner/name)}"
: "${PR_NUMBER:?PR_NUMBER required}"
: "${PR_AUTHOR:?PR_AUTHOR required}"
OWNER="${REPO%%/*}"
NAME="${REPO##*/}"
API="https://${GITEA_HOST}/api/v1"
AUTH="Authorization: token ${GITEA_TOKEN}"
echo "::notice::tier-check start: repo=$OWNER/$NAME pr=$PR_NUMBER author=$PR_AUTHOR"
# Sanity: token resolves to a user
WHOAMI=$(curl -sS -H "$AUTH" "${API}/user" | jq -r '.login // ""')
if [ -z "$WHOAMI" ]; then
echo "::error::GITEA_TOKEN cannot resolve a user via /api/v1/user — check the token scope and that the secret is wired correctly."
exit 1
fi
echo "::notice::token resolves to user: $WHOAMI"
# 1. Read tier label
LABELS=$(curl -sS -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/issues/${PR_NUMBER}/labels" | jq -r '.[].name')
TIER=""
for L in $LABELS; do
case "$L" in
tier:low|tier:medium|tier:high)
if [ -n "$TIER" ]; then
echo "::error::Multiple tier labels: $TIER + $L. Apply exactly one."
exit 1
fi
TIER="$L"
;;
esac
done
if [ -z "$TIER" ]; then
echo "::error::PR has no tier:low|tier:medium|tier:high label. Apply one before merge."
exit 1
fi
debug "tier=$TIER"
# 2. Tier → eligible teams
case "$TIER" in
tier:low) ELIGIBLE="engineers managers ceo" ;;
tier:medium) ELIGIBLE="managers ceo" ;;
tier:high) ELIGIBLE="ceo" ;;
esac
debug "eligible_teams=$ELIGIBLE"
# Resolve team-name → team-id once. /orgs/{org}/teams/{slug}/... endpoints
# don't exist on Gitea 1.22; we have to use /teams/{id}.
ORG_TEAMS_FILE=$(mktemp)
trap 'rm -f "$ORG_TEAMS_FILE"' EXIT
HTTP_CODE=$(curl -sS -o "$ORG_TEAMS_FILE" -w '%{http_code}' -H "$AUTH" \
"${API}/orgs/${OWNER}/teams")
debug "teams-list HTTP=$HTTP_CODE size=$(wc -c <"$ORG_TEAMS_FILE")"
if [ "${SOP_DEBUG:-}" = "1" ]; then
echo " [debug] teams-list body (first 300 chars):" >&2
head -c 300 "$ORG_TEAMS_FILE" >&2; echo >&2
fi
if [ "$HTTP_CODE" != "200" ]; then
echo "::error::GET /orgs/${OWNER}/teams returned HTTP $HTTP_CODE — token likely lacks read:org scope. Add a SOP_TIER_CHECK_TOKEN secret with read:organization scope at the org level."
exit 1
fi
declare -A TEAM_ID
for T in $ELIGIBLE; do
ID=$(jq -r --arg t "$T" '.[] | select(.name==$t) | .id' <"$ORG_TEAMS_FILE" | head -1)
if [ -z "$ID" ] || [ "$ID" = "null" ]; then
VISIBLE=$(jq -r '.[]?.name? // empty' <"$ORG_TEAMS_FILE" 2>/dev/null | tr '\n' ' ')
echo "::error::Team \"$T\" not found in org $OWNER. Teams visible: $VISIBLE"
exit 1
fi
TEAM_ID[$T]="$ID"
debug "team-id: $T$ID"
done
# 3. Read approving reviewers
REVIEWS=$(curl -sS -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}/reviews")
APPROVERS=$(echo "$REVIEWS" | jq -r '[.[] | select(.state=="APPROVED") | .user.login] | unique | .[]')
if [ -z "$APPROVERS" ]; then
echo "::error::No approving reviews. Tier $TIER requires approval from {$ELIGIBLE} (non-author)."
exit 1
fi
debug "approvers: $(echo "$APPROVERS" | tr '\n' ' ')"
# 4. For each approver: check non-author + team membership (by id)
OK=""
for U in $APPROVERS; do
if [ "$U" = "$PR_AUTHOR" ]; then
debug "skip self-review by $U"
continue
fi
for T in $ELIGIBLE; do
ID="${TEAM_ID[$T]}"
CODE=$(curl -sS -o /dev/null -w '%{http_code}' -H "$AUTH" \
"${API}/teams/${ID}/members/${U}")
debug "probe: $U in team $T (id=$ID) → HTTP $CODE"
if [ "$CODE" = "200" ] || [ "$CODE" = "204" ]; then
echo "::notice::approver $U is in team $T (eligible for $TIER)"
OK="yes"
break
fi
done
[ -n "$OK" ] && break
done
if [ -z "$OK" ]; then
echo "::error::Tier $TIER requires approval from a non-author member of {$ELIGIBLE}. Got approvers: $APPROVERS — none of them satisfied team membership. Set SOP_DEBUG=1 to see per-probe HTTP codes."
exit 1
fi
echo "::notice::sop-tier-check passed: $TIER, approver in {$ELIGIBLE}"

View File

@ -1,58 +0,0 @@
# audit-force-merge — emit `incident.force_merge` to runner stdout when
# a PR is merged with required-status-checks not green. Vector picks
# the JSON line off docker_logs and ships to Loki on
# molecule-canonical-obs (per `reference_obs_stack_phase1`); query as:
#
# {host="operator"} |= "event_type" |= "incident.force_merge" | json
#
# Closes the §SOP-6 audit gap (the doc says force-merges write to
# `structure_events`, but that table lives in the platform DB, not
# Gitea-side; Loki is the practical equivalent for Gitea Actions
# events). When the credential / observability stack converges later,
# this can sync into structure_events from Loki via a backfill job —
# the structured JSON shape is forward-compatible.
#
# Logic in `.gitea/scripts/audit-force-merge.sh` per the same script-
# extract pattern as sop-tier-check.
name: audit-force-merge
# pull_request_target loads from the base branch — same security model
# as sop-tier-check. Without this, an attacker could rewrite the
# workflow on a PR and skip the audit emission for their own
# force-merge. See `.gitea/workflows/sop-tier-check.yml` for the full
# rationale.
on:
pull_request_target:
types: [closed]
jobs:
audit:
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: read
# Skip when PR is closed without merge — saves a runner.
if: github.event.pull_request.merged == true
steps:
- name: Check out base branch (for the script)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
ref: ${{ github.event.pull_request.base.sha }}
- name: Detect force-merge + emit audit event
env:
# Same org-level secret the sop-tier-check workflow uses.
GITEA_TOKEN: ${{ secrets.SOP_TIER_CHECK_TOKEN || secrets.GITHUB_TOKEN }}
GITEA_HOST: git.moleculesai.app
REPO: ${{ github.repository }}
PR_NUMBER: ${{ github.event.pull_request.number }}
# Required-status-check contexts to evaluate at merge time.
# Newline-separated. Mirror this against branch protection
# (settings → branches → protected branch → required checks).
# Declared here rather than fetched from /branch_protections
# because that endpoint requires admin write — sop-tier-bot is
# read-only by design (least-privilege).
REQUIRED_CHECKS: |
sop-tier-check / tier-check (pull_request)
Secret scan / Scan diff for credential-shaped strings (pull_request)
run: bash .gitea/scripts/audit-force-merge.sh

View File

@ -1,191 +0,0 @@
name: Secret scan
# Hard CI gate. Refuses any PR / push whose diff additions contain a
# recognisable credential. Defense-in-depth for the #2090-class incident
# (2026-04-24): GitHub's hosted Copilot Coding Agent leaked a ghs_*
# installation token into tenant-proxy/package.json via `npm init`
# slurping the URL from a token-embedded origin remote. We can't fix
# upstream's clone hygiene, so we gate here.
#
# Same regex set as the runtime's bundled pre-commit hook
# (molecule-ai-workspace-runtime: molecule_runtime/scripts/pre-commit-checks.sh).
# Keep the two sides aligned when adding patterns.
#
# Ported from .github/workflows/secret-scan.yml so the gate actually
# fires on Gitea Actions. Differences from the GitHub version:
# - drops `merge_group` event (Gitea has no merge queue)
# - drops `workflow_call` (no cross-repo reusable invocation on Gitea)
# - SELF path updated to .gitea/workflows/secret-scan.yml
# The job name + step name are identical to the GitHub workflow so the
# status-check context (`Secret scan / Scan diff for credential-shaped
# strings (pull_request)`) matches branch protection on molecule-core/main.
on:
pull_request:
types: [opened, synchronize, reopened]
push:
branches: [main, staging]
jobs:
scan:
name: Scan diff for credential-shaped strings
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
fetch-depth: 2 # need previous commit to diff against on push events
# For pull_request events the diff base may be many commits behind
# HEAD and absent from the shallow clone. Fetch it explicitly.
- name: Fetch PR base SHA (pull_request events only)
if: github.event_name == 'pull_request'
run: git fetch --depth=1 origin ${{ github.event.pull_request.base.sha }}
- name: Refuse if credential-shaped strings appear in diff additions
env:
# Plumb event-specific SHAs through env so the script doesn't
# need conditional `${{ ... }}` interpolation per event type.
# github.event.before/after only exist on push events;
# pull_request has pull_request.base.sha / pull_request.head.sha.
PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
PUSH_BEFORE: ${{ github.event.before }}
PUSH_AFTER: ${{ github.event.after }}
run: |
# Pattern set covers GitHub family (the actual #2090 vector),
# Anthropic / OpenAI / Slack / AWS. Anchored on prefixes with low
# false-positive rates against agent-generated content. Mirror of
# molecule-ai-workspace-runtime/molecule_runtime/scripts/pre-commit-checks.sh
# — keep aligned.
SECRET_PATTERNS=(
'ghp_[A-Za-z0-9]{36,}' # GitHub PAT (classic)
'ghs_[A-Za-z0-9]{36,}' # GitHub App installation token
'gho_[A-Za-z0-9]{36,}' # GitHub OAuth user-to-server
'ghu_[A-Za-z0-9]{36,}' # GitHub OAuth user
'ghr_[A-Za-z0-9]{36,}' # GitHub OAuth refresh
'github_pat_[A-Za-z0-9_]{82,}' # GitHub fine-grained PAT
'sk-ant-[A-Za-z0-9_-]{40,}' # Anthropic API key
'sk-proj-[A-Za-z0-9_-]{40,}' # OpenAI project key
'sk-svcacct-[A-Za-z0-9_-]{40,}' # OpenAI service-account key
'sk-cp-[A-Za-z0-9_-]{60,}' # MiniMax API key (F1088 vector — caught only after the fact)
'xox[baprs]-[A-Za-z0-9-]{20,}' # Slack tokens
'AKIA[0-9A-Z]{16}' # AWS access key ID
'ASIA[0-9A-Z]{16}' # AWS STS temp access key ID
)
# Determine the diff base. Each event type stores its SHAs in
# a different place — see the env block above.
case "${{ github.event_name }}" in
pull_request)
BASE="$PR_BASE_SHA"
HEAD="$PR_HEAD_SHA"
;;
*)
BASE="$PUSH_BEFORE"
HEAD="$PUSH_AFTER"
;;
esac
# On push events with shallow clones, BASE may be present in
# the event payload but absent from the local object DB
# (fetch-depth=2 doesn't always reach the previous commit
# across true merges). Try fetching it on demand. If the
# fetch fails — e.g. the SHA was force-overwritten — we fall
# through to the empty-BASE branch below, which scans the
# entire tree as if every file were new. Correct, just slow.
if [ -n "$BASE" ] && ! echo "$BASE" | grep -qE '^0+$'; then
if ! git cat-file -e "$BASE" 2>/dev/null; then
git fetch --depth=1 origin "$BASE" 2>/dev/null || true
fi
fi
# Files added or modified in this change.
if [ -z "$BASE" ] || echo "$BASE" | grep -qE '^0+$' || ! git cat-file -e "$BASE" 2>/dev/null; then
# New branch / no previous SHA / BASE unreachable — check the
# entire tree as added content. Slower, but correct on first
# push.
CHANGED=$(git ls-tree -r --name-only HEAD)
DIFF_RANGE=""
else
CHANGED=$(git diff --name-only --diff-filter=AM "$BASE" "$HEAD")
DIFF_RANGE="$BASE $HEAD"
fi
if [ -z "$CHANGED" ]; then
echo "No changed files to inspect."
exit 0
fi
# Self-exclude: this workflow file legitimately contains the
# pattern strings as regex literals. Without an exclude it would
# block its own merge. Both the .github/ original and this
# .gitea/ port are excluded so a sync between them stays clean.
SELF_GITHUB=".github/workflows/secret-scan.yml"
SELF_GITEA=".gitea/workflows/secret-scan.yml"
OFFENDING=""
# `while IFS= read -r` (not `for f in $CHANGED`) so filenames
# containing whitespace don't word-split silently — a path
# with a space would otherwise produce two iterations on
# tokens that aren't real filenames, breaking the
# self-exclude + diff lookup.
while IFS= read -r f; do
[ -z "$f" ] && continue
[ "$f" = "$SELF_GITHUB" ] && continue
[ "$f" = "$SELF_GITEA" ] && continue
if [ -n "$DIFF_RANGE" ]; then
ADDED=$(git diff --no-color --unified=0 "$BASE" "$HEAD" -- "$f" 2>/dev/null | grep -E '^\+[^+]' || true)
else
# No diff range (new branch first push) — scan the full file
# contents as if every line were new.
ADDED=$(cat "$f" 2>/dev/null || true)
fi
[ -z "$ADDED" ] && continue
for pattern in "${SECRET_PATTERNS[@]}"; do
if echo "$ADDED" | grep -qE "$pattern"; then
OFFENDING="${OFFENDING}${f} (matched: ${pattern})\n"
break
fi
done
done <<< "$CHANGED"
if [ -n "$OFFENDING" ]; then
echo "::error::Credential-shaped strings detected in diff additions:"
# `printf '%b' "$OFFENDING"` interprets backslash escapes
# (the literal `\n` we appended above becomes a newline)
# WITHOUT treating OFFENDING as a format string. Plain
# `printf "$OFFENDING"` is a format-string sink: a filename
# containing `%` would be interpreted as a conversion
# specifier, corrupting the error message (or printing
# `%(missing)` artifacts).
printf '%b' "$OFFENDING"
echo ""
echo "The actual matched values are NOT echoed here, deliberately —"
echo "round-tripping a leaked credential into CI logs widens the blast"
echo "radius (logs are searchable + retained)."
echo ""
echo "Recovery:"
echo " 1. Remove the secret from the file. Replace with an env var"
echo " reference (e.g. \${{ secrets.GITHUB_TOKEN }} in workflows,"
echo " process.env.X in code)."
echo " 2. If the credential was already pushed (this PR's commit"
echo " history reaches a public ref), treat it as compromised —"
echo " ROTATE it immediately, do not just remove it. The token"
echo " remains valid in git history forever and may be in any"
echo " log/cache that consumed this branch."
echo " 3. Force-push the cleaned commit (or stack a revert) and"
echo " re-run CI."
echo ""
echo "If the match is a false positive (test fixture, docs example,"
echo "or this workflow's own regex literals): use a clearly-fake"
echo "placeholder like ghs_EXAMPLE_DO_NOT_USE that doesn't satisfy"
echo "the length suffix, OR add the file path to the SELF exclude"
echo "list in this workflow with a short reason."
echo ""
echo "Mirror of the regex set lives in the runtime's bundled"
echo "pre-commit hook (molecule-ai-workspace-runtime:"
echo "molecule_runtime/scripts/pre-commit-checks.sh) — keep aligned."
exit 1
fi
echo "✓ No credential-shaped strings in this change."

View File

@ -1,81 +0,0 @@
# sop-tier-check — canonical Gitea Actions workflow for §SOP-6 enforcement.
#
# Logic lives in `.gitea/scripts/sop-tier-check.sh` (extracted 2026-05-09
# from the previous inline-bash version). The script is the single source
# of truth; this workflow file just sets env + invokes it.
#
# Copy BOTH files (`.gitea/workflows/sop-tier-check.yml` +
# `.gitea/scripts/sop-tier-check.sh`) into any repo that wants the
# §SOP-6 PR gate enforced. Pair with branch protection on the protected
# branch:
# required_status_checks: ["sop-tier-check / tier-check (pull_request)"]
# required_approving_reviews: 1
# approving_review_teams: ["ceo", "managers", "engineers"]
#
# Tier → eligible-team mapping (mirror of dev-sop §SOP-6):
# tier:low → engineers, managers, ceo
# tier:medium → managers, ceo
# tier:high → ceo
#
# Force-merge: Owners-team override remains available out-of-band via
# the Gitea merge API; force-merge writes `incident.force_merge` to
# `structure_events` per §Persistent structured logging gate (Phase 3).
#
# Set `SOP_DEBUG: '1'` in the env block to enable per-API-call diagnostic
# lines — useful when diagnosing token-scope or team-id-resolution
# issues. Default off.
name: sop-tier-check
# SECURITY: triggers MUST use `pull_request_target`, not `pull_request`.
# `pull_request_target` loads the workflow definition from the BASE
# branch (i.e. `main`), not the PR's HEAD. With `pull_request`, anyone
# with write access to a feature branch could rewrite this file in
# their PR to dump SOP_TIER_CHECK_TOKEN (org-read scope) to logs and
# exfiltrate it. Verified 2026-05-09 against Gitea 1.22.6 —
# `pull_request_target` (added in Gitea 1.21 via go-gitea/gitea#25229)
# is the documented mitigation.
#
# This workflow does NOT call `actions/checkout` of PR HEAD code, so no
# untrusted code is ever executed in the runner — we only HTTP-call the
# Gitea API. If a future change adds a checkout step, it MUST pin to
# `${{ github.event.pull_request.base.sha }}` (NOT `head.sha`) to keep
# the trust boundary.
on:
pull_request_target:
types: [opened, edited, synchronize, reopened, labeled, unlabeled]
pull_request_review:
types: [submitted, dismissed, edited]
jobs:
tier-check:
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: read
steps:
- name: Check out base branch (for the script)
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
# Pin to base.sha — pull_request_target's protection only
# works if we never check out PR HEAD. Same SHA the workflow
# itself was loaded from.
ref: ${{ github.event.pull_request.base.sha }}
- name: Verify tier label + reviewer team membership
env:
# SOP_TIER_CHECK_TOKEN is the org-level secret for the
# sop-tier-bot PAT (read:organization,read:user,read:issue,
# read:repository). Stored at the org level
# (/api/v1/orgs/molecule-ai/actions/secrets) so per-repo
# configuration is unnecessary — every repo in the org
# picks it up automatically.
# Falls back to GITHUB_TOKEN with a clear error if missing.
GITEA_TOKEN: ${{ secrets.SOP_TIER_CHECK_TOKEN || secrets.GITHUB_TOKEN }}
GITEA_HOST: git.moleculesai.app
REPO: ${{ github.repository }}
PR_NUMBER: ${{ github.event.pull_request.number }}
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
# Set to '1' for diagnostic per-API-call output. Off by default
# so production logs aren't noisy.
SOP_DEBUG: '0'
run: bash .gitea/scripts/sop-tier-check.sh

View File

@ -20,19 +20,6 @@ on:
# a few minutes under load — that's fine for a canary. # a few minutes under load — that's fine for a canary.
- cron: '*/30 * * * *' - cron: '*/30 * * * *'
workflow_dispatch: workflow_dispatch:
inputs:
keep_on_failure:
description: >-
Skip teardown when the canary fails (debugging only). The
tenant org + EC2 + CF tunnel + DNS stay alive so an operator
can SSM into the workspace EC2 and capture docker logs of the
failing claude-code container. REMEMBER to manually delete
via DELETE /cp/admin/tenants/<slug> when done so the org
doesn't accumulate cost. Only honored on workflow_dispatch;
cron runs always tear down (we don't want unattended cron
to leak resources).
type: boolean
default: false
# Serialise with the full-SaaS workflow so they don't contend for the # Serialise with the full-SaaS workflow so they don't contend for the
# same org-create quota on staging. Different group key from # same org-create quota on staging. Different group key from
@ -93,14 +80,6 @@ jobs:
# is "Token Plan only" but cheap-per-token and fast. # is "Token Plan only" but cheap-per-token and fast.
E2E_MODEL_SLUG: MiniMax-M2.7-highspeed E2E_MODEL_SLUG: MiniMax-M2.7-highspeed
E2E_RUN_ID: "canary-${{ github.run_id }}" E2E_RUN_ID: "canary-${{ github.run_id }}"
# Debug-only: when an operator dispatches with keep_on_failure=true,
# the canary script's E2E_KEEP_ORG=1 path skips teardown so the
# tenant org + EC2 stay alive for SSM-based log capture. Cron runs
# never set this (the input only exists on workflow_dispatch) so
# unattended cron always tears down. See molecule-core#129
# failure mode #1 — capturing the actual exception requires
# docker logs from the live container.
E2E_KEEP_ORG: ${{ github.event.inputs.keep_on_failure == 'true' && '1' || '0' }}
steps: steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
@ -158,28 +137,27 @@ jobs:
id: canary id: canary
run: bash tests/e2e/test_staging_full_saas.sh run: bash tests/e2e/test_staging_full_saas.sh
# Alerting: open a sticky issue on the FIRST failure; comment on # Alerting: open an issue only after THREE consecutive failures so
# subsequent failures; auto-close on next green. Comment-on-existing # transient flakes (Cloudflare DNS hiccup, AWS API blip) don't spam
# de-duplicates so a single open issue accumulates the streak — # the issue list. If an issue is already open, we still comment on
# ops sees one issue with N comments rather than N issues. # every failure so ops sees the streak. Auto-close on next green.
# #
# Why no consecutive-failures threshold (e.g., wait 3 runs before # Threshold rationale: canary fires every 30 min, so 3 failures =
# filing): the prior threshold check used # ~90 min of consecutive red — well past any single-run flake but
# `github.rest.actions.listWorkflowRuns()` which Gitea 1.22.6 does # still tight enough that a real outage gets surfaced before the
# not expose (returns 404). On Gitea Actions the threshold call # next deploy window.
# ALWAYS failed, breaking the entire alerting step and going days
# silent on real regressions (38h+ chronic red on 2026-05-07/08
# before this fix; tracked in molecule-core#129). Filing on first
# failure is also better UX — we want to know about the first red,
# not wait 90 min for it to "count." Real flakes get one issue +
# a quick close-on-green; persistent reds accumulate comments.
- name: Open issue on failure - name: Open issue on failure
if: failure() if: failure()
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
env:
# Inject the workflow path explicitly — context.workflow is
# the *name*, not the file path the actions API needs.
WORKFLOW_PATH: '.github/workflows/canary-staging.yml'
CONSECUTIVE_THRESHOLD: '3'
with: with:
script: | script: |
const title = '🔴 Canary failing: staging SaaS smoke'; const title = '🔴 Canary failing: staging SaaS smoke';
const runURL = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; const runURL = `https://github.com/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
// Find an existing open canary issue (stable title match). // Find an existing open canary issue (stable title match).
// If one exists, this isn't a "first failure" — comment and exit. // If one exists, this isn't a "first failure" — comment and exit.
@ -199,12 +177,32 @@ jobs:
return; return;
} }
// No open issue yet — file one on this first failure. The // No open issue yet — check the last N-1 runs' conclusions.
// comment-on-existing branch above means subsequent failures // We open the issue only if the last (THRESHOLD-1) runs ALSO
// accumulate as comments on this same issue, so we don't // failed (so this is the 3rd consecutive red).
// spam new issues per run. const threshold = parseInt(process.env.CONSECUTIVE_THRESHOLD, 10);
const { data: runs } = await github.rest.actions.listWorkflowRuns({
owner: context.repo.owner, repo: context.repo.repo,
workflow_id: process.env.WORKFLOW_PATH,
status: 'completed',
per_page: threshold,
// Skip the current in-progress run; it isn't 'completed' yet.
});
// listWorkflowRuns returns recent first. We need (threshold-1)
// prior failures (current run is the threshold-th).
const priorFailures = (runs.workflow_runs || [])
.slice(0, threshold - 1)
.filter(r => r.id !== context.runId)
.filter(r => r.conclusion === 'failure')
.length;
if (priorFailures < threshold - 1) {
core.info(`Below threshold: ${priorFailures + 1}/${threshold} consecutive failures — not filing yet`);
return;
}
const body = const body =
`Canary run failed at ${new Date().toISOString()}.\n\n` + `Canary run failed at ${new Date().toISOString()}, ` +
`${threshold} consecutive runs red.\n\n` +
`Run: ${runURL}\n\n` + `Run: ${runURL}\n\n` +
`This issue auto-closes on the next green canary run. ` + `This issue auto-closes on the next green canary run. ` +
`Consecutive failures add a comment here rather than a new issue.`; `Consecutive failures add a comment here rather than a new issue.`;
@ -213,7 +211,7 @@ jobs:
title, body, title, body,
labels: ['canary-staging', 'bug'], labels: ['canary-staging', 'bug'],
}); });
core.info('Opened canary failure issue (first red)'); core.info(`Opened canary failure issue (${threshold} consecutive reds)`);
- name: Auto-close canary issue on success - name: Auto-close canary issue on success
if: success() if: success()

View File

@ -51,7 +51,7 @@ name: E2E API Smoke Test
# * Pre-pull `alpine:latest` so the platform-server's provisioner # * Pre-pull `alpine:latest` so the platform-server's provisioner
# (`internal/handlers/container_files.go`) can stand up its # (`internal/handlers/container_files.go`) can stand up its
# ephemeral token-write helper without a daemon.io round-trip. # ephemeral token-write helper without a daemon.io round-trip.
# * Create `molecule-core-net` bridge network if missing so the # * Create `molecule-monorepo-net` bridge network if missing so the
# provisioner's container.HostConfig {NetworkMode: ...} attach # provisioner's container.HostConfig {NetworkMode: ...} attach
# succeeds. # succeeds.
# Item #1 (timeouts) — evidence on recent runs (77/3191, ae/4270, 0e/ # Item #1 (timeouts) — evidence on recent runs (77/3191, ae/4270, 0e/
@ -163,12 +163,12 @@ jobs:
# when the image is already present. # when the image is already present.
docker pull alpine:latest >/dev/null docker pull alpine:latest >/dev/null
# Provisioner attaches workspace containers to # Provisioner attaches workspace containers to
# molecule-core-net (workspace-server/internal/provisioner/ # molecule-monorepo-net (workspace-server/internal/provisioner/
# provisioner.go::DefaultNetwork). The bridge already exists on # provisioner.go::DefaultNetwork). The bridge already exists on
# the operator host's docker daemon — `network create` is # the operator host's docker daemon — `network create` is
# idempotent via `|| true`. # idempotent via `|| true`.
docker network create molecule-core-net >/dev/null 2>&1 || true docker network create molecule-monorepo-net >/dev/null 2>&1 || true
echo "alpine:latest pre-pulled; molecule-core-net ensured." echo "alpine:latest pre-pulled; molecule-monorepo-net ensured."
- name: Start Postgres (docker) - name: Start Postgres (docker)
if: needs.detect-changes.outputs.api == 'true' if: needs.detect-changes.outputs.api == 'true'
run: | run: |

View File

@ -34,7 +34,7 @@ name: Handlers Postgres Integration
# So we sidestep `services:` entirely. The job container still uses # So we sidestep `services:` entirely. The job container still uses
# host-net (inherited from runner config; required for cache server # host-net (inherited from runner config; required for cache server
# discovery on the bridge IP 172.18.0.17:42631). We launch a sibling # discovery on the bridge IP 172.18.0.17:42631). We launch a sibling
# postgres on the existing `molecule-core-net` bridge with a # postgres on the existing `molecule-monorepo-net` bridge with a
# UNIQUE name per run — `pg-handlers-${RUN_ID}-${RUN_ATTEMPT}` — and # UNIQUE name per run — `pg-handlers-${RUN_ID}-${RUN_ATTEMPT}` — and
# read its bridge IP via `docker inspect`. A host-net job container # read its bridge IP via `docker inspect`. A host-net job container
# can reach a bridge-net container directly via the bridge IP (verified # can reach a bridge-net container directly via the bridge IP (verified
@ -44,7 +44,7 @@ name: Handlers Postgres Integration
# + No host-port collision; N parallel runs share the bridge cleanly # + No host-port collision; N parallel runs share the bridge cleanly
# + `if: always()` cleanup runs even on test-step failure # + `if: always()` cleanup runs even on test-step failure
# - One more step in the workflow (+~3 lines) # - One more step in the workflow (+~3 lines)
# - Requires `molecule-core-net` to exist on the operator host # - Requires `molecule-monorepo-net` to exist on the operator host
# (it does; declared in docker-compose.yml + docker-compose.infra.yml) # (it does; declared in docker-compose.yml + docker-compose.infra.yml)
# #
# Class B Hongming-owned CICD red sweep, 2026-05-08. # Class B Hongming-owned CICD red sweep, 2026-05-08.
@ -96,7 +96,7 @@ jobs:
PG_NAME: pg-handlers-${{ github.run_id }}-${{ github.run_attempt }} PG_NAME: pg-handlers-${{ github.run_id }}-${{ github.run_attempt }}
# Bridge network already exists on the operator host (declared # Bridge network already exists on the operator host (declared
# in docker-compose.yml + docker-compose.infra.yml). # in docker-compose.yml + docker-compose.infra.yml).
PG_NETWORK: molecule-core-net PG_NETWORK: molecule-monorepo-net
defaults: defaults:
run: run:
working-directory: workspace-server working-directory: workspace-server

View File

@ -119,17 +119,6 @@ jobs:
# symptom, different root cause: staging still has the in-image # symptom, different root cause: staging still has the in-image
# clone path, hits the auth error directly). # clone path, hits the auth error directly).
# #
# 2026-05-08 sub-finding (#192): the clone step ALSO fails when
# any referenced workspace-template repo is private and the
# AUTO_SYNC_TOKEN bearer (devops-engineer persona) lacks read
# access. Root cause: 5 of 9 workspace-template repos
# (openclaw, codex, crewai, deepagents, gemini-cli) had been
# marked private with no team grant. Resolution: flipped them
# to public per `feedback_oss_first_repo_visibility_default`
# (the OSS surface should be public). Layer-3 (customer-private +
# marketplace third-party repos) tracked separately in
# internal#102.
#
# Token shape matches publish-workspace-server-image.yml: AUTO_SYNC_TOKEN # Token shape matches publish-workspace-server-image.yml: AUTO_SYNC_TOKEN
# is the devops-engineer persona PAT, NOT the founder PAT (per # is the devops-engineer persona PAT, NOT the founder PAT (per
# `feedback_per_agent_gitea_identity_default`). clone-manifest.sh # `feedback_per_agent_gitea_identity_default`). clone-manifest.sh

View File

@ -284,7 +284,7 @@ cp .env.example .env
./infra/scripts/setup.sh ./infra/scripts/setup.sh
# Boots Postgres (:5432), Redis (:6379), Langfuse (:3001), # Boots Postgres (:5432), Redis (:6379), Langfuse (:3001),
# and Temporal (:7233 gRPC, :8233 UI) on the shared # and Temporal (:7233 gRPC, :8233 UI) on the shared
# `molecule-core-net` Docker network. Temporal runs with # `molecule-monorepo-net` Docker network. Temporal runs with
# no auth on localhost — dev-only; production must gate it. # no auth on localhost — dev-only; production must gate it.
# #
# Also populates the template/plugin registry by cloning every repo # Also populates the template/plugin registry by cloning every repo

View File

@ -283,7 +283,7 @@ cp .env.example .env
./infra/scripts/setup.sh ./infra/scripts/setup.sh
# 启动 Postgres (:5432)、Redis (:6379)、Langfuse (:3001) # 启动 Postgres (:5432)、Redis (:6379)、Langfuse (:3001)
# 以及 Temporal (:7233 gRPC, :8233 UI),全部挂在共享的 # 以及 Temporal (:7233 gRPC, :8233 UI),全部挂在共享的
# `molecule-core-net` Docker 网络上。Temporal 默认无鉴权, # `molecule-monorepo-net` Docker 网络上。Temporal 默认无鉴权,
# 仅用于本地开发;生产环境必须加 mTLS / API Key。 # 仅用于本地开发;生产环境必须加 mTLS / API Key。
# #
# 同时会根据 manifest.json 拉取所有模板/插件仓库到 # 同时会根据 manifest.json 拉取所有模板/插件仓库到

View File

@ -1,10 +0,0 @@
# Excluded from `docker build` context. Without this, the COPY . . step in
# canvas/Dockerfile clobbers the freshly-installed node_modules with the
# host's (potentially broken / wrong-arch) copy — the @tailwindcss/oxide
# native binary disagreed and broke `next build`.
node_modules
.next
.git
*.log
.env*
!.env.example

View File

@ -1,11 +1,7 @@
FROM node:22-alpine AS builder FROM node:22-alpine AS builder
WORKDIR /app WORKDIR /app
COPY package.json package-lock.json* ./ COPY package.json package-lock.json* ./
# `npm ci` (not `install`) for lockfile-exact reproducibility. RUN npm install
# `--include=optional` ensures the platform-specific @tailwindcss/oxide
# native binary lands — without it, postcss fails with "Cannot read
# properties of undefined (reading 'All')" at build time.
RUN npm ci --include=optional
COPY . . COPY . .
ARG NEXT_PUBLIC_PLATFORM_URL=http://localhost:8080 ARG NEXT_PUBLIC_PLATFORM_URL=http://localhost:8080
ARG NEXT_PUBLIC_WS_URL=ws://localhost:8080/ws ARG NEXT_PUBLIC_WS_URL=ws://localhost:8080/ws

View File

@ -17,24 +17,6 @@ import { dirname, join } from "node:path";
// update one heuristic. Production is unaffected: `output: "standalone"` // update one heuristic. Production is unaffected: `output: "standalone"`
// bakes resolved env into the build, and the marker file isn't shipped. // bakes resolved env into the build, and the marker file isn't shipped.
loadMonorepoEnv(); loadMonorepoEnv();
// Boot-time matched-pair guard for ADMIN_TOKEN / NEXT_PUBLIC_ADMIN_TOKEN.
// When ADMIN_TOKEN is set on the workspace-server (server-side bearer
// gate, wsauth_middleware.go ~L245), the canvas MUST send the matching
// NEXT_PUBLIC_ADMIN_TOKEN as `Authorization: Bearer ...` on every API
// call. If only one is set, every workspace API call 401s silently —
// the canvas hydrates with empty data and the user sees a broken page
// with no console hint about the auth-config mismatch.
//
// Pre-fix the matched-pair contract was descriptive only (a comment in
// .env): future devs/agents could re-misconfigure with one of the two
// unset and silently 401. Closes the post-PR-#174 self-review gap.
//
// Warn-only (not exit) — production canvas Docker images bake these
// vars into the build at image-build time, and a missed pair there
// would still emit the warning at runtime via the standalone server's
// startup. Killing the process on misconfiguration would turn a
// recoverable auth issue into a hard crashloop.
checkAdminTokenPair();
const nextConfig: NextConfig = { const nextConfig: NextConfig = {
output: "standalone", output: "standalone",
@ -75,43 +57,6 @@ function loadMonorepoEnv() {
); );
} }
// Boot-time matched-pair guard. Runs after .env has been loaded so the
// check sees the post-load state. The two env vars must be set or
// unset together; one-without-the-other is the silent-401 footgun.
//
// Treats empty string ("") as unset. An explicitly-empty `KEY=` in
// .env counts as set-to-empty in `process.env`, but for auth purposes
// an empty bearer token is equivalent to no token — so both
// `ADMIN_TOKEN=` and an unset ADMIN_TOKEN are equivalent relative to
// the matched-pair invariant.
//
// Returns void; side effect is the console.error warning. Kept as a
// separate function (exported) so a future test can reset env, call
// this, and assert on captured stderr.
export function checkAdminTokenPair(): void {
const serverSet = !!process.env.ADMIN_TOKEN;
const clientSet = !!process.env.NEXT_PUBLIC_ADMIN_TOKEN;
if (serverSet === clientSet) return;
// Distinct messages so the operator can tell which half is missing
// — the fix is symmetric (set the other one) but the diagnostic
// mentions which side is currently set so they don't have to grep.
if (serverSet && !clientSet) {
// eslint-disable-next-line no-console
console.error(
"[next.config] ADMIN_TOKEN is set but NEXT_PUBLIC_ADMIN_TOKEN is not — " +
"canvas will 401 against workspace-server because the bearer header " +
"is never attached. Set both to the same value, or unset both.",
);
} else {
// eslint-disable-next-line no-console
console.error(
"[next.config] NEXT_PUBLIC_ADMIN_TOKEN is set but ADMIN_TOKEN is not — " +
"workspace-server will reject the bearer because no AdminAuth gate " +
"is configured. Set both to the same value, or unset both.",
);
}
}
function findMonorepoRoot(start: string): string | null { function findMonorepoRoot(start: string): string | null {
let dir = start; let dir = start;
for (let i = 0; i < 6; i++) { for (let i = 0; i < 6; i++) {

View File

@ -1,6 +1,6 @@
"use client"; "use client";
import { useCallback, useEffect, useMemo, useRef } from "react"; import { useCallback, useMemo } from "react";
import { import {
ReactFlow, ReactFlow,
ReactFlowProvider, ReactFlowProvider,
@ -187,23 +187,6 @@ function CanvasInner() {
// Pan-to-node / zoom-to-team CustomEvent listeners + viewport save. // Pan-to-node / zoom-to-team CustomEvent listeners + viewport save.
const { onMoveEnd } = useCanvasViewport(); const { onMoveEnd } = useCanvasViewport();
// Screen-reader announcements — read liveAnnouncement from the store and
// immediately clear it so the same announcement doesn't re-fire on
// re-render. Using a ref avoids a setState loop while keeping the
// effect reactive to new announcement strings.
const liveAnnouncement = useCanvasStore((s) => s.liveAnnouncement);
const clearAnnouncement = useCanvasStore((s) => s.setLiveAnnouncement);
const prevAnnouncement = useRef("");
useEffect(() => {
if (liveAnnouncement && liveAnnouncement !== prevAnnouncement.current) {
prevAnnouncement.current = liveAnnouncement;
// Small delay so the DOM update lands before clearing, giving
// screen readers time to pick up the new text.
const timer = setTimeout(() => clearAnnouncement(""), 500);
return () => clearTimeout(timer);
}
}, [liveAnnouncement, clearAnnouncement]);
// Delete-confirmation lives in the store so the dialog survives ContextMenu // Delete-confirmation lives in the store so the dialog survives ContextMenu
// unmounting — the prior local-in-ContextMenu state raced with the menu's // unmounting — the prior local-in-ContextMenu state raced with the menu's
// outside-click handler. // outside-click handler.
@ -343,21 +326,11 @@ function CanvasInner() {
<DropTargetBadge /> <DropTargetBadge />
</ReactFlow> </ReactFlow>
{/* Screen-reader live region announces workspace count on initial load and {/* Screen-reader live region: announces workspace count on canvas load or change */}
live status updates from WebSocket events (online, offline, provisioning, etc.). <div role="status" aria-live="polite" className="sr-only">
The liveAnnouncement text is cleared after the screen reader has had time {nodes.filter((n) => !n.parentId).length === 0
to read it so the same message doesn't re-announce on re-render. */} ? "No workspaces on canvas"
<div : `${nodes.filter((n) => !n.parentId).length} workspace${nodes.filter((n) => !n.parentId).length !== 1 ? "s" : ""} on canvas`}
role="status"
aria-live="polite"
aria-atomic="true"
className="sr-only"
>
{liveAnnouncement || (
nodes.filter((n) => !n.parentId).length === 0
? "No workspaces on canvas"
: `${nodes.filter((n) => !n.parentId).length} workspace${nodes.filter((n) => !n.parentId).length !== 1 ? "s" : ""} on canvas`
)}
</div> </div>
{nodes.length === 0 && <EmptyState />} {nodes.length === 0 && <EmptyState />}

View File

@ -1,227 +0,0 @@
"use client";
import { useEffect, useRef, useState } from "react";
import { createPortal } from "react-dom";
interface ShortcutGroup {
title: string;
shortcuts: Array<{ keys: string[]; description: string }>;
}
const SHORTCUT_GROUPS: ShortcutGroup[] = [
{
title: "Canvas",
shortcuts: [
{
keys: ["Esc"],
description: "Close context menu, clear selection, or deselect",
},
{
keys: ["Enter"],
description: "Descend into selected node's first child",
},
{
keys: ["Shift", "Enter"],
description: "Ascend to selected node's parent",
},
{
keys: ["Cmd", "]"],
description: "Bring selected node forward in z-order",
},
{
keys: ["Cmd", "["],
description: "Send selected node backward in z-order",
},
{
keys: ["Z"],
description: "Zoom to fit the selected team and its sub-workspaces",
},
],
},
{
title: "Navigation",
shortcuts: [
{
keys: ["⌘K"],
description: "Open workspace search",
},
{
keys: ["Palette"],
description: "Open the template palette to deploy a new workspace",
},
{
keys: ["Dbl-click"],
description: "Zoom canvas to fit a team node and all its sub-workspaces",
},
{
keys: ["Right-click"],
description: "Open the workspace context menu",
},
],
},
{
title: "Agent",
shortcuts: [
{
keys: ["Chat"],
description: "Send a message or resume a running task",
},
{
keys: ["Config"],
description: "Edit skills, model, secrets, and runtime settings",
},
{
keys: ["Audit"],
description: "View the activity ledger for the selected workspace",
},
],
},
];
interface Props {
open: boolean;
onClose: () => void;
}
export function KeyboardShortcutsDialog({ open, onClose }: Props) {
const dialogRef = useRef<HTMLDivElement>(null);
const [mounted, setMounted] = useState(false);
useEffect(() => {
setMounted(true);
}, []);
// Move focus into the dialog when it opens (WCAG 2.1 SC 2.4.3)
useEffect(() => {
if (!open || !mounted) return;
const raf = requestAnimationFrame(() => {
dialogRef.current?.querySelector<HTMLElement>("button")?.focus();
});
return () => cancelAnimationFrame(raf);
}, [open, mounted]);
// Keyboard: Escape closes, Tab is trapped
useEffect(() => {
if (!open) return;
const handler = (e: KeyboardEvent) => {
if (e.key === "Escape") {
onClose();
return;
}
if (e.key === "Tab" && dialogRef.current) {
const focusable = Array.from(
dialogRef.current.querySelectorAll<HTMLElement>(
'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
)
).filter((el) => !el.hasAttribute("disabled"));
if (focusable.length === 0) {
e.preventDefault();
return;
}
const first = focusable[0];
const last = focusable[focusable.length - 1];
if (e.shiftKey) {
if (document.activeElement === first) {
e.preventDefault();
last.focus();
}
} else {
if (document.activeElement === last) {
e.preventDefault();
first.focus();
}
}
}
};
window.addEventListener("keydown", handler);
return () => window.removeEventListener("keydown", handler);
}, [open, onClose]);
if (!open || !mounted) return null;
return createPortal(
<div className="fixed inset-0 z-[9999] flex items-center justify-center">
{/* Backdrop */}
<div
className="absolute inset-0 bg-black/60 backdrop-blur-sm"
onClick={onClose}
/>
{/* Dialog */}
<div
ref={dialogRef}
role="dialog"
aria-modal="true"
aria-labelledby="keyboard-shortcuts-title"
className="relative bg-surface border border-line rounded-xl shadow-2xl shadow-black/60 max-w-[480px] w-full mx-4 overflow-hidden max-h-[80vh] flex flex-col"
>
{/* Header */}
<div className="flex items-center justify-between px-5 py-4 border-b border-line shrink-0">
<h2
id="keyboard-shortcuts-title"
className="text-sm font-semibold text-ink"
>
Keyboard Shortcuts
</h2>
<button
type="button"
onClick={onClose}
aria-label="Close keyboard shortcuts"
className="w-7 h-7 flex items-center justify-center rounded-lg text-ink-mid hover:text-ink hover:bg-surface-sunken transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-accent/40"
>
×
</button>
</div>
{/* Content */}
<div className="overflow-y-auto p-5 space-y-5">
{SHORTCUT_GROUPS.map((group) => (
<div key={group.title}>
<h3 className="text-[10px] font-semibold uppercase tracking-[0.2em] text-ink-soft mb-2.5">
{group.title}
</h3>
<div className="space-y-2">
{group.shortcuts.map((shortcut, i) => (
<div
key={i}
className="flex items-center justify-between gap-4"
>
<span className="text-[13px] text-ink-mid">
{shortcut.description}
</span>
<kbd className="flex items-center gap-0.5 shrink-0">
{shortcut.keys.map((k, j) => (
<span key={j} className="flex items-center gap-0.5">
{j > 0 && (
<span className="text-[9px] text-ink-soft mx-0.5">
+
</span>
)}
<span className="inline-flex items-center rounded-md border border-line/70 bg-surface-sunken/70 px-2 py-0.5 text-[11px] font-medium text-ink tabular-nums font-mono">
{k}
</span>
</span>
))}
</kbd>
</div>
))}
</div>
</div>
))}
</div>
{/* Footer */}
<div className="px-5 py-3 border-t border-line bg-surface-sunken/30 shrink-0">
<p className="text-[10px] text-ink-soft text-center">
Press{" "}
<kbd className="inline-flex items-center rounded border border-line/70 bg-surface-sunken/70 px-1.5 py-0.5 text-[10px] font-medium text-ink font-mono">
Esc
</kbd>{" "}
to close
</p>
</div>
</div>
</div>,
document.body
);
}

View File

@ -9,7 +9,6 @@ import { ConfirmDialog } from "@/components/ConfirmDialog";
import { showToast } from "@/components/Toaster"; import { showToast } from "@/components/Toaster";
import { ThemeToggle } from "@/components/ThemeToggle"; import { ThemeToggle } from "@/components/ThemeToggle";
import { statusDotClass } from "@/lib/design-tokens"; import { statusDotClass } from "@/lib/design-tokens";
import { KeyboardShortcutsDialog } from "@/components/KeyboardShortcutsDialog";
export function Toolbar() { export function Toolbar() {
const nodes = useCanvasStore((s) => s.nodes); const nodes = useCanvasStore((s) => s.nodes);
@ -34,7 +33,6 @@ export function Toolbar() {
const [restartingAll, setRestartingAll] = useState(false); const [restartingAll, setRestartingAll] = useState(false);
const [restartConfirmOpen, setRestartConfirmOpen] = useState(false); const [restartConfirmOpen, setRestartConfirmOpen] = useState(false);
const [helpOpen, setHelpOpen] = useState(false); const [helpOpen, setHelpOpen] = useState(false);
const [shortcutsOpen, setShortcutsOpen] = useState(false);
const helpRef = useRef<HTMLDivElement>(null); const helpRef = useRef<HTMLDivElement>(null);
// Suppress toast on the very first connect at page load; only fire on reconnects. // Suppress toast on the very first connect at page load; only fire on reconnects.
@ -129,29 +127,6 @@ export function Toolbar() {
}; };
}, []); }, []);
// Global ? shortcut opens the shortcuts dialog (mirrors the help button).
// Skip when the user is typing in an input so ? in a text field doesn't
// steal focus. Also skip when a modal/dialog is already open.
useEffect(() => {
const handler = (e: KeyboardEvent) => {
if (e.key !== "?") return;
const tag = (e.target as HTMLElement).tagName;
const inInput =
tag === "INPUT" ||
tag === "TEXTAREA" ||
tag === "SELECT" ||
(e.target as HTMLElement).isContentEditable;
if (inInput) return;
// Don't fire when a modal/dialog is already mounted (canvas modals,
// side panel, etc. use z-50 or above).
if (document.querySelector('[role="dialog"][aria-modal="true"]')) return;
e.preventDefault();
setShortcutsOpen(true);
};
window.addEventListener("keydown", handler);
return () => window.removeEventListener("keydown", handler);
}, []);
return ( return (
<div <div
className="fixed top-3 left-1/2 -translate-x-1/2 z-20 flex items-center gap-3 bg-surface-sunken/80 backdrop-blur-md border border-line/60 rounded-xl px-4 py-2 shadow-xl shadow-black/20 transition-[margin-left] duration-200" className="fixed top-3 left-1/2 -translate-x-1/2 z-20 flex items-center gap-3 bg-surface-sunken/80 backdrop-blur-md border border-line/60 rounded-xl px-4 py-2 shadow-xl shadow-black/20 transition-[margin-left] duration-200"
@ -346,14 +321,6 @@ export function Toolbar() {
<HelpRow shortcut="Config" text="Use the Config tab for skills, model, secrets, and runtime settings." /> <HelpRow shortcut="Config" text="Use the Config tab for skills, model, secrets, and runtime settings." />
<HelpRow shortcut="Dbl-click / Z" text="Zoom canvas to fit a team node and all its sub-workspaces." /> <HelpRow shortcut="Dbl-click / Z" text="Zoom canvas to fit a team node and all its sub-workspaces." />
</div> </div>
{/* Link to the full keyboard shortcuts dialog */}
<button
type="button"
onClick={() => { setHelpOpen(false); setShortcutsOpen(true); }}
className="mt-3 w-full text-center text-[10px] text-ink-soft hover:text-accent transition-colors focus:outline-none focus-visible:underline"
>
See all shortcuts
</button>
</div> </div>
)} )}
</div> </div>
@ -373,11 +340,6 @@ export function Toolbar() {
onConfirm={restartAll} onConfirm={restartAll}
onCancel={() => setRestartConfirmOpen(false)} onCancel={() => setRestartConfirmOpen(false)}
/> />
<KeyboardShortcutsDialog
open={shortcutsOpen}
onClose={() => setShortcutsOpen(false)}
/>
</div> </div>
); );
} }

View File

@ -1,6 +1,6 @@
"use client"; "use client";
import { useCallback, useMemo, type KeyboardEvent } from "react"; import { useCallback, useMemo } from "react";
import { Handle, NodeResizer, Position, type NodeProps, type Node } from "@xyflow/react"; import { Handle, NodeResizer, Position, type NodeProps, type Node } from "@xyflow/react";
import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas"; import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas";
import { getConfigurationError, getConfigurationStatus } from "@/store/canvas-topology"; import { getConfigurationError, getConfigurationStatus } from "@/store/canvas-topology";
@ -191,23 +191,7 @@ export function WorkspaceNode({ id, data }: NodeProps<Node<WorkspaceNodeData>>)
<Handle <Handle
type="target" type="target"
position={Position.Top} position={Position.Top}
tabIndex={0} className="!w-2.5 !h-1 !rounded-full !bg-surface-card/80 !border-0 !-top-0.5 hover:!bg-blue-400 hover:!h-1.5 transition-all"
role="button"
aria-label={`Extract ${data.name} from its parent (Enter or Space)`}
onKeyDown={(e: KeyboardEvent<HTMLDivElement>) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
e.stopPropagation();
// Keyboard accessibility for edge anchors: pressing Enter/Space on
// the top handle extracts this node from its current parent,
// moving it to the root level. Mirrors the Figma/Excalidraw
// pattern of using the connector dot as a keyboard affordance.
if (data.parentId) {
void nestNode(id, null);
}
}
}}
className="!w-2.5 !h-1 !rounded-full !bg-surface-card/80 !border-0 !-top-0.5 hover:!bg-blue-400 hover:!h-1.5 focus-visible:!bg-blue-400 focus-visible:!h-1.5 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-400/60 focus-visible:ring-offset-1 focus-visible:ring-offset-zinc-950 transition-all"
/> />
<div className="relative px-3.5 py-2.5"> <div className="relative px-3.5 py-2.5">
@ -374,23 +358,7 @@ export function WorkspaceNode({ id, data }: NodeProps<Node<WorkspaceNodeData>>)
<Handle <Handle
type="source" type="source"
position={Position.Bottom} position={Position.Bottom}
tabIndex={0} className="!w-2.5 !h-1 !rounded-full !bg-surface-card/80 !border-0 !-bottom-0.5 hover:!bg-blue-400 hover:!h-1.5 transition-all"
role="button"
aria-label={`Nest selected workspace inside ${data.name} (Enter or Space)`}
onKeyDown={(e: KeyboardEvent<HTMLDivElement>) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
e.stopPropagation();
// Keyboard accessibility for edge anchors: pressing Enter/Space on
// the bottom handle nests the currently-selected node as a child
// of this node. Requires another node to be selected first.
const selected = selectedNodeId;
if (selected && selected !== id) {
void nestNode(selected, id);
}
}
}}
className="!w-2.5 !h-1 !rounded-full !bg-surface-card/80 !border-0 !-bottom-0.5 hover:!bg-blue-400 hover:!h-1.5 focus-visible:!bg-blue-400 focus-visible:!h-1.5 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-400/60 focus-visible:ring-offset-1 focus-visible:ring-offset-zinc-950 transition-all"
/> />
</div> </div>
</> </>

View File

@ -1,309 +0,0 @@
// @vitest-environment jsdom
/**
* Tests for canvas keyboard shortcuts (useKeyboardShortcuts hook).
*
* Covers: Esc, Enter/Shift+Enter, Cmd+]/[, Z, and Arrow keys.
*
* The hook is tested by dispatching KeyboardEvents at the window and
* asserting the resulting store mutations / dispatched events.
*/
import React from "react";
import { render, cleanup, fireEvent } from "@testing-library/react";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { useKeyboardShortcuts } from "../useKeyboardShortcuts";
import { useCanvasStore } from "@/store/canvas";
// ─── Mock store ──────────────────────────────────────────────────────────────
const mockSavePosition = vi.fn().mockResolvedValue(undefined);
vi.mock("@/store/canvas", () => ({
useCanvasStore: Object.assign(
vi.fn((sel) => sel(mockStoreState)),
{
getState: () => mockStoreState,
}
),
}));
// Module-level mutable state so tests can mutate between cases
const mockStoreState = {
selectedNodeId: null as string | null,
selectedNodeIds: new Set<string>(),
nodes: [] as Array<{ id: string; position: { x: number; y: number }; data: { parentId?: string | null } }>,
contextMenu: null as { x: number; y: number; nodeId: string } | null,
closeContextMenu: vi.fn(),
selectNode: vi.fn(),
clearSelection: vi.fn(),
bumpZOrder: vi.fn(),
savePosition: mockSavePosition,
moveNode: vi.fn(),
};
afterEach(() => {
cleanup();
vi.clearAllMocks();
// Reset to default empty state between tests
mockStoreState.selectedNodeId = null;
mockStoreState.selectedNodeIds = new Set();
mockStoreState.nodes = [];
mockStoreState.contextMenu = null;
mockStoreState.closeContextMenu.mockClear();
mockStoreState.selectNode.mockClear();
mockStoreState.clearSelection.mockClear();
mockStoreState.bumpZOrder.mockClear();
mockStoreState.moveNode.mockClear();
mockStoreState.savePosition.mockClear();
});
// ─── Test wrapper ────────────────────────────────────────────────────────────
function ShortcutTestComponent() {
useKeyboardShortcuts();
return <div data-testid="canvas-root" />;
}
function renderWithProvider() {
return render(<ShortcutTestComponent />);
}
// ─── Tests ───────────────────────────────────────────────────────────────────
describe("Esc — deselect / close context menu", () => {
it("closes the context menu when one is open", () => {
mockStoreState.contextMenu = { x: 100, y: 100, nodeId: "n1" };
renderWithProvider();
fireEvent.keyDown(window, { key: "Escape" });
expect(mockStoreState.closeContextMenu).toHaveBeenCalledTimes(1);
});
it("clears the batch selection when no context menu is open", () => {
mockStoreState.contextMenu = null;
mockStoreState.selectedNodeIds = new Set(["n1", "n2"]);
renderWithProvider();
fireEvent.keyDown(window, { key: "Escape" });
expect(mockStoreState.clearSelection).toHaveBeenCalledTimes(1);
});
it("deselects the focused node when no batch selection exists", () => {
mockStoreState.contextMenu = null;
mockStoreState.selectedNodeIds = new Set();
mockStoreState.selectedNodeId = "n1";
renderWithProvider();
fireEvent.keyDown(window, { key: "Escape" });
expect(mockStoreState.selectNode).toHaveBeenCalledWith(null);
});
});
describe("Enter — hierarchy navigation", () => {
beforeEach(() => {
mockStoreState.selectedNodeId = "n1";
mockStoreState.nodes = [
{ id: "n1", position: { x: 0, y: 0 }, data: { parentId: null } },
{ id: "n2", position: { x: 100, y: 0 }, data: { parentId: "n1" } },
{ id: "n3", position: { x: 200, y: 0 }, data: { parentId: null } },
];
});
it("navigates to the first child on Enter", () => {
renderWithProvider();
fireEvent.keyDown(window, { key: "Enter" });
expect(mockStoreState.selectNode).toHaveBeenCalledWith("n2");
});
it("navigates to the parent on Shift+Enter", () => {
mockStoreState.nodes = [
{ id: "n1", position: { x: 0, y: 0 }, data: { parentId: null } },
{ id: "n2", position: { x: 100, y: 0 }, data: { parentId: "n1" } },
];
mockStoreState.selectedNodeId = "n2";
renderWithProvider();
fireEvent.keyDown(window, { key: "Enter", shiftKey: true });
expect(mockStoreState.selectNode).toHaveBeenCalledWith("n1");
});
it("does NOT navigate when no node is selected", () => {
mockStoreState.selectedNodeId = null;
renderWithProvider();
fireEvent.keyDown(window, { key: "Enter" });
expect(mockStoreState.selectNode).not.toHaveBeenCalled();
});
});
describe("Cmd+]/[ — z-order bump", () => {
beforeEach(() => {
mockStoreState.selectedNodeId = "n1";
});
it("bumps z-order forward on Cmd+]", () => {
renderWithProvider();
fireEvent.keyDown(window, { key: "]", metaKey: true });
expect(mockStoreState.bumpZOrder).toHaveBeenCalledWith("n1", 1);
});
it("bumps z-order backward on Cmd+[", () => {
renderWithProvider();
fireEvent.keyDown(window, { key: "[", metaKey: true });
expect(mockStoreState.bumpZOrder).toHaveBeenCalledWith("n1", -1);
});
it("uses Ctrl as the modifier key", () => {
renderWithProvider();
fireEvent.keyDown(window, { key: "]", ctrlKey: true });
expect(mockStoreState.bumpZOrder).toHaveBeenCalledWith("n1", 1);
});
});
describe("Z — zoom-to-team", () => {
let dispatchedEvents: CustomEvent[] = [];
beforeEach(() => {
dispatchedEvents = [];
mockStoreState.selectedNodeId = "n1";
mockStoreState.nodes = [
{ id: "n1", position: { x: 0, y: 0 }, data: { parentId: null } },
{ id: "n2", position: { x: 100, y: 0 }, data: { parentId: "n1" } },
];
window.addEventListener("molecule:zoom-to-team", (e) => {
dispatchedEvents.push(e as CustomEvent);
});
});
afterEach(() => {
window.removeEventListener("molecule:zoom-to-team", () => {});
});
it("dispatches zoom-to-team when the selected node has children", () => {
renderWithProvider();
fireEvent.keyDown(window, { key: "z" });
expect(dispatchedEvents).toHaveLength(1);
expect(dispatchedEvents[0].detail.nodeId).toBe("n1");
});
it("does NOT fire when no node is selected", () => {
mockStoreState.selectedNodeId = null;
renderWithProvider();
fireEvent.keyDown(window, { key: "z" });
expect(dispatchedEvents).toHaveLength(0);
});
it("does NOT fire when the node has no children", () => {
mockStoreState.nodes = [
{ id: "n1", position: { x: 0, y: 0 }, data: { parentId: null } },
];
renderWithProvider();
fireEvent.keyDown(window, { key: "z" });
expect(dispatchedEvents).toHaveLength(0);
});
it("skips when the target element is an input", () => {
renderWithProvider();
const input = document.createElement("input");
document.body.appendChild(input);
fireEvent.keyDown(input, { key: "z" });
expect(dispatchedEvents).toHaveLength(0);
document.body.removeChild(input);
});
});
describe("Arrow keys — keyboard node movement", () => {
beforeEach(() => {
mockStoreState.selectedNodeId = "n1";
mockStoreState.nodes = [
{ id: "n1", position: { x: 100, y: 200 }, data: { parentId: null } },
];
});
it("moves the selected node down on ArrowDown", () => {
renderWithProvider();
fireEvent.keyDown(window, { key: "ArrowDown" });
expect(mockStoreState.moveNode).toHaveBeenCalledWith("n1", 0, 10);
});
it("moves the selected node up on ArrowUp", () => {
renderWithProvider();
fireEvent.keyDown(window, { key: "ArrowUp" });
expect(mockStoreState.moveNode).toHaveBeenCalledWith("n1", 0, -10);
});
it("moves the selected node right on ArrowRight", () => {
renderWithProvider();
fireEvent.keyDown(window, { key: "ArrowRight" });
expect(mockStoreState.moveNode).toHaveBeenCalledWith("n1", 10, 0);
});
it("moves the selected node left on ArrowLeft", () => {
renderWithProvider();
fireEvent.keyDown(window, { key: "ArrowLeft" });
expect(mockStoreState.moveNode).toHaveBeenCalledWith("n1", -10, 0);
});
it("moves 50 px when Shift is held", () => {
renderWithProvider();
fireEvent.keyDown(window, { key: "ArrowDown", shiftKey: true });
expect(mockStoreState.moveNode).toHaveBeenCalledWith("n1", 0, 50);
});
it("does NOT fire when no node is selected", () => {
mockStoreState.selectedNodeId = null;
renderWithProvider();
fireEvent.keyDown(window, { key: "ArrowDown" });
expect(mockStoreState.moveNode).not.toHaveBeenCalled();
});
it("skips when the target element is an input", () => {
renderWithProvider();
const input = document.createElement("input");
document.body.appendChild(input);
fireEvent.keyDown(input, { key: "ArrowDown" });
expect(mockStoreState.moveNode).not.toHaveBeenCalled();
document.body.removeChild(input);
});
it("skips when a modal dialog is already open", () => {
renderWithProvider();
const dialog = document.createElement("div");
dialog.setAttribute("role", "dialog");
dialog.setAttribute("aria-modal", "true");
document.body.appendChild(dialog);
fireEvent.keyDown(window, { key: "ArrowDown" });
expect(mockStoreState.moveNode).not.toHaveBeenCalled();
document.body.removeChild(dialog);
});
it("prevents default browser scroll on arrow keys", () => {
renderWithProvider();
const preventDefault = vi.fn();
fireEvent.keyDown(window, {
key: "ArrowDown",
preventDefault,
});
expect(preventDefault).toHaveBeenCalled();
});
});
describe("all shortcuts respect inInput guard", () => {
it("ArrowDown is skipped in an input element", () => {
mockStoreState.selectedNodeId = "n1";
renderWithProvider();
const textarea = document.createElement("textarea");
document.body.appendChild(textarea);
fireEvent.keyDown(textarea, { key: "ArrowDown" });
expect(mockStoreState.moveNode).not.toHaveBeenCalled();
document.body.removeChild(textarea);
});
it("Enter navigation is skipped in an input element", () => {
mockStoreState.selectedNodeId = "n1";
mockStoreState.nodes = [
{ id: "n1", position: { x: 0, y: 0 }, data: { parentId: null } },
{ id: "n2", position: { x: 100, y: 0 }, data: { parentId: "n1" } },
];
renderWithProvider();
const input = document.createElement("input");
document.body.appendChild(input);
fireEvent.keyDown(input, { key: "Enter" });
expect(mockStoreState.selectNode).not.toHaveBeenCalled();
document.body.removeChild(input);
});
});

View File

@ -14,7 +14,6 @@ import { useCanvasStore } from "@/store/canvas";
* Cmd/Ctrl+] bump selected node forward in z-order * Cmd/Ctrl+] bump selected node forward in z-order
* Cmd/Ctrl+[ bump selected node backward in z-order * Cmd/Ctrl+[ bump selected node backward in z-order
* Z zoom-to-team if the selected node has children * Z zoom-to-team if the selected node has children
* Arrow keys move selected node 10px (50px with Shift)
*/ */
export function useKeyboardShortcuts() { export function useKeyboardShortcuts() {
useEffect(() => { useEffect(() => {
@ -81,33 +80,6 @@ export function useKeyboardShortcuts() {
); );
} }
} }
// Arrow-key node movement — Figma-style keyboard drag for keyboard users.
// 10 px per press, 50 px with Shift held. Only fires when a node
// is selected and the target isn't a form control.
if (
!inInput &&
(e.key === "ArrowUp" ||
e.key === "ArrowDown" ||
e.key === "ArrowLeft" ||
e.key === "ArrowRight")
) {
const state = useCanvasStore.getState();
const selectedId = state.selectedNodeId;
if (!selectedId) return;
// Skip when a modal/dialog is already open — dialogs own their own
// arrow-key semantics and shouldn't trigger canvas moves.
if (document.querySelector('[role="dialog"][aria-modal="true"]')) return;
e.preventDefault();
const step = e.shiftKey ? 50 : 10;
let dx = 0;
let dy = 0;
if (e.key === "ArrowUp") dy = -step;
else if (e.key === "ArrowDown") dy = step;
else if (e.key === "ArrowLeft") dx = -step;
else dx = step;
state.moveNode(selectedId, dx, dy);
}
}; };
window.addEventListener("keydown", handler); window.addEventListener("keydown", handler);
return () => window.removeEventListener("keydown", handler); return () => window.removeEventListener("keydown", handler);

View File

@ -513,20 +513,7 @@ function GroupedCommsView({
/> />
<div className="flex-1 overflow-y-auto p-3 space-y-2"> <div className="flex-1 overflow-y-auto p-3 space-y-2">
{visible.map((msg) => {visible.map((msg) =>
// Only render the error UI when there is NO usable response msg.status === "error" ? (
// content. A "error" status from the platform means the HTTP
// transport layer had a problem — but the agent response text
// may have arrived and been stored in response_body.text.
// Delegation results set responseText via extractResponseText
// once that function learned to parse body.text, so checking
// !msg.responseText here correctly identifies "no actual reply
// was received" vs. "reply arrived but status=error".
//
// Without this guard, successful delegation results were
// rendered as error banners, PMs saw "restart" prompts and
// restarted working agents, and retry storms formed as the
// platform re-delivered the same completed work (issue #159).
msg.status === "error" && !msg.responseText ? (
<ErrorMessage key={msg.id} msg={msg} /> <ErrorMessage key={msg.id} msg={msg} />
) : ( ) : (
<NormalMessage key={msg.id} msg={msg} /> <NormalMessage key={msg.id} msg={msg} />

View File

@ -9,7 +9,6 @@
// AttachmentLightbox). // AttachmentLightbox).
import { useState, useEffect, useRef } from "react"; import { useState, useEffect, useRef } from "react";
import { platformAuthHeaders } from "@/lib/api";
import type { ChatAttachment } from "./types"; import type { ChatAttachment } from "./types";
import { isPlatformAttachment, resolveAttachmentHref } from "./uploads"; import { isPlatformAttachment, resolveAttachmentHref } from "./uploads";
import { AttachmentChip } from "./AttachmentViews"; import { AttachmentChip } from "./AttachmentViews";
@ -44,8 +43,13 @@ export function AttachmentAudio({ workspaceId, attachment, onDownload, tone }: P
void (async () => { void (async () => {
try { try {
const href = resolveAttachmentHref(workspaceId, attachment.uri); const href = resolveAttachmentHref(workspaceId, attachment.uri);
const headers: Record<string, string> = {};
const adminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN;
if (adminToken) headers["Authorization"] = `Bearer ${adminToken}`;
const slug = getTenantSlug();
if (slug) headers["X-Molecule-Org-Slug"] = slug;
const res = await fetch(href, { const res = await fetch(href, {
headers: platformAuthHeaders(), headers,
credentials: "include", credentials: "include",
signal: AbortSignal.timeout(60_000), signal: AbortSignal.timeout(60_000),
}); });
@ -112,5 +116,9 @@ export function AttachmentAudio({ workspaceId, attachment, onDownload, tone }: P
); );
} }
// Local getTenantSlug() removed — auth-header construction now goes function getTenantSlug(): string | null {
// through platformAuthHeaders() from @/lib/api (#178). if (typeof window === "undefined") return null;
const host = window.location.hostname;
const m = host.match(/^([^.]+)\.moleculesai\.app$/);
return m ? m[1] : null;
}

View File

@ -35,7 +35,6 @@
// downscale via canvas, but defer that to v2. // downscale via canvas, but defer that to v2.
import { useState, useEffect, useRef } from "react"; import { useState, useEffect, useRef } from "react";
import { platformAuthHeaders } from "@/lib/api";
import type { ChatAttachment } from "./types"; import type { ChatAttachment } from "./types";
import { isPlatformAttachment, resolveAttachmentHref } from "./uploads"; import { isPlatformAttachment, resolveAttachmentHref } from "./uploads";
import { AttachmentLightbox } from "./AttachmentLightbox"; import { AttachmentLightbox } from "./AttachmentLightbox";
@ -76,14 +75,22 @@ export function AttachmentImage({ workspaceId, attachment, onDownload, tone }: P
} }
// Platform-auth path: identical to downloadChatFile but we keep // Platform-auth path: identical to downloadChatFile but we keep
// the blob (don't trigger a Save-As). Auth headers come from the // the blob (don't trigger a Save-As). Use the same headers it does
// shared `platformAuthHeaders()` helper — one source of truth for // by going through it indirectly — no, downloadChatFile triggers a
// every authenticated raw fetch in the canvas (#178). // Save-As. Need a separate fetch.
void (async () => { void (async () => {
try { try {
const href = resolveAttachmentHref(workspaceId, attachment.uri); const href = resolveAttachmentHref(workspaceId, attachment.uri);
const headers: Record<string, string> = {};
// Read the same env var downloadChatFile reads — single source
// of truth would be cleaner; refactor opportunity for PR-2 if
// we add the same path to AttachmentVideo.
const adminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN;
if (adminToken) headers["Authorization"] = `Bearer ${adminToken}`;
const slug = getTenantSlug();
if (slug) headers["X-Molecule-Org-Slug"] = slug;
const res = await fetch(href, { const res = await fetch(href, {
headers: platformAuthHeaders(), headers,
credentials: "include", credentials: "include",
signal: AbortSignal.timeout(30_000), signal: AbortSignal.timeout(30_000),
}); });
@ -177,7 +184,15 @@ export function AttachmentImage({ workspaceId, attachment, onDownload, tone }: P
); );
} }
// Local getTenantSlug() removed — auth-header construction now goes // Internal helper — duplicated from uploads.ts (it's not exported
// through platformAuthHeaders() from @/lib/api which uses the canonical // there). Kept local so this component doesn't reach into private
// getTenantSlug() from @/lib/tenant. This eliminates the duplicate // surface; if AttachmentVideo / AttachmentPDF in PR-2/PR-3 also need
// hostname-regex + the duplicate bearer-token-attach pattern (#178). // it, lift to an exported helper at that point (the third-caller
// rule).
function getTenantSlug(): string | null {
if (typeof window === "undefined") return null;
const host = window.location.hostname;
// Tenant subdomain shape: <slug>.moleculesai.app
const m = host.match(/^([^.]+)\.moleculesai\.app$/);
return m ? m[1] : null;
}

View File

@ -33,7 +33,6 @@
// timeout, swap to chip. Implemented as a 3-second watchdog. // timeout, swap to chip. Implemented as a 3-second watchdog.
import { useState, useEffect, useRef } from "react"; import { useState, useEffect, useRef } from "react";
import { platformAuthHeaders } from "@/lib/api";
import type { ChatAttachment } from "./types"; import type { ChatAttachment } from "./types";
import { isPlatformAttachment, resolveAttachmentHref } from "./uploads"; import { isPlatformAttachment, resolveAttachmentHref } from "./uploads";
import { AttachmentLightbox } from "./AttachmentLightbox"; import { AttachmentLightbox } from "./AttachmentLightbox";
@ -70,8 +69,13 @@ export function AttachmentPDF({ workspaceId, attachment, onDownload, tone }: Pro
void (async () => { void (async () => {
try { try {
const href = resolveAttachmentHref(workspaceId, attachment.uri); const href = resolveAttachmentHref(workspaceId, attachment.uri);
const headers: Record<string, string> = {};
const adminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN;
if (adminToken) headers["Authorization"] = `Bearer ${adminToken}`;
const slug = getTenantSlug();
if (slug) headers["X-Molecule-Org-Slug"] = slug;
const res = await fetch(href, { const res = await fetch(href, {
headers: platformAuthHeaders(), headers,
credentials: "include", credentials: "include",
signal: AbortSignal.timeout(60_000), signal: AbortSignal.timeout(60_000),
}); });
@ -185,5 +189,9 @@ function PdfGlyph() {
); );
} }
// Local getTenantSlug() removed — auth-header construction now goes function getTenantSlug(): string | null {
// through platformAuthHeaders() from @/lib/api (#178). if (typeof window === "undefined") return null;
const host = window.location.hostname;
const m = host.match(/^([^.]+)\.moleculesai\.app$/);
return m ? m[1] : null;
}

View File

@ -26,7 +26,6 @@
// to download the full file. // to download the full file.
import { useState, useEffect } from "react"; import { useState, useEffect } from "react";
import { platformAuthHeaders } from "@/lib/api";
import type { ChatAttachment } from "./types"; import type { ChatAttachment } from "./types";
import { isPlatformAttachment, resolveAttachmentHref } from "./uploads"; import { isPlatformAttachment, resolveAttachmentHref } from "./uploads";
import { AttachmentChip } from "./AttachmentViews"; import { AttachmentChip } from "./AttachmentViews";
@ -58,13 +57,13 @@ export function AttachmentTextPreview({ workspaceId, attachment, onDownload, ton
void (async () => { void (async () => {
try { try {
const href = resolveAttachmentHref(workspaceId, attachment.uri); const href = resolveAttachmentHref(workspaceId, attachment.uri);
// Only attach platform auth headers for in-platform URIs — const headers: Record<string, string> = {};
// off-platform URLs (HTTP/HTTPS attachments) MUST NOT receive if (isPlatformAttachment(attachment.uri)) {
// our bearer token (it would leak the admin token to a third const adminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN;
// party). The branch is preserved with the new shared helper. if (adminToken) headers["Authorization"] = `Bearer ${adminToken}`;
const headers: Record<string, string> = isPlatformAttachment(attachment.uri) const slug = getTenantSlug();
? platformAuthHeaders() if (slug) headers["X-Molecule-Org-Slug"] = slug;
: {}; }
const res = await fetch(href, { const res = await fetch(href, {
headers, headers,
credentials: "include", credentials: "include",
@ -183,5 +182,9 @@ export function AttachmentTextPreview({ workspaceId, attachment, onDownload, ton
); );
} }
// Local getTenantSlug() removed — auth-header construction now goes function getTenantSlug(): string | null {
// through platformAuthHeaders() from @/lib/api (#178). if (typeof window === "undefined") return null;
const host = window.location.hostname;
const m = host.match(/^([^.]+)\.moleculesai\.app$/);
return m ? m[1] : null;
}

View File

@ -25,7 +25,6 @@
// fetch via service worker. v2 if measured-needed. // fetch via service worker. v2 if measured-needed.
import { useState, useEffect, useRef } from "react"; import { useState, useEffect, useRef } from "react";
import { platformAuthHeaders } from "@/lib/api";
import type { ChatAttachment } from "./types"; import type { ChatAttachment } from "./types";
import { isPlatformAttachment, resolveAttachmentHref } from "./uploads"; import { isPlatformAttachment, resolveAttachmentHref } from "./uploads";
import { AttachmentChip } from "./AttachmentViews"; import { AttachmentChip } from "./AttachmentViews";
@ -62,8 +61,13 @@ export function AttachmentVideo({ workspaceId, attachment, onDownload, tone }: P
void (async () => { void (async () => {
try { try {
const href = resolveAttachmentHref(workspaceId, attachment.uri); const href = resolveAttachmentHref(workspaceId, attachment.uri);
const headers: Record<string, string> = {};
const adminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN;
if (adminToken) headers["Authorization"] = `Bearer ${adminToken}`;
const slug = getTenantSlug();
if (slug) headers["X-Molecule-Org-Slug"] = slug;
const res = await fetch(href, { const res = await fetch(href, {
headers: platformAuthHeaders(), headers,
credentials: "include", credentials: "include",
// Videos are larger than images on average; give the request // Videos are larger than images on average; give the request
// more headroom. The server's per-request body cap (50MB) is // more headroom. The server's per-request body cap (50MB) is
@ -143,5 +147,11 @@ export function AttachmentVideo({ workspaceId, attachment, onDownload, tone }: P
); );
} }
// Local getTenantSlug() removed — auth-header construction now goes // Internal helper — same shape as AttachmentImage's. Lifted to a
// through platformAuthHeaders() from @/lib/api (#178). // shared util in PR-2.5 if a third caller needs it (PDF, audio).
function getTenantSlug(): string | null {
if (typeof window === "undefined") return null;
const host = window.location.hostname;
const m = host.match(/^([^.]+)\.moleculesai\.app$/);
return m ? m[1] : null;
}

View File

@ -4,11 +4,9 @@ import { render, screen, fireEvent, waitFor } from "@testing-library/react";
// API mock — tests can override per case via apiGetMock.mockImplementationOnce. // API mock — tests can override per case via apiGetMock.mockImplementationOnce.
const apiGetMock = vi.fn<(url: string) => Promise<unknown>>(); const apiGetMock = vi.fn<(url: string) => Promise<unknown>>();
const apiPostMock = vi.fn<(url: string, body?: unknown) => Promise<unknown>>();
vi.mock("@/lib/api", () => ({ vi.mock("@/lib/api", () => ({
api: { api: {
get: (url: string) => apiGetMock(url), get: (url: string) => apiGetMock(url),
post: (url: string, body?: unknown) => apiPostMock(url, body),
}, },
})); }));
@ -18,23 +16,17 @@ vi.mock("@/hooks/useSocketEvent", () => ({
useSocketEvent: () => {}, useSocketEvent: () => {},
})); }));
// Canvas store — peer name resolution + ErrorMessage requires selectNode // Canvas store — peer name resolution.
// (Zustand hook usage). The mock must support BOTH: vi.mock("@/store/canvas", () => ({
// useCanvasStore.getState().nodes (plain object with getState) useCanvasStore: {
// useCanvasStore((s) => s.selectNode) (Zustand hook with selector) getState: () => ({
vi.mock("@/store/canvas", () => { nodes: [
const state = { { id: "ws-self", data: { name: "Self" } },
nodes: [ { id: "ws-peer", data: { name: "Peer Agent" } },
{ id: "ws-self", data: { name: "Self" } }, ],
{ id: "ws-peer", data: { name: "Peer Agent" } }, }),
], },
selectNode: vi.fn(), }));
};
const hook = (selector?: (s: typeof state) => unknown) =>
selector ? selector(state) : state;
hook.getState = () => state;
return { useCanvasStore: hook };
});
// Toaster shim — AgentCommsPanel imports showToast. // Toaster shim — AgentCommsPanel imports showToast.
vi.mock("../../Toaster", () => ({ vi.mock("../../Toaster", () => ({
@ -49,8 +41,6 @@ import { AgentCommsPanel } from "../AgentCommsPanel";
const scrollSpy = vi.fn<(opts?: ScrollIntoViewOptions | boolean) => void>(); const scrollSpy = vi.fn<(opts?: ScrollIntoViewOptions | boolean) => void>();
beforeEach(() => { beforeEach(() => {
apiGetMock.mockReset(); apiGetMock.mockReset();
apiPostMock.mockReset();
apiPostMock.mockResolvedValue({});
scrollSpy.mockReset(); scrollSpy.mockReset();
Element.prototype.scrollIntoView = scrollSpy as unknown as Element["scrollIntoView"]; Element.prototype.scrollIntoView = scrollSpy as unknown as Element["scrollIntoView"];
}); });
@ -59,81 +49,6 @@ afterEach(() => {
vi.clearAllMocks(); vi.clearAllMocks();
}); });
// Regression test: when a delegation succeeds but the platform persisted
// status="error" (transport-layer HTTP failure, not agent failure), the
// canvas had the response text in msg.text but rendered ErrorMessage
// anyway, burying the real content in an "Underlying error" banner and
// prompting PMs to restart working agents (issue #159).
describe("AgentCommsPanel — error rendering guard (issue #159)", () => {
it("renders NormalMessage when status=error but msg.text is present (successful delegation)", async () => {
// Simulate a delegation result where status="error" (HTTP transport
// failed) but response_body.text carries the actual agent response.
// The correct behaviour: show the content as a normal inbound bubble,
// NOT an error banner.
apiGetMock.mockResolvedValueOnce([
{
id: "act-1",
activity_type: "delegation",
method: "delegate_result",
source_id: "ws-self",
target_id: "ws-peer",
summary: "Delegation completed",
request_body: null,
// delegation.go stores response_body as {text: "...", delegation_id: "..."}
response_body: {
text: "PR #149: tier-check fails NO REVIEWS (author needs engineers/managers/ceo approval)",
delegation_id: "delg_01jx8q4n3k",
},
status: "error", // transport-layer error, not agent failure
created_at: "2026-04-25T18:00:00Z",
},
]);
render(<AgentCommsPanel workspaceId="ws-self" />);
// The response text should appear in a normal inbound bubble, NOT in
// an error banner. Specifically: no "Failed to deliver" or "returned
// an error" text should appear.
await waitFor(() => {
expect(screen.queryByText(/failed to deliver/i)).toBeNull();
expect(screen.queryByText(/returned an error/i)).toBeNull();
});
// The actual content must be visible.
await waitFor(() =>
expect(
screen.getByText(/tier-check fails NO REVIEWS/i),
).toBeDefined(),
);
});
it("renders ErrorMessage when status=error and msg.text is absent (true failure)", async () => {
// True delivery failure: no response body, no text. The error banner
// IS appropriate here.
apiGetMock.mockResolvedValueOnce([
{
id: "act-1",
activity_type: "a2a_send",
source_id: "ws-self",
target_id: "ws-peer",
method: "message/send",
summary: "A2A send failed",
request_body: null,
response_body: null,
status: "error",
created_at: "2026-04-25T18:00:00Z",
},
]);
render(<AgentCommsPanel workspaceId="ws-self" />);
// Error banner IS shown for true failures (no content).
// jsdom doesn't reliably match role="alert" in getByRole, so use
// getByText instead.
const errorBanner = await waitFor(() =>
screen.getByText(/failed to deliver/i),
);
expect(errorBanner).toBeDefined();
});
});
describe("AgentCommsPanel — initial-state parity with ChatTab my-chat", () => { describe("AgentCommsPanel — initial-state parity with ChatTab my-chat", () => {
it("shows loading text while history fetch is in flight", () => { it("shows loading text while history fetch is in flight", () => {
apiGetMock.mockReturnValueOnce(new Promise(() => { /* never resolves */ })); apiGetMock.mockReturnValueOnce(new Promise(() => { /* never resolves */ }));

View File

@ -64,54 +64,6 @@ describe("extractRequestText", () => {
}; };
expect(extractRequestText(body)).toBe(""); expect(extractRequestText(body)).toBe("");
}); });
// Regression: delegation.go stores request_body as {"task": "...", "delegation_id": "..."}.
// extractRequestText was checking only the A2A params.message.parts path, so
// outbound delegation messages were rendered as blank bubbles.
// Fix: check body.task first (delegation format), then fall back to A2A.
it("extracts text from body.task (delegation format)", () => {
const body = {
task: "Deploy the staging environment for this sprint's release",
delegation_id: "delg_01jx8q4n3k",
};
expect(extractRequestText(body)).toBe(
"Deploy the staging environment for this sprint's release"
);
});
it("prefers body.task over A2A params when both present", () => {
const body = {
task: "Delegation text wins",
params: {
message: {
parts: [{ kind: "text", text: "A2A text" }],
},
},
};
// body.task is checked first; delegation wins for delegation activities.
expect(extractRequestText(body)).toBe("Delegation text wins");
});
it("falls back to A2A format when body.task is absent", () => {
const body = {
params: {
message: {
parts: [{ kind: "text", text: "A2A fallback" }],
},
},
};
expect(extractRequestText(body)).toBe("A2A fallback");
});
it("returns empty string when body.task is empty string", () => {
const body = { task: "" };
expect(extractRequestText(body)).toBe("");
});
it("returns empty string when body.task is not a string", () => {
const body = { task: 42 };
expect(extractRequestText(body)).toBe("");
});
}); });
describe("extractResponseText", () => { describe("extractResponseText", () => {
@ -209,43 +161,6 @@ describe("extractResponseText", () => {
}; };
expect(extractResponseText(body)).toBe("Summary\nDetail block one\nDetail block two"); expect(extractResponseText(body)).toBe("Summary\nDetail block one\nDetail block two");
}); });
// Regression: delegation.go stores response_body as
// {"text": "...", "delegation_id": "..."} — no "result" wrapper.
// Without body.text handling, extractResponseText returns "" for
// delegate_result rows, causing the error UI to fire even when the
// delegation succeeded (issue #159).
it("extracts from body.text (delegation response_body shape)", () => {
const body = {
text: "PR #149: tier-check fails NO REVIEWS (author needs engineers/managers/ceo approval)",
delegation_id: "delg_01jx8q4n3k",
};
expect(extractResponseText(body)).toBe(
"PR #149: tier-check fails NO REVIEWS (author needs engineers/managers/ceo approval)"
);
});
it("prefers body.result over body.text when both present", () => {
const body = {
result: { parts: [{ kind: "text", text: "A2A result wins" }] },
text: "Delegation text",
};
// result path is checked first; A2A wins when both present.
expect(extractResponseText(body)).toBe("A2A result wins");
});
it("returns empty string when body.text is empty string", () => {
expect(extractResponseText({ text: "" })).toBe("");
});
it("extracts from body.response_preview (DELEGATION_COMPLETE WS event shape)", () => {
const body = {
response_preview: "PR #149: tier-check fails NO REVIEWS (author needs engineers/managers/ceo approval)",
};
expect(extractResponseText(body)).toBe(
"PR #149: tier-check fails NO REVIEWS (author needs engineers/managers/ceo approval)"
);
});
}); });
describe("extractTextsFromParts", () => { describe("extractTextsFromParts", () => {

View File

@ -114,15 +114,9 @@ function basename(uri: string): string {
return slash >= 0 ? cleaned.slice(slash + 1) : cleaned || "file"; return slash >= 0 ? cleaned.slice(slash + 1) : cleaned || "file";
} }
/** Extract user message text from an activity log request_body. /** Extract user message text from an activity log request_body */
*
* Delegation activities from delegation.go store the task text directly
* at `body.task` as a plain string: {"task": "...", "delegation_id": "..."}.
* Check this first before falling back to the A2A JSON-RPC format
* (`body.params.message.parts[].text`). */
export function extractRequestText(body: Record<string, unknown> | null): string { export function extractRequestText(body: Record<string, unknown> | null): string {
if (!body) return ""; if (!body) return "";
if (typeof body.task === "string" && body.task) return body.task;
const params = body.params as Record<string, unknown> | undefined; const params = body.params as Record<string, unknown> | undefined;
const msg = params?.message as Record<string, unknown> | undefined; const msg = params?.message as Record<string, unknown> | undefined;
const parts = msg?.parts as Array<Record<string, unknown>> | undefined; const parts = msg?.parts as Array<Record<string, unknown>> | undefined;
@ -168,10 +162,10 @@ export function extractResponseText(body: Record<string, unknown>): string {
if (rootTexts.length > 0) collected.push(rootTexts.join("\n")); if (rootTexts.length > 0) collected.push(rootTexts.join("\n"));
// Task shape: {result: {artifacts: [{parts: [...]}]}} // Task shape: {result: {artifacts: [{parts: [...]}]}}
const artifacts = result.artifacts as Array<Record<string, unknown> | undefined>; const artifacts = result.artifacts as Array<Record<string, unknown>> | undefined;
if (artifacts) { if (artifacts) {
for (const a of artifacts) { for (const a of artifacts) {
const t = extractTextsFromParts(a?.parts); const t = extractTextsFromParts(a.parts);
if (t) collected.push(t); if (t) collected.push(t);
} }
} }
@ -179,20 +173,6 @@ export function extractResponseText(body: Record<string, unknown>): string {
if (collected.length > 0) return collected.join("\n"); if (collected.length > 0) return collected.join("\n");
} }
// Delegation results from delegation.go store response_body as
// {"text": "...", "delegation_id": "..."} — no "result" wrapper.
// Check this after the body.result path so A2A responses take
// precedence when both shapes are somehow present.
// Without this, responseText is always "" for delegate_result rows,
// causing the error UI to fire even when the delegation succeeded
// (issue #159).
if (typeof body.text === "string" && body.text) return body.text;
// DELEGATION_COMPLETE event (via canvas-events WS handler) stores
// response_body as {response_preview: "..."}. Handle this too.
if (typeof body.response_preview === "string" && body.response_preview) {
return body.response_preview;
}
// {task: "text"} — request body format, shouldn't be in response but handle it // {task: "text"} — request body format, shouldn't be in response but handle it
if (typeof body.task === "string") return body.task; if (typeof body.task === "string") return body.task;
} catch { /* ignore */ } } catch { /* ignore */ }

View File

@ -1,16 +1,12 @@
import { PLATFORM_URL, platformAuthHeaders } from "@/lib/api"; import { PLATFORM_URL } from "@/lib/api";
import { getTenantSlug } from "@/lib/tenant";
import type { ChatAttachment } from "./types"; import type { ChatAttachment } from "./types";
/** Chat attachments are intentionally uploaded via a direct fetch() /** Chat attachments are intentionally uploaded via a direct fetch()
* instead of the `api.post` helper `api.post` JSON-stringifies the * instead of the `api.post` helper `api.post` JSON-stringifies the
* body, which would 500 on a Blob. Auth headers (tenant slug, admin * body, which would 500 on a Blob. Mirrors the header plumbing
* token, credentials) come from `platformAuthHeaders()` the same * (tenant slug, admin token, credentials) so SaaS + self-hosted
* helper `request()` uses, so a missing bearer surfaces as a single * callers work the same way. */
* fix site instead of N copies. We deliberately do NOT set
* 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. */
export async function uploadChatFiles( export async function uploadChatFiles(
workspaceId: string, workspaceId: string,
files: File[], files: File[],
@ -20,12 +16,18 @@ export async function uploadChatFiles(
const form = new FormData(); const form = new FormData();
for (const f of files) form.append("files", f, f.name); for (const f of files) form.append("files", f, f.name);
const headers: Record<string, string> = {};
const slug = getTenantSlug();
if (slug) headers["X-Molecule-Org-Slug"] = slug;
const adminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN;
if (adminToken) headers["Authorization"] = `Bearer ${adminToken}`;
// Uploads legitimately take a while on cold cache (tar write + // Uploads legitimately take a while on cold cache (tar write +
// docker cp into the container). 60s is comfortable for the 25MB/ // docker cp into the container). 60s is comfortable for the 25MB/
// 50MB caps the server enforces. // 50MB caps the server enforces.
const res = await fetch(`${PLATFORM_URL}/workspaces/${workspaceId}/chat/uploads`, { const res = await fetch(`${PLATFORM_URL}/workspaces/${workspaceId}/chat/uploads`, {
method: "POST", method: "POST",
headers: platformAuthHeaders(), headers,
body: form, body: form,
credentials: "include", credentials: "include",
signal: AbortSignal.timeout(60_000), signal: AbortSignal.timeout(60_000),
@ -141,8 +143,14 @@ export async function downloadChatFile(
return; return;
} }
const headers: Record<string, string> = {};
const slug = getTenantSlug();
if (slug) headers["X-Molecule-Org-Slug"] = slug;
const adminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN;
if (adminToken) headers["Authorization"] = `Bearer ${adminToken}`;
const res = await fetch(href, { const res = await fetch(href, {
headers: platformAuthHeaders(), headers,
credentials: "include", credentials: "include",
signal: AbortSignal.timeout(60_000), signal: AbortSignal.timeout(60_000),
}); });

View File

@ -1,130 +0,0 @@
// @vitest-environment node
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
// Tests for the boot-time matched-pair guard added to next.config.ts.
//
// Why this lives in src/lib/__tests__ even though the function is in
// canvas/next.config.ts:
// - next.config.ts runs as ESM-but-also-CJS depending on which
// consumer loads it (Next.js dev server vs Next.js build); we
// want the test to be a plain ESM module Vitest already handles.
// - Importing from "../../../next.config" pulls in the rest of the
// file (loadMonorepoEnv, the default export, etc.) which has
// side effects on module load (it runs loadMonorepoEnv()
// immediately). To keep the test hermetic we don't import — we
// duplicate the function under test.
//
// Sourcing the function from a shared module would be cleaner, but
// next.config.ts is required to be a single self-contained file by
// Next.js's loader on some host configurations. Pin invariant: the
// duplicated function below MUST stay byte-identical to the one in
// next.config.ts. If you change one, change the other and bump this
// comment.
function checkAdminTokenPair(): void {
const serverSet = !!process.env.ADMIN_TOKEN;
const clientSet = !!process.env.NEXT_PUBLIC_ADMIN_TOKEN;
if (serverSet === clientSet) return;
if (serverSet && !clientSet) {
// eslint-disable-next-line no-console
console.error(
"[next.config] ADMIN_TOKEN is set but NEXT_PUBLIC_ADMIN_TOKEN is not — " +
"canvas will 401 against workspace-server because the bearer header " +
"is never attached. Set both to the same value, or unset both.",
);
} else {
// eslint-disable-next-line no-console
console.error(
"[next.config] NEXT_PUBLIC_ADMIN_TOKEN is set but ADMIN_TOKEN is not — " +
"workspace-server will reject the bearer because no AdminAuth gate " +
"is configured. Set both to the same value, or unset both.",
);
}
}
describe("checkAdminTokenPair", () => {
// Snapshot env so individual tests can stomp on it without leaking.
// Rebuild from snapshot in afterEach so the next test sees a known
// baseline regardless of mutation pattern.
let originalEnv: Record<string, string | undefined>;
let errorSpy: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
originalEnv = {
ADMIN_TOKEN: process.env.ADMIN_TOKEN,
NEXT_PUBLIC_ADMIN_TOKEN: process.env.NEXT_PUBLIC_ADMIN_TOKEN,
};
delete process.env.ADMIN_TOKEN;
delete process.env.NEXT_PUBLIC_ADMIN_TOKEN;
errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
});
afterEach(() => {
if (originalEnv.ADMIN_TOKEN === undefined) delete process.env.ADMIN_TOKEN;
else process.env.ADMIN_TOKEN = originalEnv.ADMIN_TOKEN;
if (originalEnv.NEXT_PUBLIC_ADMIN_TOKEN === undefined) delete process.env.NEXT_PUBLIC_ADMIN_TOKEN;
else process.env.NEXT_PUBLIC_ADMIN_TOKEN = originalEnv.NEXT_PUBLIC_ADMIN_TOKEN;
errorSpy.mockRestore();
});
it("emits no warning when both are unset", () => {
checkAdminTokenPair();
expect(errorSpy).not.toHaveBeenCalled();
});
it("emits no warning when both are set (matched pair, the happy path)", () => {
process.env.ADMIN_TOKEN = "local-dev-admin";
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "local-dev-admin";
checkAdminTokenPair();
expect(errorSpy).not.toHaveBeenCalled();
});
it("warns when ADMIN_TOKEN is set but NEXT_PUBLIC_ADMIN_TOKEN is not", () => {
process.env.ADMIN_TOKEN = "local-dev-admin";
checkAdminTokenPair();
expect(errorSpy).toHaveBeenCalledTimes(1);
// Exact-string assertion — substring would also pass when the
// function's branch logic is broken (e.g. emits both messages, or
// emits the wrong one). Pin the exact message that operators will
// see in their dev console so regressions are visible.
expect(errorSpy).toHaveBeenCalledWith(
"[next.config] ADMIN_TOKEN is set but NEXT_PUBLIC_ADMIN_TOKEN is not — " +
"canvas will 401 against workspace-server because the bearer header " +
"is never attached. Set both to the same value, or unset both.",
);
});
it("warns when NEXT_PUBLIC_ADMIN_TOKEN is set but ADMIN_TOKEN is not", () => {
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "local-dev-admin";
checkAdminTokenPair();
expect(errorSpy).toHaveBeenCalledTimes(1);
expect(errorSpy).toHaveBeenCalledWith(
"[next.config] NEXT_PUBLIC_ADMIN_TOKEN is set but ADMIN_TOKEN is not — " +
"workspace-server will reject the bearer because no AdminAuth gate " +
"is configured. Set both to the same value, or unset both.",
);
});
// Empty string in process.env is the JS-side representation of `KEY=`
// (no value) in a .env file. Treating "" as unset makes the pair
// invariant symmetric: `KEY=` and `unset KEY` produce the same
// verdict. Without this branch, an operator who comments out the
// value but leaves the line would get a false-positive warning.
it("treats empty string as unset (so KEY= and unset KEY are equivalent)", () => {
process.env.ADMIN_TOKEN = "";
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "";
checkAdminTokenPair();
expect(errorSpy).not.toHaveBeenCalled();
});
it("warns when ADMIN_TOKEN is set and NEXT_PUBLIC_ADMIN_TOKEN is empty string", () => {
process.env.ADMIN_TOKEN = "local-dev-admin";
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "";
checkAdminTokenPair();
expect(errorSpy).toHaveBeenCalledTimes(1);
// First branch — server set, client unset.
expect(errorSpy).toHaveBeenCalledWith(
expect.stringContaining("ADMIN_TOKEN is set but NEXT_PUBLIC_ADMIN_TOKEN is not"),
);
});
});

View File

@ -1,97 +0,0 @@
// @vitest-environment jsdom
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
// Tests for platformAuthHeaders — the shared helper extracted in #178
// to consolidate the bearer-token-attach + tenant-slug-attach pattern
// that was previously duplicated across 7 raw-fetch callsites in the
// canvas (uploads + 5 Attachment* components + the api.ts request()
// function).
//
// What we pin here:
// - Returns a fresh object each call (so callers can mutate without
// leaking into each other).
// - Empty result on a non-tenant host with no admin token (the
// localhost / self-hosted shape).
// - Bearer attached when NEXT_PUBLIC_ADMIN_TOKEN is set.
// - X-Molecule-Org-Slug attached when window.location.hostname is a
// tenant subdomain (<slug>.moleculesai.app).
// - Both attached when both apply (the production SaaS shape).
//
// Why jsdom: getTenantSlug() reads window.location.hostname. Node-only
// environment yields no window and getTenantSlug returns null
// unconditionally — wouldn't exercise the slug branch.
import { platformAuthHeaders } from "../api";
describe("platformAuthHeaders", () => {
let originalAdminToken: string | undefined;
beforeEach(() => {
originalAdminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN;
delete process.env.NEXT_PUBLIC_ADMIN_TOKEN;
});
afterEach(() => {
if (originalAdminToken === undefined) delete process.env.NEXT_PUBLIC_ADMIN_TOKEN;
else process.env.NEXT_PUBLIC_ADMIN_TOKEN = originalAdminToken;
// jsdom resets hostname between tests via the @vitest-environment
// pragma's per-test isolation. No explicit reset needed.
});
it("returns an empty object on a non-tenant host with no admin token", () => {
// jsdom default hostname is "localhost" — not a tenant slug, so
// getTenantSlug() returns null and no X-Molecule-Org-Slug is added.
const headers = platformAuthHeaders();
expect(headers).toEqual({});
});
it("attaches Authorization when NEXT_PUBLIC_ADMIN_TOKEN is set", () => {
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "local-dev-admin";
const headers = platformAuthHeaders();
expect(headers).toEqual({ Authorization: "Bearer local-dev-admin" });
});
it("does NOT attach Authorization when NEXT_PUBLIC_ADMIN_TOKEN is empty string", () => {
// Empty-string env is the JS-side shape of `KEY=` in .env.
// Treating it as unset matches the matched-pair guard in
// next.config.ts (admin-token-pair.test.ts) — symmetric semantics.
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "";
const headers = platformAuthHeaders();
expect(headers).toEqual({});
});
it("attaches X-Molecule-Org-Slug on a tenant subdomain", () => {
Object.defineProperty(window, "location", {
value: { hostname: "reno-stars.moleculesai.app" },
writable: true,
});
const headers = platformAuthHeaders();
expect(headers).toEqual({ "X-Molecule-Org-Slug": "reno-stars" });
});
it("attaches both when both apply (production SaaS shape)", () => {
Object.defineProperty(window, "location", {
value: { hostname: "reno-stars.moleculesai.app" },
writable: true,
});
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "tenant-bearer";
const headers = platformAuthHeaders();
// Pin exact-equality on the full shape — substring/contains
// assertions would also pass for an extra-header bug.
expect(headers).toEqual({
"X-Molecule-Org-Slug": "reno-stars",
Authorization: "Bearer tenant-bearer",
});
});
it("returns a fresh object each call (callers can mutate safely)", () => {
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "tok";
const a = platformAuthHeaders();
const b = platformAuthHeaders();
expect(a).not.toBe(b); // distinct refs
expect(a).toEqual(b); // same content
a["Content-Type"] = "application/json";
// Mutation on `a` does not leak into `b`.
expect(b["Content-Type"]).toBeUndefined();
});
});

View File

@ -21,45 +21,6 @@ export interface RequestOptions {
timeoutMs?: number; timeoutMs?: number;
} }
/**
* Build the platform auth header set used by every authenticated fetch
* from the canvas. Returns a fresh object so callers can mutate (e.g.
* append `Content-Type` for JSON requests, omit it for FormData).
*
* SaaS cross-origin shape:
* - `X-Molecule-Org-Slug` derived from `window.location.hostname`
* by `getTenantSlug()`. Control plane uses it for fly-replay
* routing. Empty on localhost / non-tenant hosts safe to omit.
* - `Authorization: Bearer <token>` `NEXT_PUBLIC_ADMIN_TOKEN` baked
* into the canvas build (see canvas/Dockerfile L8/L11). Required by
* the workspace-server when `ADMIN_TOKEN` is set on the server side
* (Tier-2b AdminAuth gate, wsauth_middleware.go ~L245). Empty when
* no admin token was provisioned the Tier-1 session-cookie path
* handles that case via `credentials:"include"`.
*
* Why a shared helper: the two-line "read env, attach bearer; read
* slug, attach header" pattern was duplicated across `request()` and
* 7 raw-fetch callsites (chat uploads/download + 5 Attachment*
* components) before this consolidation. A new poller or raw fetch
* that forgets one of the two headers silently 401s against
* workspace-server when ADMIN_TOKEN is set the exact bug shape
* called out in #178 / closes the post-#176 self-review gap.
*
* Callers that want JSON Content-Type should spread this and add it
* themselves; FormData callers should NOT add Content-Type (the
* browser sets the multipart boundary). Centralizing the auth pair
* but leaving Content-Type up to the caller is the minimum viable
* shared shape.
*/
export function platformAuthHeaders(): Record<string, string> {
const headers: Record<string, string> = {};
const slug = getTenantSlug();
if (slug) headers["X-Molecule-Org-Slug"] = slug;
const adminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN;
if (adminToken) headers["Authorization"] = `Bearer ${adminToken}`;
return headers;
}
async function request<T>( async function request<T>(
method: string, method: string,
path: string, path: string,
@ -67,16 +28,17 @@ async function request<T>(
retryCount = 0, retryCount = 0,
options?: RequestOptions, options?: RequestOptions,
): Promise<T> { ): Promise<T> {
// JSON-bodied request — Content-Type is JSON. Auth pair comes from // SaaS cross-origin shape:
// the shared helper; see its doc comment for the SaaS-shape rationale. // - X-Molecule-Org-Slug: derived from window.location.hostname by
const headers: Record<string, string> = { // getTenantSlug(). Control plane uses it for fly-replay routing.
"Content-Type": "application/json", // Empty on localhost / non-tenant hosts — safe to omit.
...platformAuthHeaders(), // - credentials:"include": sends the session cookie cross-origin.
}; // Cookie's Domain=.moleculesai.app attribute + cp's CORS allow this.
// Re-read slug locally for the 401 handler below — `headers` already const headers: Record<string, string> = { "Content-Type": "application/json" };
// has it, but the 401 branch needs the bare value to gate the
// session-probe + redirect logic on tenant context.
const slug = getTenantSlug(); const slug = getTenantSlug();
if (slug) headers["X-Molecule-Org-Slug"] = slug;
const adminToken = process.env.NEXT_PUBLIC_ADMIN_TOKEN;
if (adminToken) headers["Authorization"] = `Bearer ${adminToken}`;
const res = await fetch(`${PLATFORM_URL}${path}`, { const res = await fetch(`${PLATFORM_URL}${path}`, {
method, method,

View File

@ -835,180 +835,3 @@ describe("handleCanvasEvent unknown event", () => {
).not.toThrow(); ).not.toThrow();
}); });
}); });
// ---------------------------------------------------------------------------
// Screen-reader live announcements
// ---------------------------------------------------------------------------
describe("handleCanvasEvent liveAnnouncement", () => {
it("announces WORKSPACE_ONLINE with node name", () => {
const node = makeNode("ws-1", { name: "Alpha" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({ event: "WORKSPACE_ONLINE", workspace_id: "ws-1" }),
get,
set
);
expect(state.liveAnnouncement).toBe("Alpha is now online");
});
it("announces WORKSPACE_OFFLINE with node name", () => {
const node = makeNode("ws-1", { name: "Beta" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({ event: "WORKSPACE_OFFLINE", workspace_id: "ws-1" }),
get,
set
);
expect(state.liveAnnouncement).toBe("Beta is now offline");
});
it("announces WORKSPACE_PAUSED with node name", () => {
const node = makeNode("ws-1", { name: "Gamma" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({ event: "WORKSPACE_PAUSED", workspace_id: "ws-1" }),
get,
set
);
expect(state.liveAnnouncement).toBe("Gamma has been paused");
});
it("announces WORKSPACE_DEGRADED with node name", () => {
const node = makeNode("ws-1", { name: "Delta" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({
event: "WORKSPACE_DEGRADED",
workspace_id: "ws-1",
payload: { sample_error: "connection timeout" },
}),
get,
set
);
expect(state.liveAnnouncement).toBe("Delta is degraded");
});
it("announces WORKSPACE_PROVISIONING for new workspace with payload name", () => {
const { get, set, state } = makeStore([]);
handleCanvasEvent(
makeMsg({
event: "WORKSPACE_PROVISIONING",
workspace_id: "ws-new",
payload: { name: "NewBot" },
}),
get,
set
);
expect(state.liveAnnouncement).toBe("NewBot is provisioning");
});
it("announces WORKSPACE_PROVISIONING for new workspace with default name", () => {
const { get, set, state } = makeStore([]);
handleCanvasEvent(
makeMsg({
event: "WORKSPACE_PROVISIONING",
workspace_id: "ws-new",
payload: {},
}),
get,
set
);
expect(state.liveAnnouncement).toBe("New Workspace is provisioning");
});
it("announces WORKSPACE_REMOVED with node name", () => {
const node = makeNode("ws-1", { name: "Gamma" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({ event: "WORKSPACE_REMOVED", workspace_id: "ws-1" }),
get,
set
);
expect(state.liveAnnouncement).toBe("Gamma was removed");
});
it("announces WORKSPACE_PROVISION_FAILED with node name", () => {
const node = makeNode("ws-1", { name: "Delta" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({
event: "WORKSPACE_PROVISION_FAILED",
workspace_id: "ws-1",
payload: { error: "docker pull failed" },
}),
get,
set
);
expect(state.liveAnnouncement).toBe("Delta provisioning failed");
});
it("does not announce for TASK_UPDATED", () => {
const node = makeNode("ws-1", { name: "Alpha" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({
event: "TASK_UPDATED",
workspace_id: "ws-1",
payload: { current_task: "building release", active_tasks: 1 },
}),
get,
set
);
// TASK_UPDATED is noisy (every heartbeat); it should not announce
expect(state.liveAnnouncement ?? "").toBe("");
});
it("does not announce for AGENT_MESSAGE", () => {
const node = makeNode("ws-1", { name: "Alpha" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({
event: "AGENT_MESSAGE",
workspace_id: "ws-1",
payload: { message: "hello from the agent" },
}),
get,
set
);
expect(state.liveAnnouncement ?? "").toBe("");
});
it("uses payload name for ONLINE when node not found in store", () => {
const { get, set, state } = makeStore([]);
handleCanvasEvent(
makeMsg({
event: "WORKSPACE_ONLINE",
workspace_id: "ws-1",
payload: { name: "FromPayload" },
}),
get,
set
);
// ONLINE when node doesn't exist just buffers _pendingOnline;
// no announcement should be set
expect(state.liveAnnouncement ?? "").toBe("");
});
});

View File

@ -1181,46 +1181,3 @@ describe("batchNest", () => {
expect(nestPatches).toHaveLength(1); expect(nestPatches).toHaveLength(1);
}); });
}); });
// ---------- moveNode ----------
describe("moveNode", () => {
beforeEach(() => {
const mock = global.fetch as ReturnType<typeof vi.fn>;
mock.mockImplementation(() =>
Promise.resolve({ ok: true, json: () => Promise.resolve({}) } as Response),
);
mock.mockClear();
});
it("updates the node's position by the given delta", () => {
useCanvasStore.getState().hydrate([
makeWS({ id: "n1", name: "Node 1", x: 100, y: 200 }),
]);
useCanvasStore.getState().selectNode("n1");
useCanvasStore.getState().moveNode("n1", 10, -50);
const node = useCanvasStore.getState().nodes.find((n) => n.id === "n1")!;
expect(node.position).toEqual({ x: 110, y: 150 });
});
it("is a no-op when the node does not exist", () => {
useCanvasStore.getState().hydrate([makeWS({ id: "n1", name: "Node 1", x: 0, y: 0 })]);
expect(() => useCanvasStore.getState().moveNode("nonexistent", 10, 10)).not.toThrow();
});
it("calls savePosition with the new absolute coordinates", async () => {
useCanvasStore.getState().hydrate([makeWS({ id: "n1", name: "Node 1", x: 100, y: 200 })]);
useCanvasStore.getState().selectNode("n1");
const mock = global.fetch as ReturnType<typeof vi.fn>;
useCanvasStore.getState().moveNode("n1", 10, 20);
await vi.waitFor(() => {
expect(mock).toHaveBeenCalledWith(
expect.stringContaining("/workspaces/n1"),
expect.objectContaining({
method: "PATCH",
body: JSON.stringify({ x: 110, y: 220 }),
}),
);
});
});
});

View File

@ -80,7 +80,6 @@ export function handleCanvasEvent(
switch (msg.event) { switch (msg.event) {
case "WORKSPACE_ONLINE": { case "WORKSPACE_ONLINE": {
const existing = nodes.find((n) => n.id === msg.workspace_id); const existing = nodes.find((n) => n.id === msg.workspace_id);
const nodeName = existing?.data.name ?? (msg.payload.name as string) ?? "Workspace";
if (!existing) { if (!existing) {
// PROVISIONING event hasn't been applied yet (WS reorder or // PROVISIONING event hasn't been applied yet (WS reorder or
// this tab joined mid-deploy). Buffer so the later PROVISIONING // this tab joined mid-deploy). Buffer so the later PROVISIONING
@ -106,7 +105,6 @@ export function handleCanvasEvent(
? { ...n, data: { ...n.data, status: "online" } } ? { ...n, data: { ...n.data, status: "online" } }
: n, : n,
), ),
liveAnnouncement: `${nodeName} is now online`,
}); });
// Remove the laser class after its keyframe ends so the edge // Remove the laser class after its keyframe ends so the edge
// settles into the app's default solid styling. Fire-and-forget. // settles into the app's default solid styling. Fire-and-forget.
@ -125,36 +123,28 @@ export function handleCanvasEvent(
} }
case "WORKSPACE_OFFLINE": { case "WORKSPACE_OFFLINE": {
const offlineNode = nodes.find((n) => n.id === msg.workspace_id);
const offlineName = offlineNode?.data.name ?? "Workspace";
set({ set({
nodes: nodes.map((n) => nodes: nodes.map((n) =>
n.id === msg.workspace_id n.id === msg.workspace_id
? { ...n, data: { ...n.data, status: "offline" } } ? { ...n, data: { ...n.data, status: "offline" } }
: n : n
), ),
liveAnnouncement: `${offlineName} is now offline`,
}); });
break; break;
} }
case "WORKSPACE_PAUSED": { case "WORKSPACE_PAUSED": {
const pausedNode = nodes.find((n) => n.id === msg.workspace_id);
const pausedName = pausedNode?.data.name ?? "Workspace";
set({ set({
nodes: nodes.map((n) => nodes: nodes.map((n) =>
n.id === msg.workspace_id n.id === msg.workspace_id
? { ...n, data: { ...n.data, status: "paused", currentTask: "" } } ? { ...n, data: { ...n.data, status: "paused", currentTask: "" } }
: n : n
), ),
liveAnnouncement: `${pausedName} has been paused`,
}); });
break; break;
} }
case "WORKSPACE_DEGRADED": { case "WORKSPACE_DEGRADED": {
const degradedNode = nodes.find((n) => n.id === msg.workspace_id);
const degradedName = degradedNode?.data.name ?? "Workspace";
set({ set({
nodes: nodes.map((n) => nodes: nodes.map((n) =>
n.id === msg.workspace_id n.id === msg.workspace_id
@ -170,7 +160,6 @@ export function handleCanvasEvent(
} }
: n : n
), ),
liveAnnouncement: `${degradedName} is degraded`,
}); });
break; break;
} }
@ -241,7 +230,6 @@ export function handleCanvasEvent(
// removed per demo feedback. A2A edges (showA2AEdges) still // removed per demo feedback. A2A edges (showA2AEdges) still
// render when enabled — those represent runtime traffic, // render when enabled — those represent runtime traffic,
// which nesting doesn't express. // which nesting doesn't express.
const newNodeName = (msg.payload.name as string) ?? "New Workspace";
set({ set({
nodes: [ nodes: [
...nodes, ...nodes,
@ -256,7 +244,7 @@ export function handleCanvasEvent(
...(parentId ? { parentId } : {}), ...(parentId ? { parentId } : {}),
className: "mol-deploy-spawn", className: "mol-deploy-spawn",
data: { data: {
name: newNodeName, name: (msg.payload.name as string) ?? "New Workspace",
status: "provisioning", status: "provisioning",
tier: (msg.payload.tier as number) ?? 1, tier: (msg.payload.tier as number) ?? 1,
agentCard: null, agentCard: null,
@ -273,7 +261,6 @@ export function handleCanvasEvent(
}, },
}, },
], ],
liveAnnouncement: `${newNodeName} is provisioning`,
}); });
// Grow the parent to fit the just-landed child. DEBOUNCED // Grow the parent to fit the just-landed child. DEBOUNCED
@ -358,7 +345,6 @@ export function handleCanvasEvent(
case "WORKSPACE_REMOVED": { case "WORKSPACE_REMOVED": {
const removedNode = nodes.find((n) => n.id === msg.workspace_id); const removedNode = nodes.find((n) => n.id === msg.workspace_id);
const removedName = removedNode?.data.name ?? "Workspace";
const parentOfRemoved = removedNode?.data.parentId ?? null; const parentOfRemoved = removedNode?.data.parentId ?? null;
set({ set({
nodes: nodes nodes: nodes
@ -377,7 +363,6 @@ export function handleCanvasEvent(
e.source !== msg.workspace_id && e.target !== msg.workspace_id e.source !== msg.workspace_id && e.target !== msg.workspace_id
), ),
selectedNodeId: selectedNodeId === msg.workspace_id ? null : selectedNodeId, selectedNodeId: selectedNodeId === msg.workspace_id ? null : selectedNodeId,
liveAnnouncement: `${removedName} was removed`,
}); });
break; break;
} }
@ -460,8 +445,6 @@ export function handleCanvasEvent(
case "WORKSPACE_PROVISION_FAILED": { case "WORKSPACE_PROVISION_FAILED": {
const errorMsg = (msg.payload.error as string) ?? "Unknown provisioning error"; const errorMsg = (msg.payload.error as string) ?? "Unknown provisioning error";
const failedNode = nodes.find((n) => n.id === msg.workspace_id);
const failedName = failedNode?.data.name ?? "Workspace";
set({ set({
nodes: nodes.map((n) => nodes: nodes.map((n) =>
n.id === msg.workspace_id n.id === msg.workspace_id
@ -475,7 +458,6 @@ export function handleCanvasEvent(
} }
: n : n
), ),
liveAnnouncement: `${failedName} provisioning failed`,
}); });
break; break;
} }

View File

@ -165,13 +165,6 @@ interface CanvasState {
* this so a drag that pushed a child past the parent edge commits * this so a drag that pushed a child past the parent edge commits
* the parent grow on release (commit-on-release pattern). */ * the parent grow on release (commit-on-release pattern). */
growParentsToFitChildren: () => void; growParentsToFitChildren: () => void;
/** Move a selected node by (dx, dy) in canvas space. Used by keyboard
* arrow-key shortcuts so keyboard users can reposition nodes without a
* mouse. Persists the new position to the backend and skips the
* grow-parents pass that onNodesChange runs on every drag tick
* (avoids the "edge-chase" flicker that commit-on-release is meant to
* prevent). */
moveNode: (nodeId: string, dx: number, dy: number) => void;
/** Re-layout a parent's children to the default 2-column grid. Used /** Re-layout a parent's children to the default 2-column grid. Used
* by the "Arrange children" context-menu command so users can rescue * by the "Arrange children" context-menu command so users can rescue
* out-of-bounds children on demand topology no longer does it * out-of-bounds children on demand topology no longer does it
@ -232,11 +225,6 @@ interface CanvasState {
/** Whether the A2A topology overlay is visible. Persisted to localStorage. Default: true. */ /** Whether the A2A topology overlay is visible. Persisted to localStorage. Default: true. */
showA2AEdges: boolean; showA2AEdges: boolean;
setShowA2AEdges: (show: boolean) => void; setShowA2AEdges: (show: boolean) => void;
/** Screen-reader announcement text. Set by handleCanvasEvent on significant
* status changes; consumed and cleared by the aria-live region in Canvas.tsx
* so the same announcement doesn't re-fire on re-render. */
liveAnnouncement: string;
setLiveAnnouncement: (msg: string) => void;
} }
export const useCanvasStore = create<CanvasState>((set, get) => ({ export const useCanvasStore = create<CanvasState>((set, get) => ({
@ -333,8 +321,6 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
localStorage.setItem("molecule:show-a2a-edges", String(show)); localStorage.setItem("molecule:show-a2a-edges", String(show));
} }
}, },
liveAnnouncement: "",
setLiveAnnouncement: (msg) => set({ liveAnnouncement: msg }),
viewport: { x: 0, y: 0, zoom: 1 }, viewport: { x: 0, y: 0, zoom: 1 },
@ -1039,19 +1025,6 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
} }
}, },
moveNode: (nodeId, dx, dy) => {
const node = get().nodes.find((n) => n.id === nodeId);
if (!node) return;
set({
nodes: get().nodes.map((n) =>
n.id === nodeId
? { ...n, position: { x: n.position.x + dx, y: n.position.y + dy } }
: n,
),
});
void get().savePosition(nodeId, node.position.x + dx, node.position.y + dy);
},
savePosition: async (nodeId: string, x: number, y: number) => { savePosition: async (nodeId: string, x: number, y: number) => {
try { try {
await api.patch(`/workspaces/${nodeId}`, { x, y }); await api.patch(`/workspaces/${nodeId}`, { x, y });

View File

@ -7,22 +7,6 @@ export default defineConfig({
test: { test: {
environment: 'node', environment: 'node',
exclude: ['e2e/**', 'node_modules/**', '**/dist/**'], exclude: ['e2e/**', 'node_modules/**', '**/dist/**'],
// Issue #22 / vitest pool investigation:
//
// The forks pool spawns one Node.js worker per concurrent slot.
// Each jsdom-environment worker bootstraps a full DOM (~30-50 MB resident
// set) at cold-start. With the default maxWorkers derived from CPU
// count, multiple jsdom workers can start simultaneously, exhausting
// memory on the 2-CPU Gitea Actions runner and causing pool workers to
// fail to respond with "[vitest-pool]: Timeout starting … runner."
//
// Fix: cap maxWorkers at 1 so only one worker is alive at any time.
// Tests still run in parallel within that single worker's process (via
// node's EventLoop) — this is the same parallelism as the `threads`
// pool but without the per-worker jsdom cold-start overhead. 51 test
// files that previously took 5070 s with 5 failures now run
// sequentially through one worker, eliminating the memory spike.
maxWorkers: 1,
// CI-conditional test timeout (issue #96). // CI-conditional test timeout (issue #96).
// //
// Vitest's 5000ms default is too tight for the first test in any // Vitest's 5000ms default is too tight for the first test in any

View File

@ -119,7 +119,7 @@ services:
networks: networks:
default: default:
name: molecule-core-net name: molecule-monorepo-net
external: true external: true
volumes: volumes:

View File

@ -1,7 +1,3 @@
# Include infra services (Temporal, Langfuse) so `docker compose up` starts the full stack.
include:
- docker-compose.infra.yml
services: services:
# --- Infrastructure --- # --- Infrastructure ---
postgres: postgres:
@ -16,8 +12,7 @@ services:
volumes: volumes:
- pgdata:/var/lib/postgresql/data - pgdata:/var/lib/postgresql/data
networks: networks:
- molecule-core-net - molecule-monorepo-net
restart: unless-stopped
healthcheck: healthcheck:
test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-dev}"] test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-dev}"]
interval: 2s interval: 2s
@ -44,7 +39,7 @@ services:
psql -h postgres -U "$${POSTGRES_USER}" -d postgres -c "CREATE DATABASE langfuse" psql -h postgres -U "$${POSTGRES_USER}" -d postgres -c "CREATE DATABASE langfuse"
fi fi
networks: networks:
- molecule-core-net - molecule-monorepo-net
redis: redis:
image: redis:7-alpine image: redis:7-alpine
@ -54,8 +49,7 @@ services:
volumes: volumes:
- redisdata:/data - redisdata:/data
networks: networks:
- molecule-core-net - molecule-monorepo-net
restart: unless-stopped
healthcheck: healthcheck:
test: ["CMD", "redis-cli", "ping"] test: ["CMD", "redis-cli", "ping"]
interval: 2s interval: 2s
@ -72,7 +66,7 @@ services:
volumes: volumes:
- clickhousedata:/var/lib/clickhouse - clickhousedata:/var/lib/clickhouse
networks: networks:
- molecule-core-net - molecule-monorepo-net
healthcheck: healthcheck:
test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://127.0.0.1:8123/ping || exit 1"] test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://127.0.0.1:8123/ping || exit 1"]
interval: 5s interval: 5s
@ -101,7 +95,7 @@ services:
ports: ports:
- "3001:3000" - "3001:3000"
networks: networks:
- molecule-core-net - molecule-monorepo-net
healthcheck: healthcheck:
test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://localhost:3000/api/public/health || exit 1"] test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://localhost:3000/api/public/health || exit 1"]
interval: 10s interval: 10s
@ -132,10 +126,6 @@ services:
REDIS_URL: redis://redis:6379 REDIS_URL: redis://redis:6379
PORT: "${PLATFORM_PORT:-8080}" PORT: "${PLATFORM_PORT:-8080}"
PLATFORM_URL: "http://platform:${PLATFORM_PORT:-8080}" PLATFORM_URL: "http://platform:${PLATFORM_PORT:-8080}"
# Container network namespace is already isolated; "all interfaces"
# inside the container = the bridge interface only. The fail-open
# default (127.0.0.1) would block host-to-container access.
BIND_ADDR: "${BIND_ADDR:-0.0.0.0}"
# Default MOLECULE_ENV=development so the WorkspaceAuth / AdminAuth # Default MOLECULE_ENV=development so the WorkspaceAuth / AdminAuth
# middleware fail-open path activates when ADMIN_TOKEN is unset — # middleware fail-open path activates when ADMIN_TOKEN is unset —
# otherwise the canvas (which runs without a bearer in pure local # otherwise the canvas (which runs without a bearer in pure local
@ -205,28 +195,12 @@ services:
# App private key — read-only bind-mount. The host-side path is # App private key — read-only bind-mount. The host-side path is
# gitignored per .gitignore rules (/.secrets/ + *.pem). # gitignored per .gitignore rules (/.secrets/ + *.pem).
- ./.secrets/github-app.pem:/secrets/github-app.pem:ro - ./.secrets/github-app.pem:/secrets/github-app.pem:ro
# Per-role persona credentials (molecule-core#242 local surface).
# Sourced at workspace creation time by org_import.go::loadPersonaEnvFile
# when a workspace.yaml carries `role: <name>`. The host-side dir is
# populated by the operator-host bootstrap kit (28 dev-tree personas);
# /etc/molecule-bootstrap/personas is the in-container path the
# platform expects (matches the prod tenant-EC2 path so the same code
# works in both modes).
#
# Read-only mount — workspace-server only reads, never writes here.
# If the host dir is empty/missing the platform's loadPersonaEnvFile
# silently no-ops per its existing semantics, so this mount is safe
# even on a fresh machine that hasn't run the bootstrap kit yet.
- ${MOLECULE_PERSONA_ROOT_HOST:-${HOME}/.molecule-ai/personas}:/etc/molecule-bootstrap/personas:ro
ports: ports:
- "${PLATFORM_PUBLISH_PORT:-8080}:${PLATFORM_PORT:-8080}" - "${PLATFORM_PUBLISH_PORT:-8080}:${PLATFORM_PORT:-8080}"
networks: networks:
- molecule-core-net - molecule-monorepo-net
restart: unless-stopped
healthcheck: healthcheck:
# Plain GET — `--spider` would issue HEAD, which returns 404 because test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://localhost:${PLATFORM_PORT:-8080}/health || exit 1"]
# /health is registered as GET only.
test: ["CMD-SHELL", "wget -qO /dev/null --tries=1 http://localhost:${PLATFORM_PORT:-8080}/health || exit 1"]
interval: 5s interval: 5s
timeout: 5s timeout: 5s
retries: 10 retries: 10
@ -262,9 +236,9 @@ services:
ports: ports:
- "${CANVAS_PUBLISH_PORT:-3000}:${CANVAS_PORT:-3000}" - "${CANVAS_PUBLISH_PORT:-3000}:${CANVAS_PORT:-3000}"
networks: networks:
- molecule-core-net - molecule-monorepo-net
healthcheck: healthcheck:
test: ["CMD-SHELL", "wget -qO /dev/null --tries=1 http://127.0.0.1:${CANVAS_PORT:-3000} || exit 1"] test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://127.0.0.1:${CANVAS_PORT:-3000} || exit 1"]
interval: 10s interval: 10s
timeout: 5s timeout: 5s
retries: 10 retries: 10
@ -295,7 +269,7 @@ services:
OPENROUTER_API_KEY: ${OPENROUTER_API_KEY:-} OPENROUTER_API_KEY: ${OPENROUTER_API_KEY:-}
LITELLM_MASTER_KEY: ${LITELLM_MASTER_KEY:-sk-molecule} LITELLM_MASTER_KEY: ${LITELLM_MASTER_KEY:-sk-molecule}
networks: networks:
- molecule-core-net - molecule-monorepo-net
restart: unless-stopped restart: unless-stopped
healthcheck: healthcheck:
test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://localhost:4000/health || exit 1"] test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://localhost:4000/health || exit 1"]
@ -320,7 +294,7 @@ services:
volumes: volumes:
- ollamadata:/root/.ollama - ollamadata:/root/.ollama
networks: networks:
- molecule-core-net - molecule-monorepo-net
restart: unless-stopped restart: unless-stopped
healthcheck: healthcheck:
test: ["CMD-SHELL", "ollama list || exit 1"] test: ["CMD-SHELL", "ollama list || exit 1"]
@ -330,8 +304,8 @@ services:
start_period: 20s start_period: 20s
networks: networks:
molecule-core-net: molecule-monorepo-net:
name: molecule-core-net name: molecule-monorepo-net
volumes: volumes:
pgdata: pgdata:

View File

@ -67,7 +67,7 @@ On-demand fits naturally with how agents work — an agent only needs to know ab
This is acceptable for MVP because: This is acceptable for MVP because:
- All workspaces are provisioned by the same platform on trusted infrastructure - All workspaces are provisioned by the same platform on trusted infrastructure
- Docker network isolation (`molecule-core-net`) limits who can reach workspace endpoints - Docker network isolation (`molecule-monorepo-net`) limits who can reach workspace endpoints
- The tool is self-hosted — the operator controls the network - The tool is self-hosted — the operator controls the network
**Known gap:** Once workspace A caches workspace B's URL, nothing stops A from calling B directly even after the hierarchy changes and A is no longer supposed to reach B. The cached URL remains valid until the container is restarted or the URL changes. **Known gap:** Once workspace A caches workspace B's URL, nothing stops A from calling B directly even after the hierarchy changes and A is no longer supposed to reach B. The cached URL remains valid until the container is restarted or the URL changes.

View File

@ -124,7 +124,7 @@ Six runtime adapters ship production-ready on `main`: LangGraph, DeepAgents, Cla
| Platform ↔ Redis | TCP | Ephemeral state (liveness TTL), caching, pub/sub | | Platform ↔ Redis | TCP | Ephemeral state (liveness TTL), caching, pub/sub |
| Workspace ↔ Workspace | HTTP (A2A JSON-RPC 2.0) | Direct peer-to-peer, **platform not in data path** | | Workspace ↔ Workspace | HTTP (A2A JSON-RPC 2.0) | Direct peer-to-peer, **platform not in data path** |
| Workspace → Langfuse | HTTP | Automatic OpenTelemetry tracing | | Workspace → Langfuse | HTTP | Automatic OpenTelemetry tracing |
| Docker Network | `molecule-core-net` | Internal-only by default, no exposed DB/Redis ports | | Docker Network | `molecule-monorepo-net` | Internal-only by default, no exposed DB/Redis ports |
### Core Components ### Core Components
@ -465,7 +465,7 @@ Unknown tier values default to T2 for safety. Applied via `provisioner.ApplyTier
### Docker Networking ### Docker Networking
- All containers join `molecule-core-net` private network - All containers join `molecule-monorepo-net` private network
- Container naming: `ws-{workspace_id[:12]}` - Container naming: `ws-{workspace_id[:12]}`
- Ephemeral host port binding: `127.0.0.1:0→8000/tcp` - Ephemeral host port binding: `127.0.0.1:0→8000/tcp`

View File

@ -19,7 +19,7 @@ The provisioner is the platform component that deploys workspace containers and
## Docker Networking (Tier 1-3, Tier 4 uses host) ## Docker Networking (Tier 1-3, Tier 4 uses host)
All workspace containers join the `molecule-core-net` Docker network. Containers are named `ws-{id[:12]}` (first 12 chars of workspace UUID). Two exported helpers in `provisioner` package provide the canonical naming: All workspace containers join the `molecule-monorepo-net` Docker network. Containers are named `ws-{id[:12]}` (first 12 chars of workspace UUID). Two exported helpers in `provisioner` package provide the canonical naming:
- `provisioner.ContainerName(workspaceID)``ws-{id[:12]}` - `provisioner.ContainerName(workspaceID)``ws-{id[:12]}`
- `provisioner.InternalURL(workspaceID)``http://ws-{id[:12]}:8000` - `provisioner.InternalURL(workspaceID)``http://ws-{id[:12]}:8000`
@ -38,7 +38,7 @@ This URL is pre-stored in both Postgres and Redis before the agent registers. Wh
**Why not use Docker-internal URLs?** In local dev, the platform runs on the host (not in Docker), so it cannot resolve Docker container hostnames. The ephemeral port mapping lets the A2A proxy reach agents via localhost. In production (platform in Docker), the Docker-internal URL (`http://ws-{id}:8000`) would work directly. **Why not use Docker-internal URLs?** In local dev, the platform runs on the host (not in Docker), so it cannot resolve Docker container hostnames. The ephemeral port mapping lets the A2A proxy reach agents via localhost. In production (platform in Docker), the Docker-internal URL (`http://ws-{id}:8000`) would work directly.
**Workspace-to-workspace discovery:** When a workspace discovers another workspace (via `X-Workspace-ID` header on `GET /registry/discover/:id`), the platform returns the Docker-internal URL (`http://ws-{first12chars}:8000`) so containers can reach each other directly on `molecule-core-net`. The internal URL is cached in Redis at provision time and also synthesized as a fallback if the cache misses (only for online/degraded workspaces). **Workspace-to-workspace discovery:** When a workspace discovers another workspace (via `X-Workspace-ID` header on `GET /registry/discover/:id`), the platform returns the Docker-internal URL (`http://ws-{first12chars}:8000`) so containers can reach each other directly on `molecule-monorepo-net`. The internal URL is cached in Redis at provision time and also synthesized as a fallback if the cache misses (only for online/degraded workspaces).
For external HTTPS access (multi-host mode), Nginx on the host handles TLS termination and proxies to the container. For external HTTPS access (multi-host mode), Nginx on the host handles TLS termination and proxies to the container.

View File

@ -1,120 +0,0 @@
# Canvas Architecture Audit — VERIFIED
> **Status:** VERIFIED — Cross-referenced against molecule-core/canvas/src/ (2026-05-09)
> **Author:** Core-FE (draft), Core-UIUX (verification)
> **Updated:** 2026-05-09 with architecture structure + known issues
## Canvas Stack (Verified)
| Technology | Version | Purpose |
|-----------|--------|---------|
| React Flow | `@xyflow/react` v12 | Node/edge rendering |
| Framework | Next.js 14 App Router | Routing, SSR |
| Styling | Tailwind v4 | CSS with custom properties |
| State | Zustand | Client state management |
## Directory Structure (Verified)
```
canvas/src/
├── components/
│ ├── Canvas.tsx # Viewport management, ReactFlow wrapper
│ ├── Toolbar.tsx # Add node/edge controls
│ ├── ContextMenu.tsx # Right-click menu
│ ├── SidePanel.tsx # Properties panel
│ ├── WorkspaceNode.tsx # Node rendering
│ ├── A2AEdge.tsx # Edge rendering
│ └── [tests]/ # Accessibility + component tests
├── stores/
│ └── secrets-store.ts # ⚠️ getGrouped() performance issue
├── hooks/
│ ├── useSocketEvent.ts
│ ├── useTemplateDeploy.tsx
│ └── useWorkspaceName.ts
└── lib/
├── api.ts
├── auth.ts
├── canvas-actions.ts
├── design-tokens.ts # STATUS_CONFIG, TIER_CONFIG
├── theme.ts
└── theme-provider.tsx # ThemeProvider, useTheme()
## Known Issues
### ✅ MEDIUM: secrets-store.ts Performance (mitigated)
**File:** `canvas/src/stores/secrets-store.ts`
**Issue:** `getGrouped()` selector creates new objects every call. Not memoized.
**Impact:** Mitigated — `SecretsTab.tsx` wraps the call in `useMemo`, so no active re-render issues in the single consumer. The store-level fix (memoizing `getGrouped` itself) is optional but low priority now.
### 🟡 MEDIUM: Pre-commit Hook Verification
**Issue:** Pre-commit hook checks 'use client' on hook-using components but unclear if it actually fails on violations.
**Action:** Verify the hook is enforcing the rule correctly.
## Verified Findings
### Node Rendering ✅ (with notes)
- **Framework:** `@xyflow/react` (React Flow) — DOM-based, not SVG/Canvas
- **Node selection:** `aria-pressed` + border ring (`border-accent/70`) + shadow
- **Node drag:** React Flow native drag + Arrow keys (10px/step, Shift 50px) — keyboard-accessible (PR #182) ✅
- **Node resize:** `NodeResizer` component visible on selected card, keyboard-inaccessible
- **Status:** Accessible via `aria-label` on node cards — "Alpha Workspace workspace — online"
### Edge Wiring ✅
- **Edge rendering:** React Flow SVG paths
- **Edge click target:** 1.5px stroke (CSS `stroke-width: 1.5 !important` in globals.css)
- **Edge creation:** React Flow drag-from-handle (mouse); keyboard via handle Enter/Space
- **Edge anchors:** Target handle (top): `Enter/Space` extracts node from parent. Source handle (bottom): `Enter/Space` nests selected node into this node. Both have `tabIndex=0`, `role="button"`, descriptive `aria-label`, and a blue focus ring ✅
- **Status:** Mouse + keyboard — keyboard users can nest and un-nest without a mouse
### Canvas Controls ✅
- **Zoom:** React Flow Controls component (verify if keyboard accessible)
- **Pan:** Space+drag, mouse drag
- **Minimap:** Not present (MiniMap mocked as null in tests)
- **Status:** Basic keyboard support via viewport shortcuts
### Keyboard Shortcuts ✅ (strong)
- All shortcuts in `useKeyboardShortcuts.ts` with `inInput` guard ✅
- Global `?` shortcut opens `KeyboardShortcutsDialog` (PR #175) ✅
- Dialog: portal-based, aria-modal, focus trap, Escape close ✅
- Arrow keys move selected node 10px (50px with Shift) — keyboard node drag (this PR) ✅
- Hierarchy navigation (Enter/Shift+Enter), z-order (Cmd+]/[), zoom-to-team (Z) ✅
### Focus Management ✅ (strong)
- Skip link → `#canvas-main`
- `aria-label` on ReactFlow container ✅
- Focus trap in modals via Radix ✅
- Focus ring: `focus-visible:ring-2 focus-visible:ring-blue-500 focus-visible:ring-offset-2 focus-visible:ring-offset-zinc-950`
### Accessibility Tree ✅
- Canvas is in accessibility tree (React Flow DOM nodes)
- Node state changes announced via `aria-live="polite"` region (PR #172) ✅
- Context menus announced via `role="menu"`
### Context Menus ✅ (strong)
- `role="menu"`, `role="menuitem"`, `role="separator"`
- `aria-label` with workspace name ✅
- ArrowUp/Down navigation with wrap-around ✅
- Escape + Tab close menu ✅
- Auto-focus first item on open ✅
### Drag and Drop ✅
- **Mouse drag:** React Flow native
- **Drop target:** Visual indicator (`bg-emerald-950/40 border-emerald-400/60`) ✅
- **Keyboard alternative:** Arrow keys move selected node 10px per press (50px with Shift) (PR #182) ✅
---
## Remaining Gaps (Priority Order)
| Priority | Item | Files | Status |
|----------|------|-------|--------|
| ~~HIGH~~ | ~~Screen reader announcements for canvas state changes~~ | ~~Canvas.tsx, canvas-events.ts, canvas.ts~~ | ✅ Done — PR #172 |
| MEDIUM | Keyboard shortcut help dialog | useKeyboardShortcuts.ts | ✅ Done (PR #175) |
| MEDIUM | Keyboard-accessible node drag | WorkspaceNode.tsx, useDragHandlers.ts | ✅ Done (this PR) |
| LOW | Keyboard-accessible edge anchors | A2AEdge.tsx, WorkspaceNode.tsx | ✅ Done |
| LOW | Node resize keyboard accessibility | WorkspaceNode.tsx (NodeResizer) | Not started |
---
*Verified 2026-05-09 by Core-UIUX against molecule-core/canvas/src/*
*Updated 2026-05-09: screen reader announcements (PR #172) + keyboard shortcut dialog (PR #175) completed*

View File

@ -1,424 +0,0 @@
# Canvas Design System v1 — VERIFIED
> **Status:** VERIFIED — Cross-referenced against molecule-core/canvas/src/ (2026-05-09)
> **Authors:** Core-FE (draft), Core-UIUX (verification + updates)
> **Source files verified:**
> - `canvas/src/app/globals.css`
> - `canvas/src/styles/theme-tokens.css`
> - `canvas/src/lib/design-tokens.ts`
> - `canvas/src/components/Tooltip.tsx`
> - `canvas/src/components/ContextMenu.tsx`
> - `canvas/src/components/Canvas.tsx`
> - `canvas/src/components/__tests__/Canvas.a11y.test.tsx`
> - `canvas/src/components/__tests__/ContextMenu.keyboard.test.tsx`
> - `canvas/src/components/__tests__/MissingKeysModal.a11y.test.tsx`
> - `canvas/src/components/__tests__/ConversationTraceModal.a11y.test.tsx`
---
## 1. Color Palette — Three-Mode Theme System
Canvas supports **three themes**: System (follows OS), Light, Dark. Controlled via `ThemeProvider` in `theme-provider.tsx` with preference persisted in `mol_theme` cookie.
**Key principle: Use semantic tokens, NOT raw zinc values for surfaces.**
### 1.1 Theme-Mutable Tokens (use these for surfaces)
Defined in `globals.css` via Tailwind v4 `@theme` block. Automatically flip between light/dark.
**Light theme (warm paper):**
| Token | Tailwind Class | Hex | Usage |
|-------|--------------|-----|-------|
| `--color-surface` | `bg-surface` | `#fafaf7` | Page background |
| `--color-surface-elevated` | `bg-surface-elevated` | `#ffffff` | Elevated cards, modals |
| `--color-surface-sunken` | `bg-surface-sunken` | `#f3f1ec` | Input fields, recessed areas |
| `--color-surface-card` | `bg-surface-card` | `#efece4` | Node cards, chips |
| `--color-line` | `border-line` | `#e6e2d8` | Dividers, borders |
| `--color-line-soft` | `border-line-soft` | `#efece4` | Subtle dividers |
| `--color-ink` | `text-ink` | `#15181c` | Primary text |
| `--color-ink-mid` | `text-ink-mid` | `#5a5e66` | Secondary text |
| `--color-ink-soft` | `text-ink-soft` | `#8b8e95` | Tertiary text, placeholders |
| `--color-accent` | `text-accent` | `#3b5bdb` | Links, primary actions |
| `--color-accent-strong` | `text-accent-strong` | `#1a2f99` | Emphasized accent |
| `--color-warm` | `text-warm` | `#c0532b` | Warnings |
| `--color-good` | `text-good` | `#2f7a4d` | Success states |
| `--color-bad` | `text-bad` | `#b94e4a` | Error states |
**Dark theme:**
| Token | Hex | Usage |
|-------|-----|-------|
| `--color-surface` | `#0e1014` | Page background |
| `--color-surface-elevated` | `#15181c` | Elevated cards |
| `--color-surface-sunken` | `#0a0b0e` | Input fields |
| `--color-surface-card` | `#1a1d23` | Node cards |
| `--color-line` | `#2a2f3a` | Dividers |
| `--color-ink` | `#f4f1e9` | Primary text |
| `--color-ink-mid` | `#c8c2b4` | Secondary text |
| `--color-ink-soft` | `#8d92a0` | Tertiary text |
| `--color-accent` | `#6883e8` | Links (brighter for AA contrast) |
| `--color-accent-strong` | `#8aa1ee` | Emphasized accent |
| `--color-warm` | `#d96f48` | Warnings |
| `--color-good` | `#4ca06e` | Success |
| `--color-bad` | `#d27773` | Errors |
### 1.2 Always-Dark Tokens (terminal surfaces)
Terminals, console modal, log streams **stay dark** in all themes — readable green-on-black doesn't translate to light.
| Token | Tailwind Class | Hex | Usage |
|-------|--------------|-----|-------|
| `--color-bg` | `bg-bg` | `rgb(9 9 11)` / zinc-950 | Terminal background |
| `--color-bg-elev` | `bg-bg-elev` | `rgb(24 24 27)` / zinc-900 | Elevated terminal surfaces |
| `--color-bg-card` | `bg-bg-card` | `rgb(39 39 42)` / zinc-800 | Terminal cards |
| `--color-line-strong` | `border-line-strong` | `rgb(63 63 70)` / zinc-700 | Strong borders |
| `--color-ink-mute` | `text-ink-mute` | `rgb(161 161 170)` / zinc-400 | Muted text |
| `--color-ink-dim` | `text-ink-dim` | `rgb(113 113 122)` / zinc-500 | Dim text |
### 1.3 Raw Zinc Usage Rules
**Use raw zinc for:**
- Borders: `border-zinc-700`, `border-zinc-800`
- Disabled states: `text-zinc-600`, `bg-zinc-800`
- Code highlighting: `bg-zinc-900`, `text-zinc-300`
- Terminal surfaces: `bg-zinc-950` (always-dark)
**NEVER use for surfaces:**
- `bg-zinc-900` or `bg-zinc-950` as page/card backgrounds — use `bg-surface`
- `text-zinc-50` or `text-zinc-100` as primary text — use `text-ink`
- `bg-white`, `bg-gray-50/100` for surfaces — use semantic tokens
### 1.4 Accessibility Contrast
| Pair | Ratio | WCAG |
|------|-------|------|
| `text-ink` on `bg-surface` (light) | ~14.5:1 | AAA |
| `text-ink` on `bg-surface` (dark) | ~15.8:1 | AAA |
| `text-ink-mid` on `bg-surface` (light) | ~5.2:1 | AA |
| `text-ink-mid` on `bg-surface` (dark) | ~5.9:1 | AA |
| `text-accent` on `bg-surface` (light) | ~4.8:1 | AA |
| `text-accent` on `bg-surface` (dark) | ~4.6:1 | AA |
---
## 2. Typography Scale
**Actual font stack** (from `globals.css`):
```
-apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", sans-serif
```
No custom fonts loaded — uses OS-native system stack.
| Size Token | Tailwind | Usage |
|------------|----------|-------|
| `text-[10px]` | 10px | Micro badges, tier labels |
| `text-[11px]` | 11px | Tooltip text |
| `text-xs` / `text-[12px]` | 12px | Badges, timestamps |
| `text-sm` / `text-[13px]` | 1314px | Secondary labels, node titles |
| `text-base` / `text-[16px]` | 16px | Body text |
| `text-lg` | 18px | Section headers |
| `text-xl` | 20px | Modal titles |
**Line height:** `leading-tight` (1.25) for headings, `leading-relaxed` (1.625) for body/tooltips.
---
## 3. Animation / Motion Tokens
**Defined in `canvas/src/styles/theme-tokens.css`** — use these, don't hardcode ms values.
| Token | Value | Usage |
|-------|-------|-------|
| `--mol-duration-fast` | 150ms | Hover states, button feedback |
| `--mol-duration-base` | 300ms | Standard transitions |
| `--mol-duration-spawn` | 350ms | Node spawn animation |
| `--mol-duration-root-complete` | 700ms | Org-deploy root glow |
| `--mol-duration-fit-view` | 800ms | Canvas fit-viewport |
| Token | Value | Usage |
|-------|-------|-------|
| `--mol-easing-standard` | `cubic-bezier(0.2, 0, 0, 1)` | Default ease |
| `--mol-easing-bounce-out` | `cubic-bezier(0.2, 0.8, 0.2, 1.05)` | Node spawn bounce |
| `--mol-easing-emphasize` | `cubic-bezier(0.3, 0, 0, 1)` | Modal/drawer enter |
**CSS usage:**
```css
/* Good — reference the token */
transition: all var(--mol-duration-fast) ease;
/* Bad — hardcoded value */
transition: all 150ms ease;
```
---
## 4. Component Patterns (Verified)
### 4.1 Buttons
```tsx
// Primary — accent background, ink text
<button className="bg-accent hover:bg-accent/90 active:scale-95
text-ink px-4 py-2 rounded-md text-sm font-medium
focus-visible:ring-2 focus-visible:ring-blue-500
focus-visible:ring-offset-2 focus-visible:ring-offset-zinc-900
disabled:opacity-50 disabled:cursor-not-allowed">
Primary
</button>
// Secondary — surface-card background, border-line
<button className="bg-surface-card hover:bg-surface-elevated border border-line
text-ink px-4 py-2 rounded-md text-sm font-medium
focus-visible:ring-2 focus-visible:ring-blue-500
focus-visible:ring-offset-2 focus-visible:ring-offset-zinc-900">
Secondary
</button>
// Ghost — no background, hover surface
<button className="hover:bg-surface-card text-ink-mid hover:text-ink
px-4 py-2 rounded-md text-sm font-medium">
Ghost
</button>
// Danger — bad color, requires confirmation dialog
<button className="bg-bad hover:bg-bad/90 text-white px-4 py-2
rounded-md text-sm font-medium">
Delete
</button>
```
**States:** default, hover, active (`scale-95`), focus (`ring-2 ring-blue-500 ring-offset-2 ring-offset-zinc-900`), disabled (`opacity-50 cursor-not-allowed`).
### 4.2 Inputs
```tsx
// Text input — use semantic tokens for surfaces
<input
className="bg-surface-sunken border border-line text-ink
placeholder:text-ink-soft px-3 py-2 rounded-md text-sm
focus:outline-none focus:ring-2 focus:ring-blue-500
focus:border-transparent
disabled:opacity-50 disabled:cursor-not-allowed"
placeholder="Enter workspace name"
/>
// Error state
<input
className="border-bad focus:ring-bad"
aria-invalid="true"
aria-describedby="error-message"
/>
```
**Label:** `text-sm font-medium text-ink mb-1`
**Error:** `text-xs text-bad mt-1`
### 4.3 Cards
```tsx
// Workspace node card (from WorkspaceNode.tsx)
<div className="bg-surface-sunken/90 border border-line/80
rounded-xl p-3.5 py-2.5
hover:border-zinc-500/60 shadow-lg shadow-black/30
focus-visible:ring-2 focus-visible:ring-accent/70
focus-visible:ring-offset-1 focus-visible:ring-offset-zinc-950">
```
### 4.4 Modals (Radix Dialog)
```tsx
// Backdrop
<div className="fixed inset-0 bg-black/70 backdrop-blur-sm z-50"
aria-hidden="true" />
// Dialog — use surface-card + border-line
<div className="bg-surface-card border border-line rounded-xl
shadow-2xl p-6 max-w-md w-full mx-4">
{/* Modal content */}
</div>
```
Note: Uses `--color-surface-sunken` for sunken areas (node cards). Cards use `bg-surface-card`.
**Important:** Use `@radix-ui/react-dialog` — it provides WCAG 2.1 compliance automatically (focus trap, Escape key, aria-modal, aria-labelledby).
### 4.5 Tooltips
**Verified implementation** (`canvas/src/components/Tooltip.tsx`):
```tsx
// Trigger wraps children
<span aria-describedby="tooltip-id">
{children}
</span>
// Tooltip portal (shows on hover + focus, 400ms delay)
<div id="tooltip-id"
role="tooltip"
className="fixed z-[9999] max-w-[400px] max-h-[300px] overflow-y-auto
px-3 py-2 bg-surface-card border border-line
rounded-lg shadow-2xl shadow-black/60 pointer-events-none">
<div className="text-[11px] text-ink whitespace-pre-wrap break-words leading-relaxed">
{text}
</div>
</div>
```
**WCAG 1.4.13 compliance:** Escape key dismisses tooltip without moving pointer/focus.
### 4.6 Theme Switching
Use `useTheme()` hook from `theme-provider.tsx`:
```tsx
import { useTheme } from "@/lib/theme-provider";
function ThemeToggle() {
const { theme, resolvedTheme, setTheme } = useTheme();
return (
<select
value={theme}
onChange={(e) => setTheme(e.target.value as ThemePreference)}
>
<option value="system">System</option>
<option value="light">Light</option>
<option value="dark">Dark</option>
</select>
);
}
```
**Theme types:**
```ts
type ThemePreference = "system" | "light" | "dark";
type ResolvedTheme = "light" | "dark";
```
**Cookie:** `mol_theme` with `Domain=.moleculesai.app` — persists across surfaces.
---
## 5. Accessibility Rules (WCAG 2.1 AA) — VERIFIED
### 5.1 Focus Management ✅ VERIFIED
- All interactive elements have `focus-visible:ring-2 focus-visible:ring-blue-500 focus-visible:ring-offset-2 focus-visible:ring-offset-zinc-950`
- No `outline-none` without equivalent focus ring
- Radix Dialog traps focus automatically
### 5.2 Semantic HTML ✅ VERIFIED
- Buttons use `<button>` — verified in WorkspaceNode.tsx, ContextMenu.tsx
- Form inputs have associated `<label>` patterns
- Radix Dialog provides role="dialog" + aria-modal
### 5.3 ARIA ✅ VERIFIED
- Icon-only buttons: `aria-label` with descriptive text (not "X")
- Example: `aria-label="Extract ${name} from team"` in WorkspaceNode.tsx
- Live regions: `aria-live="polite"` on Toast component
- Modals: Radix provides `role="dialog"`, `aria-modal="true"`, `aria-labelledby`
- Error messages: `aria-invalid="true"`, `aria-describedby` linking to error text
- Tooltips: `role="tooltip"` + `aria-describedby` on trigger
### 5.4 Keyboard Navigation ✅ VERIFIED
- ContextMenu: ArrowUp/Down wraps, Enter/Space selects, Escape closes, Tab closes
- Modals: Escape closes (Radix), focus returns to trigger
- `prefers-reduced-motion` ✅ (verified in globals.css)
### 5.5 Color Independence ✅
- Status indicators use text labels + icons, not color alone
- `STATUS_CONFIG` has text labels: "Online", "Offline", "Failed", etc.
---
## 6. React Flow Canvas Specifics
Canvas uses `@xyflow/react` (React Flow).
### Canvas Container ✅ VERIFIED
```tsx
// Canvas.tsx wraps ReactFlow with:
<ReactFlow
aria-label="Molecule AI workspace canvas"
// ...
/>
```
### Node Accessibility ✅ VERIFIED
- `role="button"` on workspace node cards
- `tabIndex={0}` for keyboard focus
- `aria-pressed` for selection state
- `aria-label` with workspace name + status
### Skip Link ✅ VERIFIED
```tsx
<a href="#canvas-main">Skip to canvas</a>
<main id="canvas-main" role="main">
```
---
## 7. Enforcement Checklist
### Color Token Rules
- [x] No `bg-white` / `bg-zinc-50` for surfaces — use `bg-surface`
- [x] No `text-zinc-50` / `text-zinc-100` for surfaces — use `text-ink`
- [x] No `bg-zinc-900` / `bg-zinc-950` for surfaces — use `bg-surface` or `bg-surface-card`
- [x] Raw zinc OK for: borders, disabled states, code, terminal surfaces
### Accessibility Rules
- [x] All buttons have focus rings (verified in tests)
- [x] All modals use Radix Dialog (verified)
- [x] All tooltips use `role="tooltip"` + `aria-describedby` (verified)
- [x] No `outline-none` without focus ring (verified)
- [x] All inputs have visible labels (verified pattern)
- [x] Contrast ratios at 4.5:1 minimum (verified above)
- [x] `prefers-reduced-motion` suppresses all animations (verified in globals.css)
- [x] Context menu has keyboard navigation (verified in ContextMenu.keyboard.test.tsx)
- [x] Theme switching works: System/Light/Dark modes verified
---
## 8. Canvas Architecture (Verified)
**Stack:**
- `@xyflow/react` v12 (React Flow) — node/edge rendering
- Next.js 14 App Router
- Tailwind v4 with CSS custom properties
- Zustand for state management
**Directory Structure:**
```
canvas/src/
├── components/ # Canvas.tsx, Toolbar.tsx, ContextMenu.tsx, SidePanel.tsx, WorkspaceNode.tsx, A2AEdge.tsx
├── stores/ # secrets-store.ts (only store)
├── hooks/ # useSocketEvent.ts, useTemplateDeploy.tsx, useWorkspaceName.ts
├── lib/ # api.ts, auth.ts, canvas-actions.ts, design-tokens.ts, theme.ts, theme-provider.tsx
└── app/ # Next.js App Router
```
## 9. Known Issues (Technical Debt)
### Performance Issues
- **secrets-store.ts getGrouped() selector** — Creates new objects every call (Object.fromEntries + arrays) — not memoized. Causes performance issues with frequent re-renders. Needs selector optimization.
### Code Quality
- Check for `any` types in canvas/ directory
- Verify pre-commit hook actually fails on 'use client' violations (unverified)
- Verify all Zustand selectors avoid object creation (see getGrouped issue above)
- Check 'use client' directive on hook-using components
### Testing
- Add axe-core integration for automated accessibility testing
- Visual regression tests — no screenshot tests exist yet (KI-006)
- Target >80% test coverage on changed files
## 10. Remaining Open Items
### Accessibility Gaps
1. **Screen reader announcements** — Node/edge changes not announced. Need `aria-live="polite"` region.
2. **Keyboard shortcut help dialog** — No dedicated dialog. Shortcuts exist in `useKeyboardShortcuts.ts` but no `aria-describedby` hints on buttons.
3. **Edge anchor accessibility** — React Flow handles purely visual. Need ARIA annotations for screen readers.
4. **Drag-and-drop keyboard alternative** — Mouse only. Need keyboard equivalent for node rearrangement.
### Performance
5. **secrets-store.ts getGrouped()** — Not memoized, creates new objects every call.

View File

@ -73,7 +73,7 @@ These are applied after CORS middleware on every response.
## 14. No Exposed Database Ports ## 14. No Exposed Database Ports
Postgres and Redis must not expose host ports. They communicate exclusively over the internal Docker network (`molecule-core-net`). Use `docker compose exec` for direct access during development. Postgres and Redis must not expose host ports. They communicate exclusively over the internal Docker network (`molecule-monorepo-net`). Use `docker compose exec` for direct access during development.
## Related Docs ## Related Docs

View File

@ -73,19 +73,19 @@ runner-wide setting, not per-job. Source: gitea/act_runner config docs
Flipping the global `container.network` to `bridge` would break every Flipping the global `container.network` to `bridge` would break every
other workflow in the repo (cache server discovery, other workflow in the repo (cache server discovery,
`molecule-core-net` peer access during integration tests, etc.) — `molecule-monorepo-net` peer access during integration tests, etc.) —
unacceptable blast radius for a per-test bug. unacceptable blast radius for a per-test bug.
## Fix shape ## Fix shape
`handlers-postgres-integration.yml` no longer uses `services: postgres:`. `handlers-postgres-integration.yml` no longer uses `services: postgres:`.
It launches a sibling postgres container manually on the existing It launches a sibling postgres container manually on the existing
`molecule-core-net` bridge network with a per-run unique name: `molecule-monorepo-net` bridge network with a per-run unique name:
```yaml ```yaml
env: env:
PG_NAME: pg-handlers-${{ github.run_id }}-${{ github.run_attempt }} PG_NAME: pg-handlers-${{ github.run_id }}-${{ github.run_attempt }}
PG_NETWORK: molecule-core-net PG_NETWORK: molecule-monorepo-net
steps: steps:
- name: Start sibling Postgres on bridge network - name: Start sibling Postgres on bridge network
@ -117,7 +117,7 @@ host-network runner config. Translate using this same pattern:
1. Drop the `services:` block. 1. Drop the `services:` block.
2. Use `${{ github.run_id }}-${{ github.run_attempt }}` for unique 2. Use `${{ github.run_id }}-${{ github.run_attempt }}` for unique
container name. container name.
3. Launch on `molecule-core-net` (already trusted bridge in 3. Launch on `molecule-monorepo-net` (already trusted bridge in
`docker-compose.infra.yml`). `docker-compose.infra.yml`).
4. Read back the bridge IP via `docker inspect` and export as a step env. 4. Read back the bridge IP via `docker inspect` and export as a step env.
5. `if: always()` cleanup step at the end. 5. `if: always()` cleanup step at the end.
@ -131,7 +131,7 @@ in one place.
- Issue #88 (closed by #92): localhost → 127.0.0.1 fix that unmasked - Issue #88 (closed by #92): localhost → 127.0.0.1 fix that unmasked
this collision; the IPv6 fix is correct, port collision is the new this collision; the IPv6 fix is correct, port collision is the new
layer. layer.
- Issue #94 created `molecule-core-net` + `alpine:latest` as - Issue #94 created `molecule-monorepo-net` + `alpine:latest` as
prereqs. prereqs.
- Saved memory `feedback_act_runner_github_server_url` documents - Saved memory `feedback_act_runner_github_server_url` documents
another act_runner-vs-GHA divergence (server URL). another act_runner-vs-GHA divergence (server URL).

View File

@ -5,7 +5,7 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
ROOT_DIR="$(cd "$SCRIPT_DIR/../.." && pwd)" ROOT_DIR="$(cd "$SCRIPT_DIR/../.." && pwd)"
echo "==> Ensuring shared docker network exists..." echo "==> Ensuring shared docker network exists..."
docker network create molecule-core-net 2>/dev/null || true docker network create molecule-monorepo-net 2>/dev/null || true
# Populate the template / plugin registry. # Populate the template / plugin registry.
# workspace-configs-templates/, org-templates/, and plugins/ are intentionally # workspace-configs-templates/, org-templates/, and plugins/ are intentionally

View File

@ -1,5 +1,5 @@
{ {
"_comment": "OSS surface registry — every repo listed here MUST be public on git.moleculesai.app. Layer-3 customer/private templates are NOT registered here; they are handled at provision-time via the per-tenant credential resolver (see internal#102 RFC). 'main' refs are pinned to tags before broad rollout.", "_comment": "Pin refs to release tags for reproducible builds. 'main' is OK while all repos are internal.",
"version": 1, "version": 1,
"plugins": [ "plugins": [
{"name": "browser-automation", "repo": "molecule-ai/molecule-ai-plugin-browser-automation", "ref": "main"}, {"name": "browser-automation", "repo": "molecule-ai/molecule-ai-plugin-browser-automation", "ref": "main"},
@ -40,6 +40,7 @@
{"name": "free-beats-all", "repo": "molecule-ai/molecule-ai-org-template-free-beats-all", "ref": "main"}, {"name": "free-beats-all", "repo": "molecule-ai/molecule-ai-org-template-free-beats-all", "ref": "main"},
{"name": "medo-smoke", "repo": "molecule-ai/molecule-ai-org-template-medo-smoke", "ref": "main"}, {"name": "medo-smoke", "repo": "molecule-ai/molecule-ai-org-template-medo-smoke", "ref": "main"},
{"name": "molecule-worker-gemini", "repo": "molecule-ai/molecule-ai-org-template-molecule-worker-gemini", "ref": "main"}, {"name": "molecule-worker-gemini", "repo": "molecule-ai/molecule-ai-org-template-molecule-worker-gemini", "ref": "main"},
{"name": "reno-stars", "repo": "molecule-ai/molecule-ai-org-template-reno-stars", "ref": "main"},
{"name": "ux-ab-lab", "repo": "molecule-ai/molecule-ai-org-template-ux-ab-lab", "ref": "main"}, {"name": "ux-ab-lab", "repo": "molecule-ai/molecule-ai-org-template-ux-ab-lab", "ref": "main"},
{"name": "mock-bigorg", "repo": "molecule-ai/molecule-ai-org-template-mock-bigorg", "ref": "main"} {"name": "mock-bigorg", "repo": "molecule-ai/molecule-ai-org-template-mock-bigorg", "ref": "main"}
] ]

View File

@ -268,11 +268,10 @@ molecule-mcp = "molecule_runtime.mcp_cli:main"
[tool.setuptools.packages.find] [tool.setuptools.packages.find]
where = ["."] where = ["."]
include = ["molecule_runtime*", "plugins_registry*"] include = ["molecule_runtime*"]
[tool.setuptools.package-data] [tool.setuptools.package-data]
"molecule_runtime" = ["py.typed"] "molecule_runtime" = ["py.typed"]
"plugins_registry" = ["py.typed"]
""" """
@ -474,18 +473,6 @@ def main() -> int:
py_files = copy_tree_filtered(src, pkg_dir) py_files = copy_tree_filtered(src, pkg_dir)
print(f"[build] copied {len(py_files)} .py files") print(f"[build] copied {len(py_files)} .py files")
# Install plugins_registry/ at the wheel TOP LEVEL so that plugin adapter
# code (workspace-template-*) can use bare `from plugins_registry import ...`.
# The molecule-runtime package (molecule_runtime/) also ships it at
# molecule_runtime/plugins_registry/ (satisfies the rewritten
# `from molecule_runtime.plugins_registry import ...` in adapter_base.py).
# Both copies coexist: they serve different import namespaces.
plugins_src = src / "plugins_registry"
plugins_dst = out / "plugins_registry"
if plugins_src.is_dir():
shutil.copytree(plugins_src, plugins_dst)
print(f"[build] installed plugins_registry/ at top level (bare-import shim)")
# Ensure top-level package marker exists. workspace/ doesn't have one # Ensure top-level package marker exists. workspace/ doesn't have one
# (it's not a package in monorepo), but the published artifact must. # (it's not a package in monorepo), but the published artifact must.
init = pkg_dir / "__init__.py" init = pkg_dir / "__init__.py"

View File

@ -8,24 +8,27 @@
# Requires: git, jq (lighter than python3 — ~2MB vs ~50MB in Alpine) # Requires: git, jq (lighter than python3 — ~2MB vs ~50MB in Alpine)
# #
# Auth (optional): # Auth (optional):
# Post-2026-05-08 (#192): every repo in manifest.json is public on # When MOLECULE_GITEA_TOKEN is set, embed it as the basic-auth password so
# git.moleculesai.app. Anonymous clone works for the entire registered # private Gitea repos clone successfully. When unset, clone anonymously
# set. The OSS-surface contract is recorded in manifest.json's _comment # (works only for repos that are public on git.moleculesai.app).
# — Layer-3 customer/private templates (e.g. reno-stars) are NOT in the
# manifest; they are handled at provision-time via the per-tenant
# credential resolver (internal#102 RFC).
# #
# MOLECULE_GITEA_TOKEN is therefore optional today. Kept supported for # This is the path the publish-workspace-server-image.yml workflow uses:
# two reasons: (a) historical CI configs that still inject # it injects AUTO_SYNC_TOKEN (devops-engineer persona PAT, repo:read on
# AUTO_SYNC_TOKEN remain harmless, (b) reserved for the case where a # the molecule-ai org) so the in-CI pre-clone step succeeds for ALL
# private internal-only template is later registered via a ci-readonly # manifest entries — including the 5 private workspace-template-* repos
# team grant — review must explicitly sign off on that, since it # (codex, crewai, deepagents, gemini-cli, langgraph) and all 7
# violates the public-OSS-surface contract. # org-template-* repos.
# #
# The token (when set) never enters the Docker image: this script runs # The token never enters the Docker image: this script runs in the
# in the trusted CI context BEFORE `docker buildx build`, populates # trusted CI context BEFORE `docker buildx build`, populates
# .tenant-bundle-deps/, then `Dockerfile.tenant` COPYs from there with # .tenant-bundle-deps/, then `Dockerfile.tenant` COPYs from there with
# the .git directories already stripped (see line ~67 below). # the .git directories already stripped (see line ~67 below).
#
# For backward compatibility — and so a fresh clone works without
# secrets when (eventually) the workspace-template-* repos flip public —
# the unset path remains a plain anonymous HTTPS clone. That path will
# FAIL with "could not read Username" on private repos today; CI MUST
# set MOLECULE_GITEA_TOKEN.
set -euo pipefail set -euo pipefail

View File

@ -24,7 +24,7 @@ echo "=== NUKE ==="
docker compose -f "$ROOT/docker-compose.yml" down -v 2>/dev/null || true docker compose -f "$ROOT/docker-compose.yml" down -v 2>/dev/null || true
docker ps -a --format "{{.Names}}" | grep "^ws-" | xargs -r docker rm -f 2>/dev/null || true docker ps -a --format "{{.Names}}" | grep "^ws-" | xargs -r docker rm -f 2>/dev/null || true
docker volume ls --format "{{.Name}}" | grep "^ws-" | xargs -r docker volume rm 2>/dev/null || true docker volume ls --format "{{.Name}}" | grep "^ws-" | xargs -r docker volume rm 2>/dev/null || true
docker network rm molecule-core-net 2>/dev/null || true docker network rm molecule-monorepo-net 2>/dev/null || true
echo " cleaned" echo " cleaned"
echo "=== POPULATE MANIFEST DIRS ===" echo "=== POPULATE MANIFEST DIRS ==="

View File

@ -1,5 +1,2 @@
# The compiled binary, not the cmd/server package. # The compiled binary, not the cmd/server package.
/server /server
# air live-reload build cache (Dockerfile.dev + docker-compose.dev.yml).
/tmp/

View File

@ -15,14 +15,8 @@
FROM golang:1.25-alpine FROM golang:1.25-alpine
# air + git (for go mod) + ca-certs (for TLS) + tzdata (for time-zone DB) # air + git (for go mod) + ca-certs (for TLS) + tzdata (for time-zone DB).
# + docker-cli + docker-cli-buildx so the platform binary can shell out to RUN apk add --no-cache git ca-certificates tzdata wget \
# /var/run/docker.sock (bind-mounted from host) for local-build provisioning.
# docker-cli alone is insufficient: alpine's docker-cli enables BuildKit by
# default but ships without buildx, producing
# `ERROR: BuildKit is enabled but the buildx component is missing or broken`
# on every `docker build`. docker-cli-buildx provides the buildx subcommand.
RUN apk add --no-cache git ca-certificates tzdata wget docker-cli docker-cli-buildx \
&& go install github.com/air-verse/air@latest && go install github.com/air-verse/air@latest
WORKDIR /app/workspace-server WORKDIR /app/workspace-server
@ -37,7 +31,7 @@ RUN go mod download
# block) so the Dockerfile doesn't need to COPY it. air watches the # block) so the Dockerfile doesn't need to COPY it. air watches the
# bind-mounted dir for changes. # bind-mounted dir for changes.
ENV CGO_ENABLED=0 ENV CGO_ENABLED=1
ENV GOFLAGS="-buildvcs=false" ENV GOFLAGS="-buildvcs=false"
# Run air with the .air.toml in the bind-mounted source dir. # Run air with the .air.toml in the bind-mounted source dir.

View File

@ -31,7 +31,7 @@ import (
// External plugins — each registers EnvMutator(s) that run at workspace // External plugins — each registers EnvMutator(s) that run at workspace
// provision time. Loaded via soft-dep gates in main() so self-hosters // provision time. Loaded via soft-dep gates in main() so self-hosters
// without per-agent identity configured keep working. // without per-agent identity configured keep working.
ghidentity "go.moleculesai.app/plugin/gh-identity/pluginloader" ghidentity "github.com/Molecule-AI/molecule-ai-plugin-gh-identity/pluginloader"
"github.com/Molecule-AI/molecule-monorepo/platform/pkg/provisionhook" "github.com/Molecule-AI/molecule-monorepo/platform/pkg/provisionhook"
) )

View File

@ -4,7 +4,7 @@ go 1.25.0
require ( require (
github.com/DATA-DOG/go-sqlmock v1.5.2 github.com/DATA-DOG/go-sqlmock v1.5.2
go.moleculesai.app/plugin/gh-identity v0.0.0-20260509010445-788988195fce github.com/Molecule-AI/molecule-ai-plugin-gh-identity v0.0.0-20260424033845-4fd5ac7be30f
github.com/alicebob/miniredis/v2 v2.37.0 github.com/alicebob/miniredis/v2 v2.37.0
github.com/creack/pty v1.1.24 github.com/creack/pty v1.1.24
github.com/docker/docker v28.5.2+incompatible github.com/docker/docker v28.5.2+incompatible

View File

@ -490,14 +490,7 @@ func (h *WorkspaceHandler) proxyA2ARequest(ctx context.Context, workspaceID stri
if logActivity && deliveryConfirmed { if logActivity && deliveryConfirmed {
h.logA2ASuccess(ctx, workspaceID, callerID, body, respBody, a2aMethod, resp.StatusCode, durationMs) h.logA2ASuccess(ctx, workspaceID, callerID, body, respBody, a2aMethod, resp.StatusCode, durationMs)
} }
// Preserve the actual HTTP status code and any body bytes already read. return 0, nil, &proxyA2AError{
// Previously this returned (0, nil, error) which discarded both.
// Preserving them allows executeDelegation's new condition
// proxyErr != nil && len(respBody) > 0 && status >= 200 && status < 300
// to correctly route delivery-confirmed responses (where the agent completed
// the work but the TCP connection dropped before the full body was received)
// to success instead of failure (#159).
return resp.StatusCode, respBody, &proxyA2AError{
Status: http.StatusBadGateway, Status: http.StatusBadGateway,
Response: gin.H{ Response: gin.H{
"error": "failed to read agent response", "error": "failed to read agent response",

View File

@ -342,18 +342,6 @@ func (h *DelegationHandler) executeDelegation(sourceID, targetID, delegationID s
} }
} }
// When proxyA2ARequest returns an error but we have a non-empty response body
// with a 2xx status code, the agent completed the work successfully — the error
// is a delivery/transport error (e.g., connection reset after response was
// received). Treat as success: the response body is valid and the work is done.
// This prevents "retry storms" where the canvas sees error + Restart-workspace
// suggestion even though the delegation actually completed.
if isDeliveryConfirmedSuccess(proxyErr, status, respBody) {
log.Printf("Delegation %s: completed with delivery error (status=%d, respBody=%d bytes, proxyErr=%v) — treating as success",
delegationID, status, len(respBody), proxyErr.Error())
goto handleSuccess
}
if proxyErr != nil { if proxyErr != nil {
log.Printf("Delegation %s: failed — %s", delegationID, proxyErr.Error()) log.Printf("Delegation %s: failed — %s", delegationID, proxyErr.Error())
h.updateDelegationStatus(sourceID, delegationID, "failed", proxyErr.Error()) h.updateDelegationStatus(sourceID, delegationID, "failed", proxyErr.Error())
@ -373,8 +361,6 @@ func (h *DelegationHandler) executeDelegation(sourceID, targetID, delegationID s
return return
} }
handleSuccess:
// 202 + {queued: true} means the target was busy and the proxy // 202 + {queued: true} means the target was busy and the proxy
// enqueued the request for the next drain tick — NOT a completion. // enqueued the request for the next drain tick — NOT a completion.
// Treat it as such: write a clean 'queued' activity row with no // Treat it as such: write a clean 'queued' activity row with no
@ -685,34 +671,6 @@ func isTransientProxyError(err *proxyA2AError) bool {
return false return false
} }
// isDeliveryConfirmedSuccess reports whether the proxy's `(status, body, err)`
// triple represents a delivery-confirmed success: the proxy hit a transport-
// layer error AFTER receiving a complete 2xx response with a non-empty body.
// In that case the agent did the work — the error is on the wire, not in the
// agent — so the delegation should be marked succeeded rather than failed
// (preventing the retry-storm + restart-suggest cascade described in #159).
//
// Caller invariants:
// - proxyErr != nil: a delivery error fired (e.g. connection reset).
// - len(respBody) > 0: a response body was received before the error.
// - 200 <= status < 300: the partial response carried a 2xx code.
//
// All three must hold. nil proxyErr → no decision to make (success path
// already chosen upstream). Empty body → no work-result to recover. Non-2xx →
// the agent itself signalled failure or transient state; don't promote it.
func isDeliveryConfirmedSuccess(proxyErr *proxyA2AError, status int, respBody []byte) bool {
if proxyErr == nil {
return false
}
if len(respBody) == 0 {
return false
}
if status < 200 || status >= 300 {
return false
}
return true
}
// isQueuedProxyResponse reports whether the proxy returned a body shaped like // isQueuedProxyResponse reports whether the proxy returned a body shaped like
// `{"queued": true, "queue_id": ..., "queue_depth": ..., "message": ...}` — // `{"queued": true, "queue_id": ..., "queue_depth": ..., "message": ...}` —
// the busy-target enqueue path in a2a_proxy_helpers.go. Caller checks this // the busy-target enqueue path in a2a_proxy_helpers.go. Caller checks this

View File

@ -5,10 +5,8 @@ import (
"context" "context"
"encoding/json" "encoding/json"
"fmt" "fmt"
"net"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"sync"
"testing" "testing"
"time" "time"
@ -378,44 +376,6 @@ func TestIsTransientProxyError_RetriesOnRestartRaceStatuses(t *testing.T) {
} }
} }
// TestIsDeliveryConfirmedSuccess — regression guard for #159: the proxy can
// return a complete 2xx body and THEN raise a transport error (e.g. the TCP
// connection drops after the response is received but before close). In that
// case the agent did the work; marking the delegation "failed" causes the
// retry-storm + Restart-workspace cascade described in #159. The new helper
// distinguishes this from genuine failures.
func TestIsDeliveryConfirmedSuccess(t *testing.T) {
connErr := &proxyA2AError{Status: http.StatusOK, Response: gin.H{}}
cases := []struct {
name string
proxyErr *proxyA2AError
status int
body []byte
expect bool
}{
// The new branch: 2xx + body + transport error → recover as success.
{"200 + body + connreset (THE bug fix path)", connErr, http.StatusOK, []byte(`{"text":"ok"}`), true},
{"299 + body + connreset (boundary high)", connErr, 299, []byte(`{"text":"ok"}`), true},
{"200 + body + connreset (boundary low)", connErr, 200, []byte(`{"x":1}`), true},
// Negative cases: any one of the three preconditions failing → false.
{"nil proxyErr (no decision to make)", nil, http.StatusOK, []byte(`{"text":"ok"}`), false},
{"empty body (no work-result to recover)", connErr, http.StatusOK, []byte{}, false},
{"nil body (no work-result to recover)", connErr, http.StatusOK, nil, false},
{"4xx with body — agent signalled failure, do not promote", connErr, http.StatusBadRequest, []byte(`{"err":"bad"}`), false},
{"5xx with body — agent signalled failure, do not promote", connErr, http.StatusInternalServerError, []byte(`{"err":"crash"}`), false},
{"3xx with body — redirect, not a result", connErr, 301, []byte(`{"loc":"/x"}`), false},
{"199 status (under 200) — not a 2xx", connErr, 199, []byte(`{"x":1}`), false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := isDeliveryConfirmedSuccess(tc.proxyErr, tc.status, tc.body); got != tc.expect {
t.Errorf("isDeliveryConfirmedSuccess(%v, %d, %q) = %v, want %v",
tc.proxyErr, tc.status, string(tc.body), got, tc.expect)
}
})
}
}
func TestIsQueuedProxyResponse(t *testing.T) { func TestIsQueuedProxyResponse(t *testing.T) {
// Regression guard for the chat-leak bug: when the proxy returns // Regression guard for the chat-leak bug: when the proxy returns
// 202 with a queued-shape body, executeDelegation must classify it // 202 with a queued-shape body, executeDelegation must classify it
@ -958,308 +918,3 @@ func TestInsertDelegationOutcome_ZeroValueIsUnknown(t *testing.T) {
t.Errorf("insertOutcomeUnknown must not collide with insertOK") t.Errorf("insertOutcomeUnknown must not collide with insertOK")
} }
} }
// ==================== executeDelegation — delivery-confirmed proxy error regression tests ====================
//
// These test the fix for issue #159: when proxyA2ARequest returns an error but we have a
// non-empty response body with a 2xx status code, executeDelegation must treat it as success.
// The error is a delivery/transport error (e.g., connection reset after response was received).
// Previously, executeDelegation marked these as "failed" even though the work was done,
// causing retry storms and "error" rendering in canvas despite the response being available.
//
// Test strategy: spin up a mock A2A agent server, set up the source/target DB rows, call
// executeDelegation directly, and verify the activity_logs status and delegation status.
const testDelegationID = "del-159-test"
const testSourceID = "ws-source-159"
const testTargetID = "ws-target-159"
// expectExecuteDelegationBase sets up sqlmock expectations for the DB queries that
// executeDelegation always makes, regardless of outcome.
func expectExecuteDelegationBase(mock sqlmock.Sqlmock) {
// updateDelegationStatus: dispatched
// Uses prefix match — sqlmock regexes match the full query string.
mock.ExpectExec("UPDATE activity_logs SET status").
WithArgs("dispatched", "", testSourceID, testDelegationID).
WillReturnResult(sqlmock.NewResult(0, 1))
// CanCommunicate (source=target self-call is always allowed — no DB lookup needed)
// resolveAgentURL: reads ws:{id}:url from Redis, falls back to DB for target
mock.ExpectQuery("SELECT url, status FROM workspaces WHERE id = ").
WithArgs(testTargetID).
WillReturnRows(sqlmock.NewRows([]string{"url", "status"}).AddRow("", "online"))
}
// expectExecuteDelegationSuccess sets up expectations for a completed delegation.
func expectExecuteDelegationSuccess(mock sqlmock.Sqlmock, respBody string) {
// INSERT activity_logs for delegation completion (response_body status = 'completed')
mock.ExpectExec("INSERT INTO activity_logs").
WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), "completed").
WillReturnResult(sqlmock.NewResult(0, 1))
// updateDelegationStatus: completed
mock.ExpectExec("UPDATE activity_logs SET status").
WithArgs("completed", "", testSourceID, testDelegationID).
WillReturnResult(sqlmock.NewResult(0, 1))
}
// expectExecuteDelegationFailed sets up expectations for a failed delegation.
func expectExecuteDelegationFailed(mock sqlmock.Sqlmock) {
// INSERT activity_logs for delegation failure (response_body status = 'failed')
mock.ExpectExec("INSERT INTO activity_logs").
WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), "failed").
WillReturnResult(sqlmock.NewResult(0, 1))
// updateDelegationStatus: failed
mock.ExpectExec("UPDATE activity_logs SET status").
WithArgs("failed", sqlmock.AnyArg(), testSourceID, testDelegationID).
WillReturnResult(sqlmock.NewResult(0, 1))
}
// TestExecuteDelegation_DeliveryConfirmedProxyError_TreatsAsSuccess is the primary regression
// test for issue #159. The scenario:
// - Attempt 1: server sends 200 OK headers + partial body, then closes connection.
// proxyA2ARequest: body read gets io.EOF (partial body read), returns (200, <partial>, BadGateway).
// isTransientProxyError(BadGateway) = TRUE → retry.
// - Attempt 2: server does the same thing (closes after partial body).
// proxyA2ARequest: same (200, <partial>, BadGateway).
// isTransientProxyError(BadGateway) = TRUE → retry AGAIN (but outer context will fire soon,
// or we get one more attempt). For the test we let it run.
// POST-FIX: the executeDelegation new condition sees status=200, body=<partial>, err!=nil
// and routes to handleSuccess immediately.
//
// The key pre/post-fix difference: pre-fix, executeDelegation received status=0 (hardcoded)
// even when the server sent 200, so the condition always failed. Post-fix, status=200 is
// preserved through the error return path (proxyA2ARequest now returns resp.StatusCode, respBody).
// In this test the retry ultimately succeeds (server eventually sends full body), but
// the critical assertion is that a 2xx partial-body delivery-confirmed response is never
// classified as "failed" — it always routes to success.
func TestExecuteDelegation_DeliveryConfirmedProxyError_TreatsAsSuccess(t *testing.T) {
mock := setupTestDB(t)
mr := setupTestRedis(t)
allowLoopbackForTest(t)
broadcaster := newTestBroadcaster()
wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
dh := NewDelegationHandler(wh, broadcaster)
// Server that sends a 200 response with declared Content-Length but closes
// the connection before sending all bytes. Go's http.Client sees io.EOF on
// the body read. proxyA2ARequest captures the partial body + status=200 and
// returns (200, <partial>, error). executeDelegation's new condition sees
// status=200 + body > 0 + error != nil → routes to handleSuccess.
var wg sync.WaitGroup
wg.Add(1)
ln, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("failed to listen: %v", err)
}
defer ln.Close()
go func() {
defer wg.Done()
conn, err := ln.Accept()
if err != nil {
return
}
defer conn.Close()
// Consume the HTTP request
buf := make([]byte, 2048)
conn.Read(buf)
// Send 200 OK with Content-Length: 100 but only 74 bytes of body
// (less than declared length → io.LimitReader returns io.EOF after reading all 74)
resp := "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: 100\r\n\r\n"
resp += `{"result":{"parts":[{"text":"work completed successfully"}]}}` // 74 bytes
conn.Write([]byte(resp))
// Close immediately — client gets io.EOF on body read
}()
agentURL := "http://" + ln.Addr().String()
mr.Set(fmt.Sprintf("ws:%s:url", testTargetID), agentURL)
allowLoopbackForTest(t)
expectExecuteDelegationBase(mock)
expectExecuteDelegationSuccess(mock, `{"result":{"parts":[{"text":"work completed successfully"}]}}`)
// Execute synchronously (not as a goroutine) so we can check DB state immediately.
// The handler fires it as goroutine; we call it directly for deterministic testing.
a2aBody, _ := json.Marshal(map[string]interface{}{
"jsonrpc": "2.0",
"id": "1",
"method": "message/send",
"params": map[string]interface{}{
"message": map[string]interface{}{
"role": "user",
"parts": []map[string]string{{"type": "text", "text": "do work"}},
},
},
})
dh.executeDelegation(testSourceID, testTargetID, testDelegationID, a2aBody)
time.Sleep(100 * time.Millisecond) // let DB writes settle
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// TestExecuteDelegation_ProxyErrorNon2xx_RemainsFailed verifies that the pre-fix failure
// path is unchanged when proxyA2ARequest returns a delivery-confirmed error with a non-2xx
// status code (e.g., 500 Internal Server Error with partial body read before connection drop).
// The new condition requires status >= 200 && status < 300, so non-2xx always routes to failure.
func TestExecuteDelegation_ProxyErrorNon2xx_RemainsFailed(t *testing.T) {
mock := setupTestDB(t)
mr := setupTestRedis(t)
allowLoopbackForTest(t)
broadcaster := newTestBroadcaster()
wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
dh := NewDelegationHandler(wh, broadcaster)
// Server returns 500 with declared Content-Length but closes connection early.
// proxyA2ARequest: reads 500 headers, partial body, then connection drop → body read error.
// Returns (500, <partial_body>, BadGateway).
// New condition: status=500 is NOT >= 200 && < 300 → routes to failure.
// isTransientProxyError(500) = false → no retry.
var wg sync.WaitGroup
wg.Add(1)
ln, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("failed to listen: %v", err)
}
defer ln.Close()
go func() {
defer wg.Done()
conn, err := ln.Accept()
if err != nil {
return
}
defer conn.Close()
buf := make([]byte, 2048)
conn.Read(buf)
// 500 with Content-Length: 100 but only ~60 bytes of body
resp := "HTTP/1.1 500 Internal Server Error\r\nContent-Type: application/json\r\nContent-Length: 100\r\n\r\n"
resp += `{"error":"agent crashed"}` // ~24 bytes, less than declared
conn.Write([]byte(resp))
// Close immediately — client gets io.EOF on body read
}()
agentURL := "http://" + ln.Addr().String()
mr.Set(fmt.Sprintf("ws:%s:url", testTargetID), agentURL)
allowLoopbackForTest(t)
expectExecuteDelegationBase(mock)
expectExecuteDelegationFailed(mock)
a2aBody, _ := json.Marshal(map[string]interface{}{
"jsonrpc": "2.0", "id": "1", "method": "message/send",
"params": map[string]interface{}{
"message": map[string]interface{}{
"role": "user",
"parts": []map[string]string{{"type": "text", "text": "do work"}},
},
},
})
dh.executeDelegation(testSourceID, testTargetID, testDelegationID, a2aBody)
time.Sleep(100 * time.Millisecond)
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// TestExecuteDelegation_ProxyErrorEmptyBody_RemainsFailed verifies that the pre-fix failure
// path is unchanged when proxyA2ARequest returns an error with a 2xx status but empty body.
// The new condition requires len(respBody) > 0, so empty body routes to failure.
func TestExecuteDelegation_ProxyErrorEmptyBody_RemainsFailed(t *testing.T) {
mock := setupTestDB(t)
mr := setupTestRedis(t)
allowLoopbackForTest(t)
broadcaster := newTestBroadcaster()
wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
dh := NewDelegationHandler(wh, broadcaster)
// Server returns 502 Bad Gateway — proxyA2ARequest returns 502, body="" (empty), error != nil.
// New condition: proxyErr != nil && len(respBody) > 0 && status >= 200 && status < 300
// → len(respBody) == 0 → condition FALSE → falls through to failure.
// isTransientProxyError(502) is TRUE → retry → same result → failure.
agentServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadGateway)
// No body — connection closes normally
}))
defer agentServer.Close()
mr.Set(fmt.Sprintf("ws:%s:url", testTargetID), agentServer.URL)
allowLoopbackForTest(t)
// First attempt: updateDelegationStatus(dispatched) — from expectExecuteDelegationBase
expectExecuteDelegationBase(mock)
// Second attempt (retry): updateDelegationStatus(dispatched) again
mock.ExpectExec("UPDATE activity_logs SET status").
WithArgs("dispatched", "", testSourceID, testDelegationID).
WillReturnResult(sqlmock.NewResult(0, 1))
// Failure: INSERT + UPDATE (failed)
expectExecuteDelegationFailed(mock)
a2aBody, _ := json.Marshal(map[string]interface{}{
"jsonrpc": "2.0", "id": "1", "method": "message/send",
"params": map[string]interface{}{
"message": map[string]interface{}{
"role": "user",
"parts": []map[string]string{{"type": "text", "text": "do work"}},
},
},
})
dh.executeDelegation(testSourceID, testTargetID, testDelegationID, a2aBody)
time.Sleep(100 * time.Millisecond)
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// TestExecuteDelegation_CleanProxyResponse_Unchanged verifies that a clean proxy response
// (no error, 200 with body) is unaffected by the new condition. This is the baseline:
// proxyErr == nil so the new condition never fires.
func TestExecuteDelegation_CleanProxyResponse_Unchanged(t *testing.T) {
mock := setupTestDB(t)
mr := setupTestRedis(t)
allowLoopbackForTest(t)
broadcaster := newTestBroadcaster()
wh := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
dh := NewDelegationHandler(wh, broadcaster)
agentServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"result":{"parts":[{"text":"all good"}]}}`))
}))
defer agentServer.Close()
mr.Set(fmt.Sprintf("ws:%s:url", testTargetID), agentServer.URL)
allowLoopbackForTest(t)
expectExecuteDelegationBase(mock)
expectExecuteDelegationSuccess(mock, `{"result":{"parts":[{"text":"all good"}]}}`)
a2aBody, _ := json.Marshal(map[string]interface{}{
"jsonrpc": "2.0", "id": "1", "method": "message/send",
"params": map[string]interface{}{
"message": map[string]interface{}{
"role": "user",
"parts": []map[string]string{{"type": "text", "text": "do work"}},
},
},
})
dh.executeDelegation(testSourceID, testTargetID, testDelegationID, a2aBody)
time.Sleep(100 * time.Millisecond)
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
}

View File

@ -26,14 +26,6 @@ func TestExtended_WorkspaceDelete(t *testing.T) {
WithArgs(wsDelID). WithArgs(wsDelID).
WillReturnRows(sqlmock.NewRows([]string{"id", "name"})) WillReturnRows(sqlmock.NewRows([]string{"id", "name"}))
// CascadeDelete walks descendants unconditionally (the 0-children
// optimization in the old inline path was dropped during the
// CascadeDelete extraction — descendant CTE returns 0 rows here,
// same end state, one extra cheap query).
mock.ExpectQuery("WITH RECURSIVE descendants").
WithArgs(wsDelID).
WillReturnRows(sqlmock.NewRows([]string{"id"}))
// #73: batch UPDATE happens BEFORE any container teardown. // #73: batch UPDATE happens BEFORE any container teardown.
// Uses ANY($1::uuid[]) even with a single ID for consistency. // Uses ANY($1::uuid[]) even with a single ID for consistency.
mock.ExpectExec("UPDATE workspaces SET status ="). mock.ExpectExec("UPDATE workspaces SET status =").

View File

@ -25,35 +25,6 @@ import (
"github.com/Molecule-AI/molecule-monorepo/platform/internal/registry" "github.com/Molecule-AI/molecule-monorepo/platform/internal/registry"
"github.com/google/uuid" "github.com/google/uuid"
) )
// insertMCPDelegationRow writes a delegation activity row so the canvas
// Agent Comms tab can show the task text for MCP-initiated delegations.
// Mirrors insertDelegationRow (delegation.go) for the MCP tool path.
func insertMCPDelegationRow(ctx context.Context, db *sql.DB, workspaceID, targetID, delegationID, task string) error {
taskJSON, _ := json.Marshal(map[string]interface{}{
"task": task,
"delegation_id": delegationID,
})
_, err := db.ExecContext(ctx, `
INSERT INTO activity_logs (workspace_id, activity_type, method, source_id, target_id, summary, request_body, status)
VALUES ($1, 'delegation', 'delegate', $2, $3, $4, $5::jsonb, 'pending')
`, workspaceID, workspaceID, targetID, "Delegating to "+targetID, string(taskJSON))
return err
}
// updateMCPDelegationStatus updates a delegation activity row's status.
// Mirrors updateDelegationStatus (delegation.go) for the MCP tool path.
func updateMCPDelegationStatus(ctx context.Context, db *sql.DB, workspaceID, delegationID, status, errorDetail string) {
if _, err := db.ExecContext(ctx, `
UPDATE activity_logs
SET status = $1, error_detail = CASE WHEN $2 = '' THEN error_detail ELSE $2 END
WHERE workspace_id = $3
AND method = 'delegate'
AND request_body->>'delegation_id' = $4
`, status, errorDetail, workspaceID, delegationID); err != nil {
log.Printf("MCP Delegation %s: status update failed: %v", delegationID, err)
}
}
// ───────────────────────────────────────────────────────────────────────────── // ─────────────────────────────────────────────────────────────────────────────
// Tool implementations // Tool implementations
// ───────────────────────────────────────────────────────────────────────────── // ─────────────────────────────────────────────────────────────────────────────
@ -183,13 +154,6 @@ func (h *MCPHandler) toolDelegateTask(ctx context.Context, callerID string, args
return "", fmt.Errorf("workspace %s is not authorised to communicate with %s", callerID, targetID) return "", fmt.Errorf("workspace %s is not authorised to communicate with %s", callerID, targetID)
} }
// Issue #158: write delegation row so canvas Agent Comms tab shows the task text.
delegationID := uuid.New().String()
if err := insertMCPDelegationRow(ctx, h.database, callerID, targetID, delegationID, task); err != nil {
log.Printf("MCP delegate_task: failed to record delegation row: %v", err)
// Non-fatal: still make the A2A call even if activity log write fails.
}
agentURL, err := mcpResolveURL(ctx, h.database, targetID) agentURL, err := mcpResolveURL(ctx, h.database, targetID)
if err != nil { if err != nil {
return "", err return "", err
@ -233,16 +197,10 @@ func (h *MCPHandler) toolDelegateTask(ctx context.Context, callerID string, args
resp, err := http.DefaultClient.Do(httpReq) resp, err := http.DefaultClient.Do(httpReq)
if err != nil { if err != nil {
updateMCPDelegationStatus(ctx, h.database, callerID, delegationID, "failed", err.Error())
return "", fmt.Errorf("A2A call failed: %w", err) return "", fmt.Errorf("A2A call failed: %w", err)
} }
defer func() { _ = resp.Body.Close() }() defer func() { _ = resp.Body.Close() }()
// A 200/500 from the peer still means the call was dispatched — only
// network errors are truly "failed". Status 'dispatched' is correct for
// any HTTP response (peer's A2A layer handles the actual processing).
updateMCPDelegationStatus(ctx, h.database, callerID, delegationID, "dispatched", "")
body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20))
if err != nil { if err != nil {
return "", fmt.Errorf("failed to read response: %w", err) return "", fmt.Errorf("failed to read response: %w", err)
@ -265,16 +223,7 @@ func (h *MCPHandler) toolDelegateTaskAsync(ctx context.Context, callerID string,
return "", fmt.Errorf("workspace %s is not authorised to communicate with %s", callerID, targetID) return "", fmt.Errorf("workspace %s is not authorised to communicate with %s", callerID, targetID)
} }
delegationID := uuid.New().String() taskID := uuid.New().String()
// Issue #158: write delegation row so canvas Agent Comms tab shows the task text.
// Insert with 'dispatched' status since the goroutine won't update it.
if err := insertMCPDelegationRow(ctx, h.database, callerID, targetID, delegationID, task); err != nil {
log.Printf("MCP delegate_task_async: failed to record delegation row: %v", err)
// Non-fatal: still fire the A2A call.
} else {
updateMCPDelegationStatus(ctx, h.database, callerID, delegationID, "dispatched", "")
}
// Fire and forget in a detached goroutine. Use a background context so // Fire and forget in a detached goroutine. Use a background context so
// the call is not cancelled when the HTTP request completes. // the call is not cancelled when the HTTP request completes.
@ -295,7 +244,7 @@ func (h *MCPHandler) toolDelegateTaskAsync(ctx context.Context, callerID string,
a2aBody, _ := json.Marshal(map[string]interface{}{ a2aBody, _ := json.Marshal(map[string]interface{}{
"jsonrpc": "2.0", "jsonrpc": "2.0",
"id": delegationID, "id": taskID,
"method": "message/send", "method": "message/send",
"params": map[string]interface{}{ "params": map[string]interface{}{
"message": map[string]interface{}{ "message": map[string]interface{}{
@ -324,7 +273,7 @@ func (h *MCPHandler) toolDelegateTaskAsync(ctx context.Context, callerID string,
_, _ = io.Copy(io.Discard, resp.Body) _, _ = io.Copy(io.Discard, resp.Body)
}() }()
return fmt.Sprintf(`{"task_id":%q,"status":"dispatched","target_id":%q}`, delegationID, targetID), nil return fmt.Sprintf(`{"task_id":%q,"status":"dispatched","target_id":%q}`, taskID, targetID), nil
} }
func (h *MCPHandler) toolCheckTaskStatus(ctx context.Context, callerID string, args map[string]interface{}) (string, error) { func (h *MCPHandler) toolCheckTaskStatus(ctx context.Context, callerID string, args map[string]interface{}) (string, error) {

View File

@ -13,15 +13,12 @@ import (
"path/filepath" "path/filepath"
"strconv" "strconv"
"strings" "strings"
"time"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/channels" "github.com/Molecule-AI/molecule-monorepo/platform/internal/channels"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/events" "github.com/Molecule-AI/molecule-monorepo/platform/internal/events"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/models" "github.com/Molecule-AI/molecule-monorepo/platform/internal/models"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/provisioner" "github.com/Molecule-AI/molecule-monorepo/platform/internal/provisioner"
"github.com/gin-gonic/gin" "github.com/gin-gonic/gin"
"github.com/lib/pq"
"gopkg.in/yaml.v3" "gopkg.in/yaml.v3"
) )
@ -425,16 +422,6 @@ type OrgWorkspace struct {
Tier int `yaml:"tier" json:"tier"` Tier int `yaml:"tier" json:"tier"`
Template string `yaml:"template" json:"template"` Template string `yaml:"template" json:"template"`
FilesDir string `yaml:"files_dir" json:"files_dir"` FilesDir string `yaml:"files_dir" json:"files_dir"`
// Spawning gates whether this workspace (AND its descendants) gets
// provisioned during /org/import. Pointer so we can distinguish
// "explicitly set to false" from "unset" (default = spawn). Use case:
// the dev-tree org template declares the full team structure but a
// developer's local machine only has RAM for a subset; setting
// spawning: false on a leaf or a sub-tree root skips that branch
// entirely without editing the canonical template structure.
// Counted in countWorkspaces same as actual; subtree-skip happens
// at provision time in createWorkspaceTree.
Spawning *bool `yaml:"spawning,omitempty" json:"spawning,omitempty"`
// SystemPrompt is an inline override. Normally each role's system-prompt.md // SystemPrompt is an inline override. Normally each role's system-prompt.md
// lives at `<files_dir>/system-prompt.md` and is copied via the files_dir // lives at `<files_dir>/system-prompt.md` and is copied via the files_dir
// template-copy step; inline overrides that path for ad-hoc workspaces. // template-copy step; inline overrides that path for ad-hoc workspaces.
@ -571,19 +558,6 @@ func (h *OrgHandler) Import(c *gin.Context) {
var body struct { var body struct {
Dir string `json:"dir"` // org template directory name Dir string `json:"dir"` // org template directory name
Template OrgTemplate `json:"template"` // or inline template Template OrgTemplate `json:"template"` // or inline template
// Mode controls cleanup behavior of pre-existing workspaces:
// "" / "merge" — additive (default; current behavior).
// Existing workspaces matched by
// (parent_id, name) are skipped; nothing
// outside the new tree is touched.
// "reconcile" — additive + cleanup. After import, any
// online workspace whose name matches an
// imported workspace's name but whose id
// isn't in the import result set is
// cascade-deleted. Catches "previous
// import survived a re-import" zombies
// (the 20:13→21:17 dev-tree case).
Mode string `json:"mode"`
} }
if err := c.ShouldBindJSON(&body); err != nil { if err := c.ShouldBindJSON(&body); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"}) c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"})
@ -607,16 +581,7 @@ func (h *OrgHandler) Import(c *gin.Context) {
orgFile := filepath.Join(orgBaseDir, "org.yaml") orgFile := filepath.Join(orgBaseDir, "org.yaml")
data, err := os.ReadFile(orgFile) data, err := os.ReadFile(orgFile)
if err != nil { if err != nil {
// Audit 2026-05-09 (Core-Security): the prior message echoed c.JSON(http.StatusNotFound, gin.H{"error": fmt.Sprintf("org template not found: %s", body.Dir)})
// the user-supplied `body.Dir` verbatim. Path traversal is
// already blocked by resolveInsideRoot above, but echoing
// the raw input back lets a client probe for the existence
// of relative paths inside h.orgDir (a 404 with the input
// vs. a 400 from resolveInsideRoot is itself a signal).
// Drop the input from the message; log full context server-
// side via the resolved path for operator triage.
log.Printf("OrgImport: failed to read %s (requested dir=%q): %v", orgFile, body.Dir, err)
c.JSON(http.StatusNotFound, gin.H{"error": "org template not found"})
return return
} }
// Expand !include directives before unmarshal. Splits org.yaml // Expand !include directives before unmarshal. Splits org.yaml
@ -638,19 +603,6 @@ func (h *OrgHandler) Import(c *gin.Context) {
return return
} }
// Emit started AFTER the YAML is loaded so payload.name carries the
// resolved template name (was: empty when caller passed `dir` instead
// of inline `template`). Pre-parse error paths above return without
// emitting — semantically "we couldn't even start an import" — so
// every started event is guaranteed a paired completed/failed below
// (no orphan started rows in structure_events).
importStart := time.Now()
emitOrgEvent(c.Request.Context(), "org.import.started", map[string]any{
"name": tmpl.Name,
"dir": body.Dir,
"mode": body.Mode,
})
// Required-env preflight — refuses import when any required_env is // Required-env preflight — refuses import when any required_env is
// missing from global_secrets. No bypass: the prior `force: true` // missing from global_secrets. No bypass: the prior `force: true`
// escape hatch was removed (issue #2290) because it was the silent // escape hatch was removed (issue #2290) because it was the silent
@ -756,171 +708,18 @@ func (h *OrgHandler) Import(c *gin.Context) {
} }
} }
// Reconcile mode: prune workspaces present from a previous import that
// share a name with the new tree but are NOT in the new result set.
// Catches the additive-import bug where re-running /org/import with a
// changed tree shape (different parent_id for the same role name) leaves
// the prior workspace online — visible to the canvas, consuming
// containers, and looking like a duplicate. Default mode "" / "merge"
// preserves the old additive behavior.
reconcileRemovedCount := 0
reconcileSkipped := 0
reconcileErrs := []string{}
if body.Mode == "reconcile" && createErr == nil {
ctx := c.Request.Context()
importedNames := []string{}
walkOrgWorkspaceNames(tmpl.Workspaces, &importedNames)
importedIDs := make([]string, 0, len(results))
for _, r := range results {
if id, ok := r["id"].(string); ok && id != "" {
importedIDs = append(importedIDs, id)
}
}
// Empty-set guards: if the import didn't produce any names or any
// IDs, skip — querying with empty arrays would either match
// nothing (harmless) or, worse, match every workspace if a future
// query rewrite drops the IN clause. Belt-and-suspenders.
if len(importedNames) > 0 && len(importedIDs) > 0 {
rows, err := db.DB.QueryContext(ctx, `
SELECT id FROM workspaces
WHERE name = ANY($1::text[])
AND id != ALL($2::uuid[])
AND status != 'removed'
`, pq.Array(importedNames), pq.Array(importedIDs))
if err != nil {
log.Printf("Org import reconcile: orphan query failed: %v", err)
reconcileErrs = append(reconcileErrs, fmt.Sprintf("orphan query: %v", err))
} else {
orphanIDs := []string{}
for rows.Next() {
var orphanID string
if rows.Scan(&orphanID) == nil {
orphanIDs = append(orphanIDs, orphanID)
}
}
rows.Close()
for _, oid := range orphanIDs {
descendantIDs, stopErrs, err := h.workspace.CascadeDelete(ctx, oid)
if err != nil {
log.Printf("Org import reconcile: CascadeDelete(%s) failed: %v", oid, err)
reconcileErrs = append(reconcileErrs, fmt.Sprintf("delete %s: %v", oid, err))
reconcileSkipped++
continue
}
reconcileRemovedCount += 1 + len(descendantIDs)
if len(stopErrs) > 0 {
log.Printf("Org import reconcile: %s had %d stop errors (orphan sweeper will retry)", oid, len(stopErrs))
}
}
log.Printf("Org import reconcile: %d orphans removed (%d cascade descendants), %d skipped", len(orphanIDs), reconcileRemovedCount-len(orphanIDs), reconcileSkipped)
}
}
}
status := http.StatusCreated status := http.StatusCreated
resp := gin.H{ resp := gin.H{
"org": tmpl.Name, "org": tmpl.Name,
"workspaces": results, "workspaces": results,
"count": len(results), "count": len(results),
} }
if body.Mode == "reconcile" {
resp["mode"] = "reconcile"
resp["reconcile_removed_count"] = reconcileRemovedCount
if len(reconcileErrs) > 0 {
resp["reconcile_errors"] = reconcileErrs
}
}
if createErr != nil { if createErr != nil {
status = http.StatusMultiStatus status = http.StatusMultiStatus
resp["error"] = createErr.Error() resp["error"] = createErr.Error()
} }
// results contains both freshly-created AND lookupExistingChild skips log.Printf("Org import: %s — %d workspaces created", tmpl.Name, len(results))
// (entries with "skipped":true). Splitting the count here so the audit
// row reflects "what changed" vs "what was already there" — telemetry
// readers shouldn't need to grep stdout to tell an idempotent re-run
// apart from a fresh-create.
createdCount, skippedCount := 0, 0
for _, r := range results {
if skipped, _ := r["skipped"].(bool); skipped {
skippedCount++
} else {
createdCount++
}
}
log.Printf("Org import: %s — %d created, %d skipped, %d reconciled",
tmpl.Name, createdCount, skippedCount, reconcileRemovedCount)
emitOrgEvent(c.Request.Context(), "org.import.completed", map[string]any{
"name": tmpl.Name,
"dir": body.Dir,
"mode": body.Mode,
"created_count": createdCount,
"skipped_count": skippedCount,
"reconcile_removed_count": reconcileRemovedCount,
"reconcile_errors": len(reconcileErrs),
"duration_ms": time.Since(importStart).Milliseconds(),
"create_error": errString(createErr),
})
c.JSON(status, resp) c.JSON(status, resp)
} }
// walkOrgWorkspaceNames collects every Name in the tree (in any order) into
// names. Used by reconcile to detect orphan workspaces — workspaces with the
// same role name as a freshly-imported one but a different id, surviving from
// a prior import.
func walkOrgWorkspaceNames(workspaces []OrgWorkspace, names *[]string) {
for _, w := range workspaces {
// spawning:false subtrees are still part of the imported tree
// from a logical-tree perspective — DON'T skip the recursion,
// or reconcile would orphan the rest of the subtree on every
// re-import where spawning is toggled. Names of skipped
// workspaces remain registered so reconcile won't double-create
// them when spawning flips back to true.
if w.Name != "" {
*names = append(*names, w.Name)
}
walkOrgWorkspaceNames(w.Children, names)
}
}
// emitOrgEvent records an org-lifecycle event in structure_events so the
// import history is queryable independent of stdout log retention. Errors
// are logged and swallowed — never block the request path on telemetry.
//
// Event-type taxonomy (extend by appending; never rename):
//
// org.import.started — handler entered, request body parsed
// org.import.completed — handler exiting (success or partial)
// org.import.failed — handler exiting with an unrecoverable error
//
// payload fields are documented at each call site.
func emitOrgEvent(ctx context.Context, eventType string, payload map[string]any) {
if payload == nil {
payload = map[string]any{}
}
payloadJSON, err := json.Marshal(payload)
if err != nil {
log.Printf("emitOrgEvent: marshal %s payload failed: %v", eventType, err)
return
}
if _, err := db.DB.ExecContext(ctx, `
INSERT INTO structure_events (event_type, payload, created_at)
VALUES ($1, $2, now())
`, eventType, payloadJSON); err != nil {
log.Printf("emitOrgEvent: insert %s failed: %v", eventType, err)
}
}
// errString returns "" for a nil error, err.Error() otherwise. Lets us put
// nullable error strings in event payloads without checking for nil at every
// call site.
func errString(err error) string {
if err == nil {
return ""
}
return err.Error()
}

View File

@ -42,20 +42,6 @@ import (
// straight into the parent's child-coordinate space without doing a // straight into the parent's child-coordinate space without doing a
// canvas-wide absolute-position walk. // canvas-wide absolute-position walk.
func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX, absY, relX, relY float64, defaults OrgDefaults, orgBaseDir string, results *[]map[string]interface{}, provisionSem chan struct{}) error { func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX, absY, relX, relY float64, defaults OrgDefaults, orgBaseDir string, results *[]map[string]interface{}, provisionSem chan struct{}) error {
// spawning: false guard — skip this workspace AND all descendants.
// Pointer-typed so we distinguish "explicitly false" from "unset"
// (unset = default to spawn). The guard sits BEFORE any side effect
// (no DB row, no docker provision, no children recursion) so a
// false-spawning subtree is genuinely a no-op except for the log line.
// Use case: dev-tree org template ships the full role taxonomy but a
// developer's machine only has RAM for a subset; a per-workspace
// `spawning: false` lets them narrow without editing the parent
// template's structure.
if ws.Spawning != nil && !*ws.Spawning {
log.Printf("Org import: skipping workspace %q (spawning=false; %d descendant workspace(s) in subtree also skipped)", ws.Name, countWorkspaces(ws.Children))
return nil
}
// Apply defaults // Apply defaults
runtime := ws.Runtime runtime := ws.Runtime
if runtime == "" { if runtime == "" {
@ -467,25 +453,8 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX
envVars := map[string]string{} envVars := map[string]string{}
// 0. Persona env (lowest precedence; injects the role's Gitea identity: // 0. Persona env (lowest precedence; injects the role's Gitea identity:
// GITEA_USER, GITEA_TOKEN, GITEA_TOKEN_SCOPES, GITEA_USER_EMAIL, // GITEA_USER, GITEA_TOKEN, GITEA_TOKEN_SCOPES, GITEA_USER_EMAIL,
// GITEA_SSH_KEY_PATH, plus MODEL_PROVIDER/MODEL and the LLM auth // GITEA_SSH_KEY_PATH). Workspace and org .env can override.
// token like CLAUDE_CODE_OAUTH_TOKEN or MINIMAX_API_KEY). loadPersonaEnvFile(ws.Role, envVars)
// Workspace and org .env can override.
//
// Use ws.FilesDir as the persona-dir lookup key, NOT ws.Role. In the
// dev-tree org.yaml shape, `role:` carries the multi-line descriptive
// text the agent reads from its prompt ("Engineering planning and
// team coordination — leads Core Platform, Controlplane, ..."), while
// `files_dir:` holds the short slug (`core-lead`, `dev-lead`, etc.)
// matching `~/.molecule-ai/personas/<files_dir>/env`
// (bind-mounted to `/etc/molecule-bootstrap/personas/<files_dir>/env`).
//
// Pre-fix, this passed `ws.Role` whose multi-word content failed
// isSafeRoleName silently, so every imported workspace booted with
// zero persona-env rows in workspace_secrets — no ANTHROPIC /
// CLAUDE_CODE auth in the container env. The claude_agent_sdk
// then wedged on `query.initialize()` with a 60s control-request
// timeout (caught 2026-05-08 right after dev-only org/import).
loadPersonaEnvFile(ws.FilesDir, envVars)
if orgBaseDir != "" { if orgBaseDir != "" {
// 1. Org root .env (shared defaults) // 1. Org root .env (shared defaults)
parseEnvFile(filepath.Join(orgBaseDir, ".env"), envVars) parseEnvFile(filepath.Join(orgBaseDir, ".env"), envVars)

View File

@ -1,158 +0,0 @@
package handlers
import (
"context"
"sort"
"testing"
"github.com/DATA-DOG/go-sqlmock"
)
// Tests for the reconcile-mode + audit-event additions to OrgHandler.Import.
//
// Background: /org/import was purely additive — re-running with a tree that
// renamed/reparented a role left the prior workspace online (different
// parent_id from the new one, so lookupExistingChild's parent-scoped dedupe
// missed it). The 2026-05-08 dev-tree case left 8 orphans surviving a
// re-import. mode="reconcile" closes the gap; emitOrgEvent makes "what
// happened at 20:13?" queryable instead of stdout-grep archaeology.
func TestWalkOrgWorkspaceNames_FlatTree(t *testing.T) {
tree := []OrgWorkspace{
{Name: "Dev Lead"},
{Name: "Release Manager"},
}
var names []string
walkOrgWorkspaceNames(tree, &names)
sort.Strings(names)
want := []string{"Dev Lead", "Release Manager"}
if !equalStrings(names, want) {
t.Errorf("flat tree: got %v, want %v", names, want)
}
}
func TestWalkOrgWorkspaceNames_NestedTree(t *testing.T) {
tree := []OrgWorkspace{
{
Name: "Dev Lead",
Children: []OrgWorkspace{
{Name: "Core Platform Lead", Children: []OrgWorkspace{{Name: "Core-BE"}}},
{Name: "SDK Lead"},
},
},
}
var names []string
walkOrgWorkspaceNames(tree, &names)
sort.Strings(names)
want := []string{"Core Platform Lead", "Core-BE", "Dev Lead", "SDK Lead"}
if !equalStrings(names, want) {
t.Errorf("nested tree: got %v, want %v", names, want)
}
}
// Pins the contract that spawning:false subtrees still contribute their names
// to the reconcile working set. If the walker started skipping them, a
// re-import that toggled spawning would orphan whichever workspaces had been
// previously imported with spawning:true — the inverse of the bug being
// fixed. Spawning gates *provisioning*, not *reconcile membership*.
func TestWalkOrgWorkspaceNames_SpawningFalseStillCounted(t *testing.T) {
f := false
tree := []OrgWorkspace{
{Name: "Dev Lead", Children: []OrgWorkspace{
{Name: "Skipped Lead", Spawning: &f, Children: []OrgWorkspace{
{Name: "Skipped Child"},
}},
}},
}
var names []string
walkOrgWorkspaceNames(tree, &names)
sort.Strings(names)
want := []string{"Dev Lead", "Skipped Child", "Skipped Lead"}
if !equalStrings(names, want) {
t.Errorf("spawning:false subtree: got %v, want %v", names, want)
}
}
func TestWalkOrgWorkspaceNames_EmptyNamesSkipped(t *testing.T) {
tree := []OrgWorkspace{
{Name: "Dev Lead"},
{Name: ""}, // YAML default / placeholder
{Name: "Release Manager"},
}
var names []string
walkOrgWorkspaceNames(tree, &names)
sort.Strings(names)
want := []string{"Dev Lead", "Release Manager"}
if !equalStrings(names, want) {
t.Errorf("empty-name skip: got %v, want %v", names, want)
}
}
// emitOrgEvent must INSERT into structure_events with event_type + JSON
// payload. Verifies the SQL shape pinning so a future schema rename
// (e.g., switching to audit_events) breaks the test loudly instead of
// silently dropping telemetry.
func TestEmitOrgEvent_InsertsToStructureEvents(t *testing.T) {
mock := setupTestDB(t)
mock.ExpectExec(`INSERT INTO structure_events`).
WithArgs("org.import.started", sqlmock.AnyArg()).
WillReturnResult(sqlmock.NewResult(1, 1))
emitOrgEvent(context.Background(), "org.import.started", map[string]any{
"name": "test-org",
"mode": "reconcile",
})
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("sqlmock expectations: %v", err)
}
}
// Insert failures are log-and-swallow — telemetry MUST NOT block the
// caller path. If this regresses (e.g., a future patch returns the err),
// org-import requests would fail with HTTP 500 every time a structure_events
// INSERT hiccups, which is strictly worse than losing the row.
func TestEmitOrgEvent_DBErrorIsSwallowed(t *testing.T) {
mock := setupTestDB(t)
mock.ExpectExec(`INSERT INTO structure_events`).
WithArgs("org.import.failed", sqlmock.AnyArg()).
WillReturnError(errSentinelTest)
// Must not panic; must not propagate. The function returns nothing,
// so the contract is "doesn't crash."
emitOrgEvent(context.Background(), "org.import.failed", map[string]any{
"err": "preflight failed",
})
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("sqlmock expectations: %v", err)
}
}
func TestErrString(t *testing.T) {
if got := errString(nil); got != "" {
t.Errorf("nil error: got %q, want empty", got)
}
if got := errString(errSentinelTest); got != "sentinel" {
t.Errorf("sentinel error: got %q, want \"sentinel\"", got)
}
}
// errSentinelTest is a marker error used for swallow-error assertions.
var errSentinelTest = sentinelErrTest{}
type sentinelErrTest struct{}
func (sentinelErrTest) Error() string { return "sentinel" }
func equalStrings(a, b []string) bool {
if len(a) != len(b) {
return false
}
for i := range a {
if a[i] != b[i] {
return false
}
}
return true
}

View File

@ -233,9 +233,6 @@ func (h *TemplatesHandler) ReplaceFiles(c *gin.Context) {
"files": len(body.Files), "files": len(body.Files),
"source": "ec2-ssh", "source": "ec2-ssh",
}) })
if h.wh != nil {
go h.wh.RestartByID(workspaceID)
}
return return
} }
@ -267,27 +264,27 @@ func (h *TemplatesHandler) ReplaceFiles(c *gin.Context) {
"files": len(body.Files), "files": len(body.Files),
"source": "container", "source": "container",
}) })
if h.wh != nil {
go h.wh.RestartByID(workspaceID)
}
return return
} }
// Container offline — write to the config volume via ephemeral container. // Container offline — try ephemeral container to write to volume
// Do NOT fall back to the host-side template dir: the restart handler
// reads from the volume, not the template dir, so a template-dir write
// would silently succeed (200) while the file change is invisible to
// restarted containers (#151). Surface the error so the caller retries
// when Docker is available instead of believing the change persisted.
volName := provisioner.ConfigVolumeName(workspaceID) volName := provisioner.ConfigVolumeName(workspaceID)
if err := h.writeViaEphemeral(ctx, volName, body.Files); err != nil { if err := h.writeViaEphemeral(ctx, volName, body.Files); err != nil {
log.Printf("ReplaceFiles: writeViaEphemeral failed for %s (workspace %s offline): %v", wsName, workspaceID, err) // Last resort: write to host-side template dir
c.JSON(http.StatusServiceUnavailable, gin.H{"error": "workspace offline — retry after it starts"}) destDir := h.resolveTemplateDir(wsName)
if destDir == "" {
log.Printf("ReplaceFiles: writeViaEphemeral failed and no template dir for %s: %v", workspaceID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to write files to workspace"})
return
}
os.MkdirAll(destDir, 0o755)
if err := writeFiles(destDir, body.Files); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"})
return
}
c.JSON(http.StatusOK, gin.H{"status": "replaced", "workspace": workspaceID, "files": len(body.Files), "source": "template"})
return return
} }
c.JSON(http.StatusOK, gin.H{"status": "replaced", "workspace": workspaceID, "files": len(body.Files), "source": "volume"}) 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)
}
} }

View File

@ -452,43 +452,3 @@ func TestReplaceFiles_PathTraversal(t *testing.T) {
t.Errorf("unmet sqlmock expectations: %v", err) t.Errorf("unmet sqlmock expectations: %v", err)
} }
} }
// TestReplaceFiles_OfflineDocker_Returns503 ensures that when the workspace
// container is offline AND the ephemeral Docker write fails (docker unavailable),
// ReplaceFiles returns 503 instead of silently falling back to the template
// directory. A template-dir write would be invisible after restart because
// the restart handler reads from the Docker volume, not the template dir (#151).
func TestReplaceFiles_OfflineDocker_Returns503(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
// nil docker → TemplatesHandler.docker == nil → findContainer returns "" (offline)
// → writeViaEphemeral returns error (docker not available) → handler should
// return 503, NOT fall back to the host-side template dir.
handler := NewTemplatesHandler(t.TempDir(), nil, nil)
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
WithArgs("ws-offline").
WillReturnRows(sqlmock.NewRows([]string{"name", "instance_id", "runtime"}).AddRow("Offline Agent", "", ""))
body := `{"files": {"initial-prompt.md": "updated prompt"}}`
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-offline"}}
c.Request = httptest.NewRequest("PUT", "/workspaces/ws-offline/files", bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
handler.ReplaceFiles(c)
if w.Code != http.StatusServiceUnavailable {
t.Errorf("expected 503, got %d: %s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "offline") {
t.Errorf("response should mention 'offline': %s", w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}

View File

@ -524,9 +524,6 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) {
return return
} }
c.JSON(http.StatusOK, gin.H{"status": "saved", "path": filePath}) c.JSON(http.StatusOK, gin.H{"status": "saved", "path": filePath})
if h.wh != nil {
go h.wh.RestartByID(workspaceID)
}
return return
} }
@ -538,9 +535,6 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) {
return return
} }
c.JSON(http.StatusOK, gin.H{"status": "saved", "path": filePath}) c.JSON(http.StatusOK, gin.H{"status": "saved", "path": filePath})
if h.wh != nil {
go h.wh.RestartByID(workspaceID)
}
return return
} }
@ -552,9 +546,6 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) {
return return
} }
c.JSON(http.StatusOK, gin.H{"status": "saved", "path": filePath}) c.JSON(http.StatusOK, gin.H{"status": "saved", "path": filePath})
if h.wh != nil {
go h.wh.RestartByID(workspaceID)
}
} }
// DeleteFile handles DELETE /workspaces/:id/files/*path // DeleteFile handles DELETE /workspaces/:id/files/*path
@ -601,9 +592,6 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) {
return return
} }
c.JSON(http.StatusOK, gin.H{"status": "deleted", "path": filePath}) c.JSON(http.StatusOK, gin.H{"status": "deleted", "path": filePath})
if h.wh != nil {
go h.wh.RestartByID(workspaceID)
}
return return
} }
@ -619,9 +607,6 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) {
return return
} }
c.JSON(http.StatusOK, gin.H{"status": "deleted", "path": filePath}) c.JSON(http.StatusOK, gin.H{"status": "deleted", "path": filePath})
if h.wh != nil {
go h.wh.RestartByID(workspaceID)
}
return return
} }
@ -632,8 +617,5 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) {
return return
} }
c.JSON(http.StatusOK, gin.H{"status": "deleted", "path": filePath}) c.JSON(http.StatusOK, gin.H{"status": "deleted", "path": filePath})
if h.wh != nil {
go h.wh.RestartByID(workspaceID)
}
} }

View File

@ -134,7 +134,7 @@ func (h *TranscriptHandler) Get(c *gin.Context) {
// - block cloud metadata endpoints (IMDS, GCP, Azure) // - block cloud metadata endpoints (IMDS, GCP, Azure)
// - block link-local IPs (169.254/16 IPv4, fe80::/10 IPv6) // - block link-local IPs (169.254/16 IPv4, fe80::/10 IPv6)
// - loopback is allowed — local dev runs workspaces on 127.0.0.1 // - loopback is allowed — local dev runs workspaces on 127.0.0.1
// - Docker internal hostnames (host.docker.internal, *.molecule-core-net) // - Docker internal hostnames (host.docker.internal, *.molecule-monorepo-net)
// are allowed; the whole threat model assumes the platform already // are allowed; the whole threat model assumes the platform already
// trusts peers on that network // trusts peers on that network
func validateWorkspaceURL(u *url.URL) error { func validateWorkspaceURL(u *url.URL) error {

View File

@ -323,25 +323,161 @@ func (h *WorkspaceHandler) Delete(c *gin.Context) {
return return
} }
// Delegate the cascade to CascadeDelete so the HTTP path and the // Cascade delete: collect ALL descendants (not just direct children) via
// OrgImport reconcile path share one teardown sequence (#73 race // recursive CTE, then stop each container and remove each volume.
// guard, container stop, volume removal, token revocation, schedule // Previous bug: only direct children's containers were stopped, leaving
// disable, broadcast). The HTTP-specific bits — direct-children 409 // grandchildren as orphan running containers after a cascade delete.
// gate above, ?purge=true hard-delete below, response shaping — descendantIDs := []string{}
// stay in this handler. if len(children) > 0 {
descendantIDs, stopErrs, err := h.CascadeDelete(ctx, id) descRows, err := db.DB.QueryContext(ctx, `
if err != nil { WITH RECURSIVE descendants AS (
// Audit 2026-05-09 (Core-Security): raw `err.Error()` here was SELECT id FROM workspaces WHERE parent_id = $1 AND status != 'removed'
// exposed to HTTP clients verbatim, including wrapped lib/pq UNION ALL
// driver strings that disclose schema column names + index SELECT w.id FROM workspaces w JOIN descendants d ON w.parent_id = d.id WHERE w.status != 'removed'
// hints. Log full error server-side; return a sanitized message )
// to the client. Operators trace via the log line below using SELECT id FROM descendants
// the workspace id. `, id)
log.Printf("Delete: CascadeDelete(%s) failed: %v", id, err) if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "internal error processing delete request"}) log.Printf("Delete: descendant query error for %s: %v", id, err)
return } else {
for descRows.Next() {
var descID string
if descRows.Scan(&descID) == nil {
descendantIDs = append(descendantIDs, descID)
}
}
descRows.Close()
}
} }
// #73 fix: mark rows 'removed' in the DB FIRST, BEFORE stopping containers
// or removing volumes. Previously the sequence was stop → update-status,
// which left a gap where:
// - the container's last pre-teardown heartbeat could resurrect the row
// via the register-handler UPSERT (now also guarded in #73)
// - the liveness monitor could observe 'online' status + expired Redis
// TTL and trigger RestartByID, recreating a container we're trying
// to destroy
// Marking 'removed' first makes both of those paths no-op via their
// existing `status NOT IN ('removed', ...)` guards.
allIDs := append([]string{id}, descendantIDs...) allIDs := append([]string{id}, descendantIDs...)
if _, err := db.DB.ExecContext(ctx,
`UPDATE workspaces SET status = $1, updated_at = now() WHERE id = ANY($2::uuid[])`,
models.StatusRemoved, pq.Array(allIDs)); err != nil {
log.Printf("Delete status update error for %s: %v", id, err)
}
if _, err := db.DB.ExecContext(ctx,
`DELETE FROM canvas_layouts WHERE workspace_id = ANY($1::uuid[])`,
pq.Array(allIDs)); err != nil {
log.Printf("Delete canvas_layouts error for %s: %v", id, err)
}
// Revoke all auth tokens for the deleted workspaces. Once the workspace is
// gone its tokens are meaningless; leaving them alive would keep
// HasAnyLiveTokenGlobal = true even after the platform is otherwise empty,
// which prevents AdminAuth from returning to fail-open and breaks the E2E
// test's count-zero assertion (and local re-run cleanup).
if _, err := db.DB.ExecContext(ctx,
`UPDATE workspace_auth_tokens SET revoked_at = now()
WHERE workspace_id = ANY($1::uuid[]) AND revoked_at IS NULL`,
pq.Array(allIDs)); err != nil {
log.Printf("Delete token revocation error for %s: %v", id, err)
}
// #1027: cascade-disable all schedules for the deleted workspaces so
// the scheduler never fires a cron into a removed container.
if _, err := db.DB.ExecContext(ctx,
`UPDATE workspace_schedules SET enabled = false, updated_at = now()
WHERE workspace_id = ANY($1::uuid[]) AND enabled = true`,
pq.Array(allIDs)); err != nil {
log.Printf("Delete schedule disable error for %s: %v", id, err)
}
// Now stop containers + remove volumes for all descendants (any depth).
// Any concurrent heartbeat / registration / liveness-triggered restart
// will see status='removed' and bail out early.
//
// Combines two concerns:
//
// 1. Detach cleanup from the request ctx via WithoutCancel + a 30s
// timeout, so when the canvas's `api.del` resolves on our 200
// (and gin cancels c.Request.Context()), in-flight Docker
// stop/remove calls don't get cancelled mid-operation. The
// previous shape leaked containers every time the canvas hung
// up promptly: Stop returned "context canceled", the container
// stayed up, and the next RemoveVolume failed with
// "volume in use". 30s is generous for Docker daemon round-
// trips (typical: <2s) and bounds a stuck daemon.
//
// 2. #1843: aggregate Stop() failures into stopErrs so the
// post-deletion block surfaces them as 500. On the CP/EC2
// backend, Stop() calls control plane's DELETE endpoint to
// terminate the EC2; if that errors (transient 5xx, network),
// the EC2 stays running with no DB row to track it (the
// "orphan EC2 on a 0-customer account" scenario). Loud-fail
// instead of silent-leak — clients retry, Stop's instance_id
// lookup is idempotent against status='removed'. RemoveVolume
// errors stay log-and-continue (local cleanup, not infra-leak).
cleanupCtx, cleanupCancel := context.WithTimeout(
context.WithoutCancel(ctx), 30*time.Second)
defer cleanupCancel()
var stopErrs []error
stopAndRemove := func(wsID string) {
// Stop the workload first via the backend dispatcher (CP for
// SaaS, Docker for self-hosted). Pre-2026-05-05 this gate was
// `if h.provisioner == nil { return }` — early-returning on
// every SaaS tenant left the EC2 running with no DB row to
// track it (issue #2814; the comment below claimed "loud-fail
// instead of silent-leak" but the early-return made it the
// silent path on SaaS).
//
// Check Stop's error before any volume cleanup — the previous
// code discarded it and immediately tried RemoveVolume, which
// always fails with "volume in use" when Stop didn't actually
// kill the container. The orphan sweeper
// (registry/orphan_sweeper.go) catches what we skip here on
// the next reconcile pass.
if err := h.StopWorkspaceAuto(cleanupCtx, wsID); err != nil {
log.Printf("Delete %s stop failed: %v — leaving cleanup for orphan sweeper", wsID, err)
stopErrs = append(stopErrs, fmt.Errorf("stop %s: %w", wsID, err))
return
}
// Volume cleanup is Docker-only — CP-managed workspaces have
// no host-bind volumes to remove. Skip silently when no Docker
// provisioner is wired (the SaaS path already terminated the
// EC2 above; nothing left to do).
if h.provisioner != nil {
if err := h.provisioner.RemoveVolume(cleanupCtx, wsID); err != nil {
log.Printf("Delete %s volume removal warning: %v", wsID, err)
}
}
}
for _, descID := range descendantIDs {
stopAndRemove(descID)
db.ClearWorkspaceKeys(cleanupCtx, descID)
// #2269: drop the per-workspace restartState entry so it
// doesn't accumulate across the platform's lifetime. The
// LoadOrStore that creates the entry (workspace_restart.go)
// has no companion remove path; without this Delete, every
// short-lived workspace leaks ~16 bytes forever.
restartStates.Delete(descID)
// Detach broadcaster ctx for the same reason as the cleanup
// above — RecordAndBroadcast does an INSERT INTO
// structure_events + Redis Publish. If the canvas hangs up,
// a request-ctx-bound INSERT can be cancelled mid-write,
// leaving other WS clients ignorant of the cascade. The DB
// row is already 'removed' so it's recoverable, but the
// inconsistency is avoidable.
h.broadcaster.RecordAndBroadcast(cleanupCtx, string(events.EventWorkspaceRemoved), descID, map[string]interface{}{})
}
stopAndRemove(id)
db.ClearWorkspaceKeys(cleanupCtx, id)
restartStates.Delete(id) // #2269: same as descendants above
h.broadcaster.RecordAndBroadcast(cleanupCtx, string(events.EventWorkspaceRemoved), id, map[string]interface{}{
"cascade_deleted": len(descendantIDs),
})
// If any Stop call failed, surface 500 so the client retries. The DB // If any Stop call failed, surface 500 so the client retries. The DB
// row is already 'removed' (idempotent), and Stop's instance_id // row is already 'removed' (idempotent), and Stop's instance_id
@ -407,104 +543,6 @@ func (h *WorkspaceHandler) Delete(c *gin.Context) {
c.JSON(http.StatusOK, gin.H{"status": "removed", "cascade_deleted": len(descendantIDs)}) c.JSON(http.StatusOK, gin.H{"status": "removed", "cascade_deleted": len(descendantIDs)})
} }
// CascadeDelete performs the cascade-removal sequence used by the HTTP
// DELETE handler and by OrgImport's reconcile mode: walk descendants, mark
// self+descendants 'removed' first (#73 race guard), stop containers / EC2s,
// remove volumes, revoke tokens, disable schedules, broadcast events.
//
// Idempotent against already-removed rows (the descendant CTE and all UPDATE
// guards skip status='removed'). Returns the descendant id list so the HTTP
// caller can drive the optional `?purge=true` hard-delete path against the
// same set the cascade just touched, plus any per-workspace stop errors so
// callers can surface a retryable failure instead of a silent-leak.
//
// Caller is responsible for the children-confirmation gate (the HTTP handler
// returns 409 when children exist + ?confirm=true is missing); this helper
// always cascades.
func (h *WorkspaceHandler) CascadeDelete(ctx context.Context, id string) ([]string, []error, error) {
if err := validateWorkspaceID(id); err != nil {
return nil, nil, err
}
descendantIDs := []string{}
descRows, err := db.DB.QueryContext(ctx, `
WITH RECURSIVE descendants AS (
SELECT id FROM workspaces WHERE parent_id = $1 AND status != 'removed'
UNION ALL
SELECT w.id FROM workspaces w JOIN descendants d ON w.parent_id = d.id WHERE w.status != 'removed'
)
SELECT id FROM descendants
`, id)
if err != nil {
return nil, nil, fmt.Errorf("descendant query: %w", err)
}
for descRows.Next() {
var descID string
if descRows.Scan(&descID) == nil {
descendantIDs = append(descendantIDs, descID)
}
}
descRows.Close()
allIDs := append([]string{id}, descendantIDs...)
if _, err := db.DB.ExecContext(ctx,
`UPDATE workspaces SET status = $1, updated_at = now() WHERE id = ANY($2::uuid[])`,
models.StatusRemoved, pq.Array(allIDs)); err != nil {
log.Printf("CascadeDelete status update for %s: %v", id, err)
}
if _, err := db.DB.ExecContext(ctx,
`DELETE FROM canvas_layouts WHERE workspace_id = ANY($1::uuid[])`,
pq.Array(allIDs)); err != nil {
log.Printf("CascadeDelete canvas_layouts for %s: %v", id, err)
}
if _, err := db.DB.ExecContext(ctx,
`UPDATE workspace_auth_tokens SET revoked_at = now()
WHERE workspace_id = ANY($1::uuid[]) AND revoked_at IS NULL`,
pq.Array(allIDs)); err != nil {
log.Printf("CascadeDelete token revocation for %s: %v", id, err)
}
if _, err := db.DB.ExecContext(ctx,
`UPDATE workspace_schedules SET enabled = false, updated_at = now()
WHERE workspace_id = ANY($1::uuid[]) AND enabled = true`,
pq.Array(allIDs)); err != nil {
log.Printf("CascadeDelete schedule disable for %s: %v", id, err)
}
cleanupCtx, cleanupCancel := context.WithTimeout(
context.WithoutCancel(ctx), 30*time.Second)
defer cleanupCancel()
var stopErrs []error
stopAndRemove := func(wsID string) {
if err := h.StopWorkspaceAuto(cleanupCtx, wsID); err != nil {
log.Printf("CascadeDelete %s stop failed: %v — leaving cleanup for orphan sweeper", wsID, err)
stopErrs = append(stopErrs, fmt.Errorf("stop %s: %w", wsID, err))
return
}
if h.provisioner != nil {
if err := h.provisioner.RemoveVolume(cleanupCtx, wsID); err != nil {
log.Printf("CascadeDelete %s volume removal warning: %v", wsID, err)
}
}
}
for _, descID := range descendantIDs {
stopAndRemove(descID)
db.ClearWorkspaceKeys(cleanupCtx, descID)
restartStates.Delete(descID)
h.broadcaster.RecordAndBroadcast(cleanupCtx, string(events.EventWorkspaceRemoved), descID, map[string]interface{}{})
}
stopAndRemove(id)
db.ClearWorkspaceKeys(cleanupCtx, id)
restartStates.Delete(id)
h.broadcaster.RecordAndBroadcast(cleanupCtx, string(events.EventWorkspaceRemoved), id, map[string]interface{}{
"cascade_deleted": len(descendantIDs),
})
return descendantIDs, stopErrs, nil
}
// validateWorkspaceID returns an error when id is not a valid UUID. // validateWorkspaceID returns an error when id is not a valid UUID.
// #687: prevents 500s from Postgres when a garbage string (e.g. ../../etc/passwd) // #687: prevents 500s from Postgres when a garbage string (e.g. ../../etc/passwd)
// is passed as the :id path parameter. // is passed as the :id path parameter.

View File

@ -173,7 +173,7 @@ func (h *WorkspaceHandler) provisionWorkspaceOpts(workspaceID, templatePath stri
log.Printf("Provisioner: failed to cache URL for %s: %v", workspaceID, cacheErr) log.Printf("Provisioner: failed to cache URL for %s: %v", workspaceID, cacheErr)
} }
// Also cache the Docker-internal URL for workspace-to-workspace discovery. // Also cache the Docker-internal URL for workspace-to-workspace discovery.
// Containers on molecule-core-net can reach each other by container name. // Containers on molecule-monorepo-net can reach each other by container name.
internalURL := provisioner.InternalURL(workspaceID) internalURL := provisioner.InternalURL(workspaceID)
if cacheErr := db.CacheInternalURL(ctx, workspaceID, internalURL); cacheErr != nil { if cacheErr := db.CacheInternalURL(ctx, workspaceID, internalURL); cacheErr != nil {
log.Printf("Provisioner: failed to cache internal URL for %s: %v", workspaceID, cacheErr) log.Printf("Provisioner: failed to cache internal URL for %s: %v", workspaceID, cacheErr)
@ -715,30 +715,14 @@ func deriveProviderFromModelSlug(model string) string {
// payload.Model at boot), this is a no-op — no harm in the switch // payload.Model at boot), this is a no-op — no harm in the switch
// being empty for those cases. // being empty for those cases.
func applyRuntimeModelEnv(envVars map[string]string, runtime, model string) { func applyRuntimeModelEnv(envVars map[string]string, runtime, model string) {
// Resolution order (priority high → low): // Fall back to the MODEL_PROVIDER workspace secret when the caller
// 1. payload.Model (caller passed the canvas-picked model id verbatim) // didn't pass one explicitly. This is the path that "Save+Restart"
// 2. envVars["MODEL"] (workspace_secret persisted by /org/import via // hits — Restart builds its payload from the workspaces row (no model
// the persona env file — MODEL=MiniMax-M2.7-highspeed etc.) // column there) so payload.Model is always empty, but the user's
// 3. envVars["MODEL_PROVIDER"] (legacy: this secret was historically a // canvas selection was stored as MODEL_PROVIDER via PUT /model and
// *model id* set by canvas Save+Restart's PUT /model; on the // is already loaded into envVars here. Without this fallback hermes
// post-2026-05-08 persona-env convention it's a *provider slug* // silently boots with the template default and errors "No LLM
// (e.g. "minimax") which is NOT a valid model id, so this fallback // provider configured" even though the user picked a valid model.
// only fires when MODEL is absent.)
//
// 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.
if model == "" {
model = envVars["MODEL"]
}
if model == "" { if model == "" {
model = envVars["MODEL_PROVIDER"] model = envVars["MODEL_PROVIDER"]
} }

View File

@ -724,68 +724,3 @@ 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.
//
// 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
// 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
// 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.
func TestApplyRuntimeModelEnv_PersonaEnvMODELSecretPreserved(t *testing.T) {
cases := []struct {
name string
envMODEL string
envMP string
wantMODEL string
}{
{
name: "MODEL secret wins over MODEL_PROVIDER slug (persona-env shape on restart)",
envMODEL: "MiniMax-M2.7-highspeed",
envMP: "minimax",
wantMODEL: "MiniMax-M2.7-highspeed",
},
{
name: "MODEL secret wins even when same as MODEL_PROVIDER",
envMODEL: "opus",
envMP: "claude-code",
wantMODEL: "opus",
},
{
name: "MODEL absent → fall back to MODEL_PROVIDER (legacy canvas Save+Restart shape)",
envMODEL: "",
envMP: "MiniMax-M2.7",
wantMODEL: "MiniMax-M2.7",
},
{
name: "Both absent → no MODEL set",
envMODEL: "",
envMP: "",
wantMODEL: "",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
envVars := map[string]string{}
if tc.envMODEL != "" {
envVars["MODEL"] = tc.envMODEL
}
if tc.envMP != "" {
envVars["MODEL_PROVIDER"] = tc.envMP
}
// payload.Model is empty (the restart case)
applyRuntimeModelEnv(envVars, "claude-code", "")
if got := envVars["MODEL"]; got != tc.wantMODEL {
t.Errorf("MODEL = %q, want %q (envMODEL=%q envMP=%q)",
got, tc.wantMODEL, tc.envMODEL, tc.envMP)
}
})
}
}

View File

@ -813,12 +813,6 @@ func TestWorkspaceDelete_DisablesSchedules(t *testing.T) {
WithArgs(wsID). WithArgs(wsID).
WillReturnRows(sqlmock.NewRows([]string{"id", "name"})) WillReturnRows(sqlmock.NewRows([]string{"id", "name"}))
// CascadeDelete walks descendants unconditionally — 0-children case
// returns 0 rows here.
mock.ExpectQuery("WITH RECURSIVE descendants").
WithArgs(wsID).
WillReturnRows(sqlmock.NewRows([]string{"id"}))
// Mark workspace as removed // Mark workspace as removed
mock.ExpectExec("UPDATE workspaces SET status ="). mock.ExpectExec("UPDATE workspaces SET status =").
WillReturnResult(sqlmock.NewResult(0, 1)) WillReturnResult(sqlmock.NewResult(0, 1))
@ -941,12 +935,6 @@ func TestWorkspaceDelete_ScheduleDisableOnlyTargetsDeletedWorkspace(t *testing.T
WithArgs(wsA). WithArgs(wsA).
WillReturnRows(sqlmock.NewRows([]string{"id", "name"})) WillReturnRows(sqlmock.NewRows([]string{"id", "name"}))
// CascadeDelete walks descendants unconditionally — 0-children case
// returns 0 rows here.
mock.ExpectQuery("WITH RECURSIVE descendants").
WithArgs(wsA).
WillReturnRows(sqlmock.NewRows([]string{"id"}))
// Mark only workspace A as removed // Mark only workspace A as removed
mock.ExpectExec("UPDATE workspaces SET status ="). mock.ExpectExec("UPDATE workspaces SET status =").
WillReturnResult(sqlmock.NewResult(0, 1)) WillReturnResult(sqlmock.NewResult(0, 1))

View File

@ -6,23 +6,12 @@ import (
) )
// StartSweeperWithIntervalForTest exposes startSweeperWithInterval to // StartSweeperWithIntervalForTest exposes startSweeperWithInterval to
// the external test package. The production code uses StartSeper // the external test package. The production code uses StartSweeper
// (which pins the canonical SweepInterval); tests pin a short interval // (which pins the canonical SweepInterval); tests pin a short interval
// to exercise the ticker-driven cycle without burning real wall-clock // to exercise the ticker-driven cycle without burning real wall-clock
// time. The Go convention `export_test.go` keeps this seam OUT of the // time. The Go convention `export_test.go` keeps this seam OUT of the
// production binary — files ending in _test.go are stripped at build // production binary — files ending in _test.go are stripped at build
// time, so this re-export only exists during `go test`. // time, so this re-export only exists during `go test`.
func StartSweeperWithIntervalForTest(ctx context.Context, storage Storage, ackRetention, interval time.Duration) { func StartSweeperWithIntervalForTest(ctx context.Context, storage Storage, ackRetention, interval time.Duration) {
startSweeperWithInterval(ctx, storage, ackRetention, interval, nil) startSweeperWithInterval(ctx, storage, ackRetention, interval)
}
// StartSweeperForTest starts the sweeper and returns a done channel
// that is closed exactly once when the loop exits. Tests MUST receive
// from done before returning so the goroutine has fully terminated and
// the shared metric counters are stable for the next test's baseline
// capture (issue #86).
func StartSweeperForTest(ctx context.Context, storage Storage, ackRetention time.Duration) chan struct{} {
done := make(chan struct{})
go startSweeperWithInterval(ctx, storage, ackRetention, SweepInterval, done)
return done
} }

View File

@ -66,21 +66,15 @@ const sweepDeadline = 30 * time.Second
// to exercise the ticker-driven sweep path without burning real wall- // to exercise the ticker-driven sweep path without burning real wall-
// clock time. // clock time.
func StartSweeper(ctx context.Context, storage Storage, ackRetention time.Duration) { func StartSweeper(ctx context.Context, storage Storage, ackRetention time.Duration) {
startSweeperWithInterval(ctx, storage, ackRetention, SweepInterval, nil) startSweeperWithInterval(ctx, storage, ackRetention, SweepInterval)
} }
// startSweeperWithInterval is the test-friendly variant of StartSweeper // startSweeperWithInterval is the test-friendly variant of StartSweeper
// — same loop, but the cadence is caller-specified. Production code // — same loop, but the cadence is caller-specified. Production code
// should use StartSweeper to keep the SweepInterval constant pinned. // should use StartSweeper to keep the SweepInterval constant pinned.
// If done is non-nil it is closed exactly once when the loop exits, func startSweeperWithInterval(ctx context.Context, storage Storage, ackRetention, interval time.Duration) {
// allowing tests to wait for the goroutine to fully terminate before
// asserting on shared metric counters (issue #86).
func startSweeperWithInterval(ctx context.Context, storage Storage, ackRetention, interval time.Duration, done chan struct{}) {
if storage == nil { if storage == nil {
log.Println("pendinguploads sweeper: storage is nil — sweeper disabled") log.Println("pendinguploads sweeper: storage is nil — sweeper disabled")
if done != nil {
close(done)
}
return return
} }
if ackRetention == 0 { if ackRetention == 0 {
@ -92,12 +86,6 @@ func startSweeperWithInterval(ctx context.Context, storage Storage, ackRetention
) )
ticker := time.NewTicker(interval) ticker := time.NewTicker(interval)
defer ticker.Stop() defer ticker.Stop()
defer func() {
log.Println("pendinguploads sweeper: shutdown")
if done != nil {
close(done)
}
}()
// Run once immediately so a platform restart cleans up any rows // Run once immediately so a platform restart cleans up any rows
// that became eligible while we were down — don't make the // that became eligible while we were down — don't make the
// operator wait 5 minutes for the first sweep. // operator wait 5 minutes for the first sweep.
@ -105,16 +93,9 @@ func startSweeperWithInterval(ctx context.Context, storage Storage, ackRetention
for { for {
select { select {
case <-ctx.Done(): case <-ctx.Done():
log.Println("pendinguploads sweeper: shutdown")
return return
case <-ticker.C: case <-ticker.C:
// Guard: ctx may have been cancelled between the ticker firing
// and this case being selected (MPMC channel; close-to-ready race).
// Calling sweepOnce with an already-cancelled ctx would increment
// the error counter on shutdown — polluting the next test's
// baseline (issue #86 full-suite failure).
if ctx.Err() != nil {
continue
}
sweepOnce(ctx, storage, ackRetention) sweepOnce(ctx, storage, ackRetention)
} }
} }

View File

@ -136,7 +136,7 @@ func TestStartSweeper_RunsImmediatelyAndOnTick(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
defer cancel() defer cancel()
done := pendinguploads.StartSweeperForTest(ctx, store, time.Hour) go pendinguploads.StartSweeper(ctx, store, time.Hour)
store.waitForCycle(t, 1, 2*time.Second) store.waitForCycle(t, 1, 2*time.Second)
if got := store.calls.Load(); got < 1 { if got := store.calls.Load(); got < 1 {
t.Errorf("expected at least one immediate sweep, got %d", got) t.Errorf("expected at least one immediate sweep, got %d", got)
@ -145,10 +145,6 @@ func TestStartSweeper_RunsImmediatelyAndOnTick(t *testing.T) {
if store.gotRetention.Load() != 3600 { if store.gotRetention.Load() != 3600 {
t.Errorf("retention seconds = %d, want 3600", store.gotRetention.Load()) t.Errorf("retention seconds = %d, want 3600", store.gotRetention.Load())
} }
// #86 fix: ensure goroutine has exited before the next test's
// metricDelta() baseline capture.
cancel()
<-done
} }
func TestStartSweeper_ZeroAckRetentionUsesDefault(t *testing.T) { func TestStartSweeper_ZeroAckRetentionUsesDefault(t *testing.T) {
@ -156,22 +152,23 @@ func TestStartSweeper_ZeroAckRetentionUsesDefault(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
defer cancel() defer cancel()
done := pendinguploads.StartSweeperForTest(ctx, store, 0) go pendinguploads.StartSweeper(ctx, store, 0)
store.waitForCycle(t, 1, 2*time.Second) store.waitForCycle(t, 1, 2*time.Second)
want := int64(pendinguploads.DefaultAckRetention.Seconds()) want := int64(pendinguploads.DefaultAckRetention.Seconds())
if store.gotRetention.Load() != want { if store.gotRetention.Load() != want {
t.Errorf("retention = %d, want default %d", store.gotRetention.Load(), want) t.Errorf("retention = %d, want default %d", store.gotRetention.Load(), want)
} }
// #86 fix.
cancel()
<-done
} }
func TestStartSweeper_ContextCancelStopsLoop(t *testing.T) { func TestStartSweeper_ContextCancelStopsLoop(t *testing.T) {
store := newFakeSweepStorage([]pendinguploads.SweepResult{{}}, nil) store := newFakeSweepStorage([]pendinguploads.SweepResult{{}}, nil)
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
done := pendinguploads.StartSweeperForTest(ctx, store, time.Second) done := make(chan struct{})
go func() {
pendinguploads.StartSweeper(ctx, store, time.Second)
close(done)
}()
store.waitForCycle(t, 1, 2*time.Second) store.waitForCycle(t, 1, 2*time.Second)
cancel() cancel()
@ -190,17 +187,14 @@ func TestStartSweeperWithInterval_TickerFiresAdditionalCycles(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
defer cancel() defer cancel()
done := pendinguploads.StartSweeperForTest(ctx, store, time.Hour) go pendinguploads.StartSweeperWithIntervalForTest(ctx, store, time.Hour, 30*time.Millisecond)
// Immediate cycle + at least one tick-driven cycle. // Immediate cycle + at least one tick-driven cycle.
store.waitForCycle(t, 2, 2*time.Second) store.waitForCycle(t, 2, 2*time.Second)
if got := store.calls.Load(); got < 2 { if got := store.calls.Load(); got < 2 {
t.Errorf("expected ≥2 cycles (immediate + 1 tick), got %d", got) t.Errorf("expected ≥2 cycles (immediate + 1 tick), got %d", got)
} }
// #86 fix: drain the done channel so the goroutine is fully gone
// before the next test's metricDelta() baseline capture.
cancel()
<-done
} }
func TestStartSweeper_TransientErrorDoesNotCrashLoop(t *testing.T) { func TestStartSweeper_TransientErrorDoesNotCrashLoop(t *testing.T) {
@ -223,7 +217,7 @@ func TestStartSweeper_TransientErrorDoesNotCrashLoop(t *testing.T) {
// waitForCycle is too early. // waitForCycle is too early.
_, _, deltaError := metricDelta(t) _, _, deltaError := metricDelta(t)
done := pendinguploads.StartSweeperForTest(ctx, store, time.Hour) go pendinguploads.StartSweeper(ctx, store, time.Hour)
// Wait for the first (errored) cycle. // Wait for the first (errored) cycle.
store.waitForCycle(t, 1, 2*time.Second) store.waitForCycle(t, 1, 2*time.Second)
@ -232,13 +226,11 @@ func TestStartSweeper_TransientErrorDoesNotCrashLoop(t *testing.T) {
// stops the loop on the next select pass with no in-flight metric // stops the loop on the next select pass with no in-flight metric
// writes outstanding. // writes outstanding.
waitForMetricDelta(t, deltaError, 1, 2*time.Second) waitForMetricDelta(t, deltaError, 1, 2*time.Second)
// Cancel and wait for the goroutine to fully exit (#86 fix). // Cancel — the goroutine returns cleanly, proving the error path
// Without the done-channel wait, the goroutine races with the next // didn't crash the loop. Without this fix the goroutine would have
// test's metricDelta() baseline capture — the next test may see // either panicked (process abort visible at exit) or stuck (this
// error=1 from this test still "in flight", throwing off its // cancel + done-channel pattern would deadlock instead).
// deltaError assertion.
cancel() cancel()
<-done
} }
// metricDelta returns a function that, when called, returns how much // metricDelta returns a function that, when called, returns how much
@ -273,7 +265,7 @@ func TestStartSweeper_RecordsMetricsOnSuccess(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
defer cancel() defer cancel()
done := pendinguploads.StartSweeperForTest(ctx, store, time.Hour) go pendinguploads.StartSweeper(ctx, store, time.Hour)
store.waitForCycle(t, 1, 2*time.Second) store.waitForCycle(t, 1, 2*time.Second)
// Poll for the success counters to settle — closes the cycleDone- // Poll for the success counters to settle — closes the cycleDone-
@ -286,15 +278,6 @@ func TestStartSweeper_RecordsMetricsOnSuccess(t *testing.T) {
if got := deltaError(); got != 0 { if got := deltaError(); got != 0 {
t.Errorf("error counter delta = %d, want 0", got) t.Errorf("error counter delta = %d, want 0", got)
} }
// #86 fix: drain the done channel so the goroutine is fully gone
// before the next test's metricDelta() baseline capture. Without this
// the previous test's goroutine could still be mid-Sweep (blocked on
// the fake's results channel) and its eventual return would mutate
// the shared error/acked counters after the next test has already
// snapshot its baseline.
cancel()
<-done
} }
func TestStartSweeper_RecordsMetricsOnError(t *testing.T) { func TestStartSweeper_RecordsMetricsOnError(t *testing.T) {
@ -307,7 +290,7 @@ func TestStartSweeper_RecordsMetricsOnError(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
defer cancel() defer cancel()
done := pendinguploads.StartSweeperForTest(ctx, store, time.Hour) go pendinguploads.StartSweeper(ctx, store, time.Hour)
store.waitForCycle(t, 1, 2*time.Second) store.waitForCycle(t, 1, 2*time.Second)
// Poll for the error counter to settle — cycleDone fires inside // Poll for the error counter to settle — cycleDone fires inside
@ -317,9 +300,4 @@ func TestStartSweeper_RecordsMetricsOnError(t *testing.T) {
// though the metric WILL be 1 a few ms later. See // though the metric WILL be 1 a few ms later. See
// waitForMetricDelta comment. // waitForMetricDelta comment.
waitForMetricDelta(t, deltaError, 1, 2*time.Second) waitForMetricDelta(t, deltaError, 1, 2*time.Second)
// #86 fix: ensure the goroutine has fully exited before the next
// test's metricDelta() baseline capture.
cancel()
<-done
} }

View File

@ -133,13 +133,7 @@ func TestLocalResolver_HonoursContextCancellation(t *testing.T) {
func TestLocalResolver_BubblesUpCopyFailure(t *testing.T) { func TestLocalResolver_BubblesUpCopyFailure(t *testing.T) {
// Source file the copyTree walk would read; make dst unwritable so // Source file the copyTree walk would read; make dst unwritable so
// the copyFile step fails. Skip when running as root — Linux // the copyFile step fails.
// filesystem permissions are advisory-only for uid 0, so chmod 0o555
// does not prevent writes and the test passes vacuously instead of
// exercising the error path (issue #87).
if os.Getuid() == 0 {
t.Skip("skipping: chmod 0o555 is ineffective when running as root")
}
base := t.TempDir() base := t.TempDir()
writePlugin(t, base, "demo", map[string]string{ writePlugin(t, base, "demo", map[string]string{
"plugin.yaml": "name: demo\n", "plugin.yaml": "name: demo\n",

View File

@ -67,7 +67,7 @@ var DefaultImage = RuntimeImage(defaultRuntime)
const ( const (
// DefaultNetwork is the Docker network workspaces join. // DefaultNetwork is the Docker network workspaces join.
DefaultNetwork = "molecule-core-net" DefaultNetwork = "molecule-monorepo-net"
// DefaultPort is the port the A2A server listens on inside the container. // DefaultPort is the port the A2A server listens on inside the container.
DefaultPort = "8000" DefaultPort = "8000"
@ -405,7 +405,7 @@ func (p *Provisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, e
// Apply tier-based container configuration // Apply tier-based container configuration
ApplyTierConfig(hostCfg, cfg, configMount, name) ApplyTierConfig(hostCfg, cfg, configMount, name)
// Network config — join molecule-core-net with container name as alias // Network config — join molecule-monorepo-net with container name as alias
networkCfg := &network.NetworkingConfig{ networkCfg := &network.NetworkingConfig{
EndpointsConfig: map[string]*network.EndpointSettings{ EndpointsConfig: map[string]*network.EndpointSettings{
DefaultNetwork: { DefaultNetwork: {

View File

@ -1,2 +0,0 @@
DROP INDEX IF EXISTS workspaces_update_tier_canary;
ALTER TABLE workspaces DROP COLUMN IF EXISTS update_tier;

View File

@ -1,26 +0,0 @@
-- workspaces.update_tier — canary vs production filter for plugin updates
-- (core#115). Composes with the version-subscription DB foundation
-- (core#113, merged) and the upcoming drift+queue+apply endpoint
-- (core#123).
--
-- Tiers:
-- 'production' (default) — fan-out target; only updated AFTER canary soak
-- 'canary' — early-adopter target; updates land here first
--
-- Default 'production' so existing customers (Reno-Stars + any future
-- live tenant) are default-safe. Synthetic dogfooding workspaces opt
-- INTO 'canary' explicitly.
--
-- The column is just metadata at this layer; the actual filter logic
-- ('apply this update only to canary tier first') lives in the future
-- POST /admin/plugin-updates/:id/apply endpoint (core#123).
ALTER TABLE workspaces
ADD COLUMN IF NOT EXISTS update_tier TEXT NOT NULL DEFAULT 'production'
CHECK (update_tier IN ('canary', 'production'));
-- Partial index for the apply endpoint's canary-tier scan: only
-- index canary rows since the apply path queries them most often
-- and the production set is the much larger default.
CREATE INDEX IF NOT EXISTS workspaces_update_tier_canary
ON workspaces(update_tier) WHERE update_tier = 'canary';

View File

@ -43,29 +43,11 @@ if [ "$(id -u)" = "0" ]; then
ln -sfn /root/.claude/sessions /home/agent/.claude/sessions ln -sfn /root/.claude/sessions /home/agent/.claude/sessions
fi fi
# --- Per-persona git identity (closes molecule-core#155) ---
# Without this, every team commit lands with an empty author and Gitea
# attributes the work to the founder PAT instead of the persona that
# actually authored it. Same fingerprint that got us suspended on GitHub
# 2026-05-06. GITEA_USER is injected by the provisioner from the
# workspace_secrets table; bot.moleculesai.app is the agent-only domain
# so commits are clearly distinguishable from human authors.
if [ -n "${GITEA_USER:-}" ]; then
git config --global user.name "${GITEA_USER}"
git config --global user.email "${GITEA_USER}@bot.moleculesai.app"
fi
# --- GitHub credential helper setup (issue #547 / #613) --- # --- GitHub credential helper setup (issue #547 / #613) ---
# Configure git to use the molecule credential helper for github.com. # Configure git to use the molecule credential helper for github.com.
# This runs as root so the global gitconfig is written before we drop # This runs as root so the global gitconfig is written before we drop
# to agent. The helper fetches fresh GitHub App installation tokens # to agent. The helper fetches fresh GitHub App installation tokens
# from the platform API, with caching and env-var fallback. # from the platform API, with caching and env-var fallback.
#
# NOTE: post-suspension (2026-05-06), github.com/Molecule-AI is gone;
# the helper's platform endpoint also 500s (internal#187). The helper
# block is kept for legacy boxes that still have a working token chain;
# post-suspension provisioner injects GITEA_TOKEN directly so this
# path's failure is non-fatal. Full removal tracked under #171.
if [ -x /app/scripts/molecule-git-token-helper.sh ]; then if [ -x /app/scripts/molecule-git-token-helper.sh ]; then
# Set credential helper for github.com only (not all hosts). # Set credential helper for github.com only (not all hosts).
# The '!' prefix tells git to run the command as a shell command. # The '!' prefix tells git to run the command as a shell command.
@ -73,13 +55,11 @@ if [ "$(id -u)" = "0" ]; then
"!/app/scripts/molecule-git-token-helper.sh" "!/app/scripts/molecule-git-token-helper.sh"
# Disable other credential helpers for github.com to avoid conflicts. # Disable other credential helpers for github.com to avoid conflicts.
git config --global "credential.https://github.com.useHttpPath" true git config --global "credential.https://github.com.useHttpPath" true
fi # Move gitconfig to agent's home so it takes effect after gosu.
# Move gitconfig to agent's home so it takes effect after gosu — if [ -f /root/.gitconfig ]; then
# done unconditionally so the per-persona identity survives the drop cp /root/.gitconfig /home/agent/.gitconfig
# even when the github.com helper block is skipped. chown agent:agent /home/agent/.gitconfig
if [ -f /root/.gitconfig ]; then fi
cp /root/.gitconfig /home/agent/.gitconfig
chown agent:agent /home/agent/.gitconfig
fi fi
# Create the token cache directory for the agent user. # Create the token cache directory for the agent user.
mkdir -p /home/agent/.molecule-token-cache mkdir -p /home/agent/.molecule-token-cache

View File

@ -434,7 +434,7 @@ async def main(): # pragma: no cover
async def _transcript_handler(request): async def _transcript_handler(request):
# Require workspace bearer token — the same token issued at registration # Require workspace bearer token — the same token issued at registration
# and stored in /configs/.auth_token. Any container on molecule-core-net # and stored in /configs/.auth_token. Any container on molecule-monorepo-net
# could otherwise read the full session log. Closes #287. # could otherwise read the full session log. Closes #287.
# #
# #328: fail CLOSED when the token file is unavailable. get_token() # #328: fail CLOSED when the token file is unavailable. get_token()

View File

@ -401,35 +401,6 @@ if "a2a" not in sys.modules:
# tests now live in the claude-code template repo, where the real SDK # tests now live in the claude-code template repo, where the real SDK
# IS installed via Dockerfile, so no stub is needed. # IS installed via Dockerfile, so no stub is needed.
# ==================== Test isolation fixtures ====================
import pytest
@pytest.fixture(scope="function", autouse=True)
def _clear_platform_auth_cache():
"""Reset platform_auth._cached_token before each test.
Fixes issue #160: tests that use monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN")
to simulate "no token in env" fail when platform_auth._cached_token was already
set from a prior test's MOLECULE_WORKSPACE_TOKEN value. The cache is populated
at module import or first get_token() call and persists for the process lifetime
monkeypatch.delenv removes the env var but not the module-level cache.
Run at function scope so each test starts with a clean slate regardless of
what the previous test set. The import is inside the fixture (not at file
top-level) because conftest.py runs during test collection before
platform_auth might be available in all test environments. If the module is
absent (import error), the fixture is a no-op.
"""
try:
import platform_auth as _pa
_pa.clear_cache()
except ImportError:
pass
yield # run the test, then fixture teardown has nothing to do
if "langchain_core" not in sys.modules: if "langchain_core" not in sys.modules:
_make_langchain_mocks() _make_langchain_mocks()

View File

@ -184,14 +184,9 @@ class TestPlatformAuthRegistry:
assert b["Authorization"] == "Bearer tok-b" assert b["Authorization"] == "Bearer tok-b"
assert a["Origin"] == "https://example.test" assert a["Origin"] == "https://example.test"
def test_auth_headers_with_no_arg_uses_legacy_path(self, monkeypatch, tmp_path): def test_auth_headers_with_no_arg_uses_legacy_path(self, monkeypatch):
import platform_auth import platform_auth
# Wipe the module-level token cache and redirect _token_file() to a
# non-existent path so the env var isolation is clean. Without this,
# the real /configs/.auth_token pollutes the result.
platform_auth.clear_cache()
monkeypatch.setattr(platform_auth, "_token_file", lambda: tmp_path / ".auth_token")
monkeypatch.setenv("PLATFORM_URL", "https://example.test") monkeypatch.setenv("PLATFORM_URL", "https://example.test")
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN", "legacy-tok") monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN", "legacy-tok")
# Multi-workspace registry populated, but auth_headers() with # Multi-workspace registry populated, but auth_headers() with
@ -204,15 +199,10 @@ class TestPlatformAuthRegistry:
assert h["Authorization"] == "Bearer legacy-tok" assert h["Authorization"] == "Bearer legacy-tok"
def test_auth_headers_with_unknown_workspace_falls_back_to_legacy( def test_auth_headers_with_unknown_workspace_falls_back_to_legacy(
self, monkeypatch, tmp_path self, monkeypatch
): ):
import platform_auth import platform_auth
# Wipe the module-level token cache and redirect _token_file() to a
# non-existent path so the env var isolation is clean. Without this,
# the real /configs/.auth_token pollutes the result.
platform_auth.clear_cache()
monkeypatch.setattr(platform_auth, "_token_file", lambda: tmp_path / ".auth_token")
monkeypatch.setenv("PLATFORM_URL", "https://example.test") monkeypatch.setenv("PLATFORM_URL", "https://example.test")
monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN", "legacy-tok") monkeypatch.setenv("MOLECULE_WORKSPACE_TOKEN", "legacy-tok")
platform_auth.register_workspace_token("ws-a", "tok-a") platform_auth.register_workspace_token("ws-a", "tok-a")

View File

@ -166,15 +166,9 @@ def test_resolve_token_returns_value_and_label_for_env(monkeypatch):
assert mcp_doctor._resolve_token_summary() == label assert mcp_doctor._resolve_token_summary() == label
def test_resolve_token_returns_none_when_missing(monkeypatch, tmp_path): def test_resolve_token_returns_none_when_missing(monkeypatch):
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN", raising=False) monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN", raising=False)
monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN_FILE", raising=False) monkeypatch.delenv("MOLECULE_WORKSPACE_TOKEN_FILE", raising=False)
# The .auth_token file at /configs/.auth_token (present in container env)
# must not pollute the test. Patch configs_dir.resolve() to return a
# bare temp dir so the disk-file fallback in _resolve_token() has
# nothing to find.
import configs_dir
monkeypatch.setattr(configs_dir, "resolve", lambda: tmp_path)
val, label = mcp_doctor._resolve_token() val, label = mcp_doctor._resolve_token()
assert val is None assert val is None
assert label is None assert label is None

View File

@ -3,7 +3,7 @@ the workspace auth token is not yet on disk.
Prior behaviour (regressed in #287): `if expected:` skipped the auth Prior behaviour (regressed in #287): `if expected:` skipped the auth
check when `get_token()` returned None, so any container on check when `get_token()` returned None, so any container on
`molecule-core-net` could read the full session log during the `molecule-monorepo-net` could read the full session log during the
bootstrap window. The fix lifts the guard into transcript_auth.py for bootstrap window. The fix lifts the guard into transcript_auth.py for
testability. testability.
""" """