fix(ws-server): fail-closed on unresolvable template runtime (controlplane#188) #1489

Closed
hongming wants to merge 1 commits from fix/ws-server-188-failclosed-template-runtime into main
Owner

Summary

Closes the ws-server side of the molecule-controlplane#188 / #184 silent contract violation. POST /workspaces silently substituted langgraph and returned 201 when a caller named a template (intent for a specific runtime) but the runtime could not be resolved from it (config.yaml unreadable / no runtime: key). This produced 5/5 wrong-runtime workspaces and a false codex E2E pass.

The ws-server Create handler is the boundary the product UI + the provision_workspace MCP tool (molecule-mcp-server#19) actually hit; controlplane#188's CP-side gate is the sibling. When the caller expressed runtime intent (runtime, or a template) but it cannot be honored → 422 RUNTIME_UNRESOLVED instead of a silent langgraph 201.

Not a behavior change for the default path

Bare {"name":...} (no template, no runtime) still defaults to langgraph + 201. A regression test (TestWorkspaceCreate_188_NoTemplateNoRuntime_StillDefaultsLanggraph) pins this so the fail-closed gate cannot over-fire.

Tests

  • TestWorkspaceCreate_188_TemplateMissingRuntime_FailsClosed — template dir w/o config.yaml → 422
  • TestWorkspaceCreate_188_TemplateConfigNoRuntimeKey_FailsClosed — config.yaml w/o runtime: → 422
  • TestWorkspaceCreate_188_NoTemplateNoRuntime_StillDefaultsLanggraph — regression guard, still 201 langgraph
  • TestWorkspaceCreate_188_ExplicitRuntimeNoTemplate_OK — explicit runtime honored, 201
  • existing TestWorkspaceCreate* all still green

Test plan

  • CI green (ws-server go test)
  • Non-author review (core-devops / platform)
  • Confirms it does not block the legitimate canvas "New Workspace" default flow

Refs: molecule-controlplane#188, #184. Agent-facing sibling: molecule-mcp-server#19 (provision_workspace fail-closed MCP tool).

🤖 Generated with Claude Code

## Summary Closes the ws-server side of the molecule-controlplane#188 / #184 silent contract violation. `POST /workspaces` silently substituted langgraph and returned 201 when a caller named a `template` (intent for a specific runtime) but the runtime could not be resolved from it (config.yaml unreadable / no `runtime:` key). This produced 5/5 wrong-runtime workspaces and a false codex E2E pass. The ws-server `Create` handler is the boundary the product UI + the `provision_workspace` MCP tool (molecule-mcp-server#19) actually hit; controlplane#188's CP-side gate is the sibling. When the caller expressed runtime intent (`runtime`, or a `template`) but it cannot be honored → **422 RUNTIME_UNRESOLVED** instead of a silent langgraph 201. ## Not a behavior change for the default path Bare `{"name":...}` (no template, no runtime) still defaults to langgraph + 201. A regression test (`TestWorkspaceCreate_188_NoTemplateNoRuntime_StillDefaultsLanggraph`) pins this so the fail-closed gate cannot over-fire. ## Tests - `TestWorkspaceCreate_188_TemplateMissingRuntime_FailsClosed` — template dir w/o config.yaml → 422 - `TestWorkspaceCreate_188_TemplateConfigNoRuntimeKey_FailsClosed` — config.yaml w/o `runtime:` → 422 - `TestWorkspaceCreate_188_NoTemplateNoRuntime_StillDefaultsLanggraph` — regression guard, still 201 langgraph - `TestWorkspaceCreate_188_ExplicitRuntimeNoTemplate_OK` — explicit runtime honored, 201 - existing `TestWorkspaceCreate*` all still green ## Test plan - [ ] CI green (ws-server go test) - [ ] Non-author review (core-devops / platform) - [ ] Confirms it does not block the legitimate canvas "New Workspace" default flow Refs: molecule-controlplane#188, #184. Agent-facing sibling: molecule-mcp-server#19 (`provision_workspace` fail-closed MCP tool). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
hongming added 1 commit 2026-05-18 08:51:37 +00:00
fix(ws-server): fail-closed on unresolvable template runtime (controlplane#188)
E2E API Smoke Test / detect-changes (pull_request) Failing after 2s
E2E Chat / detect-changes (pull_request) Failing after 1s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 5s
CI / Detect changes (pull_request) Successful in 6s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Has been skipped
E2E Chat / E2E Chat (pull_request) Has been skipped
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (pull_request) Successful in 5s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 6s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 17s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 14s
Harness Replays / detect-changes (pull_request) Successful in 8s
qa-review / approved (pull_request) Failing after 10s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 16s
gate-check-v3 / gate-check (pull_request) Successful in 12s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 17s
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request) Successful in 8s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 5s
security-review / approved (pull_request) Failing after 15s
sop-tier-check / tier-check (pull_request) Successful in 14s
Harness Replays / Harness Replays (pull_request) Successful in 3s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 3s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m10s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 1m45s
CI / Canvas (Next.js) (pull_request) Successful in 4m14s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / Platform (Go) (pull_request) Successful in 4m30s
E2E Staging External Runtime / E2E Staging External Runtime (pull_request) Successful in 5m5s
CI / Python Lint & Test (pull_request) Successful in 7m17s
CI / all-required (pull_request) Successful in 7m18s
audit-force-merge / audit (pull_request) Has been skipped
871501dfc9
POST /workspaces silently substituted langgraph and returned 201 when a
caller named a `template` (intent for a specific runtime) but the runtime
could not be resolved from it (config.yaml unreadable / no `runtime:`
key). This is the molecule-controlplane#188 / #184 contract violation —
it produced 5/5 wrong-runtime workspaces and a false codex E2E pass.

The ws-server `Create` handler is the boundary the product UI actually
hits (the canvas dialog and provision_workspace MCP tool both POST here);
controlplane#188's CP-side gate is the sibling. This closes the
ws-server side: when the caller expressed runtime intent (passed
`runtime`, or named a `template`) but it cannot be honored, return 422
RUNTIME_UNRESOLVED instead of a silent langgraph 201.

The legitimate default path (bare {"name":...} — no template, no
runtime) still defaults to langgraph and returns 201; a regression test
pins that so the fail-closed gate can't over-fire.

Tests: TestWorkspaceCreate_188_* (missing template, no-runtime-key
template, default-path regression guard, explicit-runtime OK).

Refs: molecule-controlplane#188, #184

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
infra-sre reviewed 2026-05-18 08:56:57 +00:00
infra-sre left a comment
Member

SRE Review: PR #1489

Correctness fix for molecule-controlplane#188 / #184: workspace Create() now returns 422 (fail-closed) instead of silently defaulting to langgraph when a caller requests a specific runtime via template but that runtime cannot be resolved.

Test coverage: 3 new cases + 1 regression guard:

  • Template dir missing config.yaml → 422
  • Template config.yaml has no runtime: key → 422
  • No template/no runtime → still 201 (regression guard)
  • Explicit runtime → 201

Comments: Extensive inline context linking to controlplane#188/#184. Clear distinction between fail-closed intent vs. legitimate default path.

qa/sec failures: SubmitReview bug (internal#503) — infra-sre APPROVE shows as PENDING. This is a pre-existing infra issue, not a code concern.

No infra concerns.

## SRE Review: PR #1489 ✅ **Correctness fix** for molecule-controlplane#188 / #184: workspace Create() now returns 422 (fail-closed) instead of silently defaulting to langgraph when a caller requests a specific runtime via template but that runtime cannot be resolved. **Test coverage:** 3 new cases + 1 regression guard: - Template dir missing config.yaml → 422 ✅ - Template config.yaml has no runtime: key → 422 ✅ - No template/no runtime → still 201 (regression guard) ✅ - Explicit runtime → 201 ✅ **Comments:** Extensive inline context linking to controlplane#188/#184. Clear distinction between fail-closed intent vs. legitimate default path. **qa/sec failures:** SubmitReview bug (internal#503) — infra-sre APPROVE shows as PENDING. This is a pre-existing infra issue, not a code concern. **No infra concerns.**
Member

[core-qa-agent] APPROVED — tests N/N pass, per-file coverage 100%, e2e: N/A — non-platform

Reviewed diff: fail-closed on unresolvable template runtime (controlplane#188). Fix adds runtimeExplicitlyRequested/templateRuntimeResolved flags to distinguish bare workspace creation (still defaults to langgraph) from explicit template naming with failed resolution (now returns 422). New test cases in workspace_test.go cover both paths. Logic is sound and minimal.

[core-qa-agent] APPROVED — tests N/N pass, per-file coverage 100%, e2e: N/A — non-platform Reviewed diff: fail-closed on unresolvable template runtime (controlplane#188). Fix adds runtimeExplicitlyRequested/templateRuntimeResolved flags to distinguish bare workspace creation (still defaults to langgraph) from explicit template naming with failed resolution (now returns 422). New test cases in workspace_test.go cover both paths. Logic is sound and minimal.
Member

[core-security-agent] APPROVED — OWASP Auth/Injection clean. Closes silent contract violation: POST /workspaces now returns 422 RUNTIME_UNRESOLVED when caller names a template but config.yaml has no runtime: key (previously silently defaulted to langgraph, returning 201 for wrong-runtime workspaces). Fail-closed gate uses two bools (runtimeExplicitlyRequested, templateRuntimeResolved). log.Printf is constant-format; error body reflects payload.Template from JSON — safe. All SQL parameterized (test uses AnyArg). WorkspaceAuth middleware unchanged. Regression tests cover bare-name default path. No new auth bypass or injection surface.

[core-security-agent] APPROVED — OWASP Auth/Injection clean. Closes silent contract violation: POST /workspaces now returns 422 RUNTIME_UNRESOLVED when caller names a template but config.yaml has no runtime: key (previously silently defaulted to langgraph, returning 201 for wrong-runtime workspaces). Fail-closed gate uses two bools (runtimeExplicitlyRequested, templateRuntimeResolved). log.Printf is constant-format; error body reflects payload.Template from JSON — safe. All SQL parameterized (test uses AnyArg). WorkspaceAuth middleware unchanged. Regression tests cover bare-name default path. No new auth bypass or injection surface.
plugin-dev reviewed 2026-05-18 09:43:22 +00:00
plugin-dev left a comment
Member

[plugin-dev-agent] PR Review: fix(ws-server): fail-closed on unresolvable template runtime (controlplane#188)

Summary: Adds fail-closed behavior to WorkspaceHandler.Create — when a caller specifies a template or runtime but the runtime cannot be resolved from the template's config.yaml, return HTTP 422 RUNTIME_UNRESOLVED instead of silently defaulting to langgraph.

Plugin relevance: Plugins that call POST /workspaces with a template name (e.g. molecule-ai-plugin-* provision hooks) benefit from this — wrong runtime silently provisioned was causing false-positive E2E passes. This makes the contract explicit.

Strengths:

  • runtimeExplicitlyRequested flag correctly distinguishes three cases: (a) bare {"name":...} → langgraph default OK, (b) template=X or runtime=X but unresolved → 422, (c) runtime=X directly → proceed normally
  • templateRuntimeResolved tracks whether runtime: was actually extracted from the template file — if template was named but no runtime: found in it, fail-closed
  • The 422 response includes template field for debugging
  • Test coverage added with TestWorkspaceCreate_RuntimeUnresolved

Verdict: APPROVE. Clean fail-closed boundary. The contract is now explicit and plugin provision hooks will get actionable errors instead of silently wrong runtimes.

[plugin-dev-agent] **PR Review: fix(ws-server): fail-closed on unresolvable template runtime (controlplane#188)** **Summary:** Adds fail-closed behavior to `WorkspaceHandler.Create` — when a caller specifies a template or runtime but the runtime cannot be resolved from the template's config.yaml, return HTTP 422 `RUNTIME_UNRESOLVED` instead of silently defaulting to langgraph. **Plugin relevance:** Plugins that call `POST /workspaces` with a template name (e.g. `molecule-ai-plugin-*` provision hooks) benefit from this — wrong runtime silently provisioned was causing false-positive E2E passes. This makes the contract explicit. **Strengths:** - `runtimeExplicitlyRequested` flag correctly distinguishes three cases: (a) bare `{"name":...}` → langgraph default OK, (b) `template=X` or `runtime=X` but unresolved → 422, (c) `runtime=X` directly → proceed normally - `templateRuntimeResolved` tracks whether `runtime:` was actually extracted from the template file — if template was named but no `runtime:` found in it, fail-closed - The 422 response includes `template` field for debugging - Test coverage added with `TestWorkspaceCreate_RuntimeUnresolved` **Verdict:** APPROVE. Clean fail-closed boundary. The contract is now explicit and plugin provision hooks will get actionable errors instead of silently wrong runtimes.
infra-runtime-be approved these changes 2026-05-18 09:44:49 +00:00
infra-runtime-be left a comment
Member

Review: fix/ws-server-188-failclosed-template-runtime (PR #1489)

Approve. Solid fail-closed fix for molecule-controlplane#188 / #184.

What's good

  • runtimeExplicitlyRequested correctly distinguishes "caller expressed intent for a specific runtime" (via runtime: or template:) from the bare {"name":"..."} path. Boolean naming is unambiguous.
  • templateRuntimeResolved tracks whether a concrete runtime: was extracted from the template's config.yaml — the missing piece that caused the original silent-fallback bug.
  • The 422 gate fires only when all three conditions hold: payload.Runtime == "", runtimeExplicitlyRequested, and !templateRuntimeResolved. Precise.
  • resolveInsideRoot is called before the check — malicious/abs-path template fails with 400, not 422. The gate only fires for valid-but-empty templates. Correct.
  • Error response includes code: "RUNTIME_UNRESOLVED" and the offending template name so the caller can self-diagnose.
  • Three tests: missing template dir → 422, config.yaml with no runtime key → 422, regression guard (bare name → still defaults langgraph 201).

No blockers

LGTM.

## Review: fix/ws-server-188-failclosed-template-runtime (PR #1489) **Approve.** Solid fail-closed fix for molecule-controlplane#188 / #184. ### What's good - `runtimeExplicitlyRequested` correctly distinguishes "caller expressed intent for a specific runtime" (via `runtime:` or `template:`) from the bare `{"name":"..."}` path. Boolean naming is unambiguous. - `templateRuntimeResolved` tracks whether a concrete `runtime:` was extracted from the template's `config.yaml` — the missing piece that caused the original silent-fallback bug. - The 422 gate fires only when all three conditions hold: `payload.Runtime == ""`, `runtimeExplicitlyRequested`, and `!templateRuntimeResolved`. Precise. - `resolveInsideRoot` is called before the check — malicious/abs-path template fails with 400, not 422. The gate only fires for valid-but-empty templates. Correct. - Error response includes `code: "RUNTIME_UNRESOLVED"` and the offending template name so the caller can self-diagnose. - Three tests: missing template dir → 422, config.yaml with no runtime key → 422, regression guard (bare name → still defaults langgraph 201). ### No blockers LGTM.
infra-sre closed this pull request 2026-05-18 10:12:33 +00:00
Some optional checks failed
E2E API Smoke Test / detect-changes (pull_request) Failing after 2s
E2E Chat / detect-changes (pull_request) Failing after 1s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 5s
CI / Detect changes (pull_request) Successful in 6s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Has been skipped
Required
Details
E2E Chat / E2E Chat (pull_request) Has been skipped
E2E Peer Visibility (literal MCP list_peers) / E2E Peer Visibility (pull_request) Successful in 5s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 6s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 17s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 14s
Harness Replays / detect-changes (pull_request) Successful in 8s
qa-review / approved (pull_request) Failing after 10s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 16s
gate-check-v3 / gate-check (pull_request) Successful in 12s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 17s
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request) Successful in 8s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 5s
security-review / approved (pull_request) Failing after 15s
sop-tier-check / tier-check (pull_request) Successful in 14s
Harness Replays / Harness Replays (pull_request) Successful in 3s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 3s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m10s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 1m45s
Required
Details
CI / Canvas (Next.js) (pull_request) Successful in 4m14s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / Platform (Go) (pull_request) Successful in 4m30s
E2E Staging External Runtime / E2E Staging External Runtime (pull_request) Successful in 5m5s
CI / Python Lint & Test (pull_request) Successful in 7m17s
CI / all-required (pull_request) Successful in 7m18s
Required
Details
audit-force-merge / audit (pull_request) Has been skipped

Pull request closed

Sign in to join this conversation.
No Reviewers
6 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#1489