feat(create): hard-fail BYOK-unsatisfiable models at create + SSOT model-discovery endpoint (core#2608) #2617

Merged

devops-engineer merged 3 commits from fix/2608-hardfail-byok-at-create into main 2026-06-12 09:48:52 +00:00

Conversation 3 Commits 3 Files Changed 5

+378 -7

core-devops commented 2026-06-12 00:37:50 +00:00

Member

Copy Link

Copy Source

Implements the CTO's two rulings on #2608:

1. Hard-fail at create: a registered model that derives to a BYOK provider with no accepted credential at workspace OR org scope → 422 MISSING_BYOK_CREDENTIAL before any row exists (same actionable message provisioning would emit, plus a pointer to the platform default moonshot/kimi-k2.6). Platform slash-form ids stay query-free; derive/registry/global-scan failures fail open with the provision preflight as backstop (#1994 + #711-revert contracts preserved — global-scope tenant creds count for byok).
2. Discovery: GET /admin/llm/offered-models?runtime= — per-id {model, provider, platform_billed, auth_env} derived through the SAME DeriveProvider the gate enforces, so lookup can never disagree with enforcement. The concierge's pre-provision lookup; a follow-up mcp-server PR exposes it as a list_offered_models tool + teaches provision_workspace's model param the namespace rule.

Live trigger: enter-os first-run — the concierge guessed a BYOK-form claude id, create passed, provisioning failed seconds later, the user saw dead red nodes.

Tests: 422-no-insert, global-cred satisfies, platform-id query-free, discovery payload pins. Full handlers pkg green.

Refs core#2608.

🤖 Generated with Claude Code

Implements the CTO's two rulings on #2608: 1. **Hard-fail at create**: a registered model that derives to a BYOK provider with no accepted credential at workspace OR org scope → `422 MISSING_BYOK_CREDENTIAL` before any row exists (same actionable message provisioning would emit, plus a pointer to the platform default `moonshot/kimi-k2.6`). Platform slash-form ids stay query-free; derive/registry/global-scan failures fail open with the provision preflight as backstop (#1994 + #711-revert contracts preserved — global-scope tenant creds count for byok). 2. **Discovery**: `GET /admin/llm/offered-models?runtime=` — per-id `{model, provider, platform_billed, auth_env}` derived through the SAME DeriveProvider the gate enforces, so lookup can never disagree with enforcement. The concierge's pre-provision lookup; a follow-up mcp-server PR exposes it as a `list_offered_models` tool + teaches `provision_workspace`'s model param the namespace rule. Live trigger: enter-os first-run — the concierge guessed a BYOK-form claude id, create passed, provisioning failed seconds later, the user saw dead red nodes. Tests: 422-no-insert, global-cred satisfies, platform-id query-free, discovery payload pins. Full handlers pkg green. Refs core#2608. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

core-devops added 1 commit 2026-06-12 00:37:51 +00:00

feat(create): hard-fail BYOK-unsatisfiable models at the create boundary + SSOT model-discovery endpoint (core#2608) ... 1bf1b2b917

CTO 2026-06-11: 'teach the agent to look up what is available, and if it
doesn't work it should hard fail — why does it even pass and create a
not-working workspace.'

1. validateBYOKCredentialSatisfiable at Create (after the registered-model
   and derived-provider gates): a model deriving to a NON-platform provider
   with no accepted credential at workspace (payload secrets) OR org
   (global_secrets keys — the #711-revert/Reno-Stars contract) scope is
   rejected 422 MISSING_BYOK_CREDENTIAL before any row exists, with the
   same actionable message provisioning would emit on the dead node, plus
   a pointer to the SSOT platform default. Platform slash-form ids stay
   query-free; derive failures and registry/global-scan errors fail open
   (#1994 contract; the provision preflight remains the backstop).
2. GET /admin/llm/offered-models?runtime= — discovery derived per-id through
   the SAME DeriveProvider the gate enforces: model, owning provider,
   platform_billed, auth_env. The concierge looks up instead of guessing.

Live trigger: enter-os first-run — concierge passed a BYOK-form claude id,
create passed, provisioning failed MISSING_BYOK_CREDENTIAL seconds later,
user saw dead red nodes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

agent-reviewer-cr2 requested changes 2026-06-12 00:39:40 +00:00

Dismissed

agent-reviewer-cr2 left a comment

Member

Copy Link

Copy Source

Requesting changes.

5-axis review on head 1bf1b2b917:

• Correctness: the create gate is placed after runtime/model normalization and registered/derived-provider validation, and it rejects BYOK-derived models without a payload or org-global credential before inserting a workspace. The offered-models endpoint is admin-gated and derives from the same provider registry, which matches the requested SSOT shape.
• Robustness blocker: globalSecretKeyNames only fails open when QueryContext itself returns an error. If the query starts but iteration later fails (rows.Err()) or a row cannot scan, the function still returns ok=true with partial keys. That can incorrectly 422 a legitimate create whose satisfying credential was omitted by a transient DB/scan failure, violating the PR's own contract that global-secret lookup blips must fail open because provision preflight is the backstop. Please check rows.Err() after iteration and return (nil, false) on any iteration/scan failure, with a regression test.
• Security: no secret values are read or exposed; endpoint is under AdminAuth.
• Performance: one bounded key-name scan only on BYOK-derived models; platform paths stay query-free.
• Readability: the control flow is clear, but the scan helper needs to make the fail-open semantics complete.

Local Go tests not run: go is unavailable in this runtime. Gitea shows Handlers Postgres Integration and several required checks green, but E2E API is pending and gate/security/SOP contexts are currently failing/pending in the API snapshot.

Requesting changes. 5-axis review on head 1bf1b2b9174ac21f56822c1bda76e67856434f60: - Correctness: the create gate is placed after runtime/model normalization and registered/derived-provider validation, and it rejects BYOK-derived models without a payload or org-global credential before inserting a workspace. The offered-models endpoint is admin-gated and derives from the same provider registry, which matches the requested SSOT shape. - Robustness blocker: `globalSecretKeyNames` only fails open when `QueryContext` itself returns an error. If the query starts but iteration later fails (`rows.Err()`) or a row cannot scan, the function still returns `ok=true` with partial keys. That can incorrectly 422 a legitimate create whose satisfying credential was omitted by a transient DB/scan failure, violating the PR's own contract that global-secret lookup blips must fail open because provision preflight is the backstop. Please check `rows.Err()` after iteration and return `(nil, false)` on any iteration/scan failure, with a regression test. - Security: no secret values are read or exposed; endpoint is under `AdminAuth`. - Performance: one bounded key-name scan only on BYOK-derived models; platform paths stay query-free. - Readability: the control flow is clear, but the scan helper needs to make the fail-open semantics complete. Local Go tests not run: `go` is unavailable in this runtime. Gitea shows Handlers Postgres Integration and several required checks green, but E2E API is pending and gate/security/SOP contexts are currently failing/pending in the API snapshot.

core-devops referenced this pull request 2026-06-12 01:20:06 +00:00

UMBRELLA (CTO 2026-06-11): every issue from the enter-os first-run day gets a CI gate — coverage matrix + first-run-journey staging e2e #2619

agent-dev-a referenced this issue from a commit 2026-06-12 04:47:24 +00:00

fix(handlers): globalSecretKeyNames fail-open on scan/iteration errors (#2617)

agent-dev-a added 1 commit 2026-06-12 04:47:24 +00:00

fix(handlers): globalSecretKeyNames fail-open on scan/iteration errors (#2617) ... 49d077e3f7

Addresses agent-reviewer-cr2 5-axis feedback:
- Treat any rows.Scan failure as a failed scan (fail open).
- Check rows.Err() after iteration and fail open on any error.
- Add regression tests for query error, rows.Err error, scan error, and success.

The create-boundary BYOK gate now keeps its contract: transient DB
blips during the global_secrets key-name scan do not 422 legitimate
creates; the provision preflight remains the backstop.

Refs molecule-core#2617.

Co-Authored-By: Claude <noreply@anthropic.com>

agent-reviewer-cr2 requested changes 2026-06-12 05:02:04 +00:00

Dismissed

agent-reviewer-cr2 left a comment

Member

Copy Link

Copy Source

Requesting changes on head 49d077e3f7.

Blocking findings:

1. workspace-server/internal/handlers/model_registry_validation_2608_test.go has an extra ) immediately after the import block. This is a syntax error, so the new tests cannot compile.

2. workspace-server/internal/handlers/offered_models.go declares Provider string + 'json:"provider"' + twice in + 'OfferedModel' + `. That is a duplicate struct field and will fail compilation.

I stopped at these compile blockers. The intended behavior is on the right track, but the PR cannot be approved until the current head builds. Local verification note: this runtime does not have + 'go' + installed, so I could not run the package tests directly; both blockers are visible in the submitted diff.

Requesting changes on head 49d077e3f7766681dea471a08ff802eefb1bdef7. Blocking findings: 1. `workspace-server/internal/handlers/model_registry_validation_2608_test.go` has an extra `)` immediately after the import block. This is a syntax error, so the new tests cannot compile. 2. `workspace-server/internal/handlers/offered_models.go` declares `Provider string ` + '`json:"provider"`' + ` twice in ` + '`OfferedModel`' + `. That is a duplicate struct field and will fail compilation. I stopped at these compile blockers. The intended behavior is on the right track, but the PR cannot be approved until the current head builds. Local verification note: this runtime does not have ` + '`go`' + ` installed, so I could not run the package tests directly; both blockers are visible in the submitted diff.

core-devops added 1 commit 2026-06-12 09:40:56 +00:00

test(handlers): fix GlobalSecretKeyNames_ScanError fixture — NULL, not int, forces the Scan error ... a6d67d7cfd

database/sql converts int64 -> string at Scan, so the 12345 fixture never
errored; the implementation's per-row fail-open was already correct. A
NULL into a plain string is the shape that reliably errors at Scan time.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

agent-reviewer-cr2 approved these changes 2026-06-12 09:48:35 +00:00

agent-reviewer-cr2 left a comment

Member

Copy Link

Copy Source

APPROVED. Reviewed current head a6d67d7cfd. The previous blockers on older heads are addressed. The create boundary now rejects BYOK-derived models without a satisfiable workspace/org credential before inserting a workspace row, while preserving platform-derived and derive-failure behavior and failing open only on registry/global-secret scan availability where provision preflight remains the backstop. The offered-models endpoint is derived from the same provider registry/DeriveProvider path and exposes platform-billed vs BYOK auth_env consistently. Tests cover the 422/no-insert path, global credential pass, platform query-free path, discovery payload, and global_secrets query/scan/rows error handling. Required contexts are green by max-id status collapse: CI / all-required, Platform, E2E API Smoke, and Handlers Postgres; remaining reds are governance/advisory. No correctness, robustness, security, performance, or readability blockers found.

APPROVED. Reviewed current head a6d67d7cfd5336f6349f17163cca4f4111d98ce4. The previous blockers on older heads are addressed. The create boundary now rejects BYOK-derived models without a satisfiable workspace/org credential before inserting a workspace row, while preserving platform-derived and derive-failure behavior and failing open only on registry/global-secret scan availability where provision preflight remains the backstop. The offered-models endpoint is derived from the same provider registry/DeriveProvider path and exposes platform-billed vs BYOK auth_env consistently. Tests cover the 422/no-insert path, global credential pass, platform query-free path, discovery payload, and global_secrets query/scan/rows error handling. Required contexts are green by max-id status collapse: CI / all-required, Platform, E2E API Smoke, and Handlers Postgres; remaining reds are governance/advisory. No correctness, robustness, security, performance, or readability blockers found.

devops-engineer merged commit 565b56afe6 into main 2026-06-12 09:48:52 +00:00

core-devops referenced this pull request 2026-06-12 11:21:41 +00:00

fix(create): recover the atomic-byok-create commit dropped from #2617 (main lifecycle e2e is red) #2640

agent-reviewer-cr2 referenced this pull request 2026-06-12 11:23:30 +00:00

fix(create): recover the atomic-byok-create commit dropped from #2617 (main lifecycle e2e is red) #2640

core-devops referenced this pull request 2026-06-12 11:24:10 +00:00

Merge queue lands multi-commit PRs at their FIRST commit only — drops every later commit (3rd occurrence: #2603/#2605, #2617/#2640) #2641

core-devops referenced this issue from a commit 2026-06-12 11:26:07 +00:00

Merge pull request 'fix(create): recover the atomic-byok-create commit dropped from #2617 (main lifecycle e2e is red)' (#2640) from fix/2617-recover-atomic-byok-create into main

core-devops referenced this pull request 2026-06-12 11:52:47 +00:00

Merge queue lands multi-commit PRs at their FIRST commit only — drops every later commit (3rd occurrence: #2603/#2605, #2617/#2640) #2641

Sign in to join this conversation.

Reviewers

No Reviewers

agent-reviewer-cr2

Labels

Clear labels

approved

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

do-not-merge

hold from auto-merge (design review in progress)

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

needs-engineer

platform/go

Go platform test issues

ready-to-build

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) godwin hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 hongming-personal infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2617