+``` + +--- + +## 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. diff --git a/docs/development/constraints-and-rules.md b/docs/development/constraints-and-rules.md index 4c2ffc71..0d980871 100644 --- a/docs/development/constraints-and-rules.md +++ b/docs/development/constraints-and-rules.md @@ -73,7 +73,7 @@ These are applied after CORS middleware on every response. ## 14. No Exposed Database Ports -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. +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. ## Related Docs diff --git a/docs/runbooks/handlers-postgres-integration-port-collision.md b/docs/runbooks/handlers-postgres-integration-port-collision.md index 0b9df483..931f6427 100644 --- a/docs/runbooks/handlers-postgres-integration-port-collision.md +++ b/docs/runbooks/handlers-postgres-integration-port-collision.md @@ -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 other workflow in the repo (cache server discovery, -`molecule-monorepo-net` peer access during integration tests, etc.) — +`molecule-core-net` peer access during integration tests, etc.) — unacceptable blast radius for a per-test bug. ## Fix shape `handlers-postgres-integration.yml` no longer uses `services: postgres:`. It launches a sibling postgres container manually on the existing -`molecule-monorepo-net` bridge network with a per-run unique name: +`molecule-core-net` bridge network with a per-run unique name: ```yaml env: PG_NAME: pg-handlers-${{ github.run_id }}-${{ github.run_attempt }} - PG_NETWORK: molecule-monorepo-net + PG_NETWORK: molecule-core-net steps: - 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. 2. Use `${{ github.run_id }}-${{ github.run_attempt }}` for unique container name. -3. Launch on `molecule-monorepo-net` (already trusted bridge in +3. Launch on `molecule-core-net` (already trusted bridge in `docker-compose.infra.yml`). 4. Read back the bridge IP via `docker inspect` and export as a step env. 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 this collision; the IPv6 fix is correct, port collision is the new layer. -- Issue #94 created `molecule-monorepo-net` + `alpine:latest` as +- Issue #94 created `molecule-core-net` + `alpine:latest` as prereqs. - Saved memory `feedback_act_runner_github_server_url` documents another act_runner-vs-GHA divergence (server URL). diff --git a/infra/scripts/setup.sh b/infra/scripts/setup.sh index 814799e1..f6ff490d 100755 --- a/infra/scripts/setup.sh +++ b/infra/scripts/setup.sh @@ -5,7 +5,7 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" ROOT_DIR="$(cd "$SCRIPT_DIR/../.." && pwd)" echo "==> Ensuring shared docker network exists..." -docker network create molecule-monorepo-net 2>/dev/null || true +docker network create molecule-core-net 2>/dev/null || true # Populate the template / plugin registry. # workspace-configs-templates/, org-templates/, and plugins/ are intentionally diff --git a/scripts/build_runtime_package.py b/scripts/build_runtime_package.py index 55639213..8d7b5045 100755 --- a/scripts/build_runtime_package.py +++ b/scripts/build_runtime_package.py @@ -268,10 +268,11 @@ molecule-mcp = "molecule_runtime.mcp_cli:main" [tool.setuptools.packages.find] where = ["."] -include = ["molecule_runtime*"] +include = ["molecule_runtime*", "plugins_registry*"] [tool.setuptools.package-data] "molecule_runtime" = ["py.typed"] +"plugins_registry" = ["py.typed"] """ @@ -473,6 +474,18 @@ def main() -> int: py_files = copy_tree_filtered(src, pkg_dir) 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 # (it's not a package in monorepo), but the published artifact must. init = pkg_dir / "__init__.py" diff --git a/scripts/nuke-and-rebuild.sh b/scripts/nuke-and-rebuild.sh index a3e75fc4..a9ef59a3 100644 --- a/scripts/nuke-and-rebuild.sh +++ b/scripts/nuke-and-rebuild.sh @@ -24,7 +24,7 @@ echo "=== NUKE ===" 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 volume ls --format "{{.Name}}" | grep "^ws-" | xargs -r docker volume rm 2>/dev/null || true -docker network rm molecule-monorepo-net 2>/dev/null || true +docker network rm molecule-core-net 2>/dev/null || true echo " cleaned" echo "=== POPULATE MANIFEST DIRS ===" diff --git a/workspace-server/internal/handlers/mcp_tools.go b/workspace-server/internal/handlers/mcp_tools.go index dfb93e48..24e991bb 100644 --- a/workspace-server/internal/handlers/mcp_tools.go +++ b/workspace-server/internal/handlers/mcp_tools.go @@ -25,6 +25,35 @@ import ( "github.com/Molecule-AI/molecule-monorepo/platform/internal/registry" "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 // ───────────────────────────────────────────────────────────────────────────── @@ -154,6 +183,13 @@ func (h *MCPHandler) toolDelegateTask(ctx context.Context, callerID string, args 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) if err != nil { return "", err @@ -197,10 +233,16 @@ func (h *MCPHandler) toolDelegateTask(ctx context.Context, callerID string, args resp, err := http.DefaultClient.Do(httpReq) if err != nil { + updateMCPDelegationStatus(ctx, h.database, callerID, delegationID, "failed", err.Error()) return "", fmt.Errorf("A2A call failed: %w", err) } 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)) if err != nil { return "", fmt.Errorf("failed to read response: %w", err) @@ -223,7 +265,16 @@ func (h *MCPHandler) toolDelegateTaskAsync(ctx context.Context, callerID string, return "", fmt.Errorf("workspace %s is not authorised to communicate with %s", callerID, targetID) } - taskID := uuid.New().String() + delegationID := 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 // the call is not cancelled when the HTTP request completes. @@ -244,7 +295,7 @@ func (h *MCPHandler) toolDelegateTaskAsync(ctx context.Context, callerID string, a2aBody, _ := json.Marshal(map[string]interface{}{ "jsonrpc": "2.0", - "id": taskID, + "id": delegationID, "method": "message/send", "params": map[string]interface{}{ "message": map[string]interface{}{ @@ -273,7 +324,7 @@ func (h *MCPHandler) toolDelegateTaskAsync(ctx context.Context, callerID string, _, _ = io.Copy(io.Discard, resp.Body) }() - return fmt.Sprintf(`{"task_id":%q,"status":"dispatched","target_id":%q}`, taskID, targetID), nil + return fmt.Sprintf(`{"task_id":%q,"status":"dispatched","target_id":%q}`, delegationID, targetID), nil } func (h *MCPHandler) toolCheckTaskStatus(ctx context.Context, callerID string, args map[string]interface{}) (string, error) { diff --git a/workspace-server/internal/handlers/org.go b/workspace-server/internal/handlers/org.go index 233cc69f..8b5c4585 100644 --- a/workspace-server/internal/handlers/org.go +++ b/workspace-server/internal/handlers/org.go @@ -607,7 +607,16 @@ func (h *OrgHandler) Import(c *gin.Context) { orgFile := filepath.Join(orgBaseDir, "org.yaml") data, err := os.ReadFile(orgFile) if err != nil { - c.JSON(http.StatusNotFound, gin.H{"error": fmt.Sprintf("org template not found: %s", body.Dir)}) + // Audit 2026-05-09 (Core-Security): the prior message echoed + // 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 } // Expand !include directives before unmarshal. Splits org.yaml diff --git a/workspace-server/internal/handlers/transcript.go b/workspace-server/internal/handlers/transcript.go index 4690f8d6..bdfe828f 100644 --- a/workspace-server/internal/handlers/transcript.go +++ b/workspace-server/internal/handlers/transcript.go @@ -134,7 +134,7 @@ func (h *TranscriptHandler) Get(c *gin.Context) { // - block cloud metadata endpoints (IMDS, GCP, Azure) // - block link-local IPs (169.254/16 IPv4, fe80::/10 IPv6) // - loopback is allowed — local dev runs workspaces on 127.0.0.1 -// - Docker internal hostnames (host.docker.internal, *.molecule-monorepo-net) +// - Docker internal hostnames (host.docker.internal, *.molecule-core-net) // are allowed; the whole threat model assumes the platform already // trusts peers on that network func validateWorkspaceURL(u *url.URL) error { diff --git a/workspace-server/internal/handlers/workspace_crud.go b/workspace-server/internal/handlers/workspace_crud.go index cc487a4a..c2674d32 100644 --- a/workspace-server/internal/handlers/workspace_crud.go +++ b/workspace-server/internal/handlers/workspace_crud.go @@ -331,8 +331,14 @@ func (h *WorkspaceHandler) Delete(c *gin.Context) { // stay in this handler. descendantIDs, stopErrs, err := h.CascadeDelete(ctx, id) if err != nil { + // Audit 2026-05-09 (Core-Security): raw `err.Error()` here was + // exposed to HTTP clients verbatim, including wrapped lib/pq + // driver strings that disclose schema column names + index + // hints. Log full error server-side; return a sanitized message + // to the client. Operators trace via the log line below using + // the workspace id. log.Printf("Delete: CascadeDelete(%s) failed: %v", id, err) - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.JSON(http.StatusInternalServerError, gin.H{"error": "internal error processing delete request"}) return } allIDs := append([]string{id}, descendantIDs...) diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index f3657d0b..57d6c5a6 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -173,7 +173,7 @@ func (h *WorkspaceHandler) provisionWorkspaceOpts(workspaceID, templatePath stri log.Printf("Provisioner: failed to cache URL for %s: %v", workspaceID, cacheErr) } // Also cache the Docker-internal URL for workspace-to-workspace discovery. - // Containers on molecule-monorepo-net can reach each other by container name. + // Containers on molecule-core-net can reach each other by container name. internalURL := provisioner.InternalURL(workspaceID) if cacheErr := db.CacheInternalURL(ctx, workspaceID, internalURL); cacheErr != nil { log.Printf("Provisioner: failed to cache internal URL for %s: %v", workspaceID, cacheErr) diff --git a/workspace-server/internal/provisioner/provisioner.go b/workspace-server/internal/provisioner/provisioner.go index c46c59db..11e730af 100644 --- a/workspace-server/internal/provisioner/provisioner.go +++ b/workspace-server/internal/provisioner/provisioner.go @@ -67,7 +67,7 @@ var DefaultImage = RuntimeImage(defaultRuntime) const ( // DefaultNetwork is the Docker network workspaces join. - DefaultNetwork = "molecule-monorepo-net" + DefaultNetwork = "molecule-core-net" // DefaultPort is the port the A2A server listens on inside the container. DefaultPort = "8000" @@ -405,7 +405,7 @@ func (p *Provisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, e // Apply tier-based container configuration ApplyTierConfig(hostCfg, cfg, configMount, name) - // Network config — join molecule-monorepo-net with container name as alias + // Network config — join molecule-core-net with container name as alias networkCfg := &network.NetworkingConfig{ EndpointsConfig: map[string]*network.EndpointSettings{ DefaultNetwork: { diff --git a/workspace/entrypoint.sh b/workspace/entrypoint.sh index 8b83ddc1..fb207904 100644 --- a/workspace/entrypoint.sh +++ b/workspace/entrypoint.sh @@ -43,11 +43,29 @@ if [ "$(id -u)" = "0" ]; then ln -sfn /root/.claude/sessions /home/agent/.claude/sessions 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) --- # Configure git to use the molecule credential helper for github.com. # This runs as root so the global gitconfig is written before we drop # to agent. The helper fetches fresh GitHub App installation tokens # 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 # Set credential helper for github.com only (not all hosts). # The '!' prefix tells git to run the command as a shell command. @@ -55,11 +73,13 @@ if [ "$(id -u)" = "0" ]; then "!/app/scripts/molecule-git-token-helper.sh" # Disable other credential helpers for github.com to avoid conflicts. git config --global "credential.https://github.com.useHttpPath" true - # Move gitconfig to agent's home so it takes effect after gosu. - if [ -f /root/.gitconfig ]; then - cp /root/.gitconfig /home/agent/.gitconfig - chown agent:agent /home/agent/.gitconfig - fi + fi + # Move gitconfig to agent's home so it takes effect after gosu — + # done unconditionally so the per-persona identity survives the drop + # even when the github.com helper block is skipped. + if [ -f /root/.gitconfig ]; then + cp /root/.gitconfig /home/agent/.gitconfig + chown agent:agent /home/agent/.gitconfig fi # Create the token cache directory for the agent user. mkdir -p /home/agent/.molecule-token-cache diff --git a/workspace/main.py b/workspace/main.py index 5ae5ebef..77c2d2d6 100644 --- a/workspace/main.py +++ b/workspace/main.py @@ -434,7 +434,7 @@ async def main(): # pragma: no cover async def _transcript_handler(request): # Require workspace bearer token — the same token issued at registration - # and stored in /configs/.auth_token. Any container on molecule-monorepo-net + # and stored in /configs/.auth_token. Any container on molecule-core-net # could otherwise read the full session log. Closes #287. # # #328: fail CLOSED when the token file is unavailable. get_token() diff --git a/workspace/tests/test_transcript_auth.py b/workspace/tests/test_transcript_auth.py index e28f4d21..e3556e2a 100644 --- a/workspace/tests/test_transcript_auth.py +++ b/workspace/tests/test_transcript_auth.py @@ -3,7 +3,7 @@ the workspace auth token is not yet on disk. Prior behaviour (regressed in #287): `if expected:` skipped the auth check when `get_token()` returned None, so any container on -`molecule-monorepo-net` could read the full session log during the +`molecule-core-net` could read the full session log during the bootstrap window. The fix lifts the guard into transcript_auth.py for testability. """