feat(integrations): add the local Open Design Cloud flow for Codex#6055
feat(integrations): add the local Open Design Cloud flow for Codex#6055open-design-crew[bot] wants to merge 7 commits into
Conversation
|
π§ͺ This PR introduces user-facing flow changes that need a manual QA pass before merge β please hold off merging for now; we'll loop QA in once the PR is merge-ready. |
π Landing page previewThis PR is deployed to a Cloudflare Pages preview β not staging or production:
|
mrcfps
left a comment
There was a problem hiding this comment.
@app/open-design-crew Thank you for the substantial work hereβthe capability isolation, durable Cloud workflow, secure-profile boundary, contracts, and focused coverage are thoughtfully assembled. I reviewed all changed ranges and verified the workspace guard, workspace typecheck, and the focused new daemon suites locally. One primary Preview-path issue needs correction before merge: the capability response currently prevents verified interactive prototypes from running. I also left two merge-safe follow-ups around secure-profile mutation consistency and CLI parity for keyless providers.
π Powered by Looper Β· runner=reviewer Β· agent=codex Β· An autonomous AI dev team for your GitHub repos.
Visual regression reviewHead:
0 changed Β· 0 unchanged Β· 0 new without baseline Β· 1 failed Capture or diff failures
Visual diff is advisory only and does not block merging. |
mrcfps
left a comment
There was a problem hiding this comment.
@app/open-design-crew Thank you for the substantial follow-up and for adding focused coverage around the local MCP and plugin packaging paths. I reviewed the full current changed range and verified the workspace guard/typecheck plus focused daemon, contracts, plugin-runtime, tools-pack, and Cloud E2E suites. This head introduces an additional primary-flow blocker by making the default Cloud plugin require a running local Open Design app, contrary to the linked issue and the PR's stated use case. The prior unresolved Preview CSP blocker in apps/daemon/src/routes/cloud-capabilities.ts also remains applicable because that code is unchanged on this head.
| All modes use the independently registered local `open-design` MCP server. The | ||
| plugin does not include an MCP transport and does not call a remote MCP domain. | ||
|
|
||
| If `open-design` is unavailable, explain that Open Design must be installed and | ||
| running. Ask the user to install or repair the Codex MCP registration from Open | ||
| Design Settings β MCP server or by running `od mcp install codex`. Do not invent |
There was a problem hiding this comment.
Blocking β keep the default Cloud install independent from Electron and the local daemon. These lines now require every mode to use an independently registered local open-design MCP and tell users to install and run the Open Design app. That directly contradicts both the linked #6054 acceptance criterion and this PR's current Why section, which require the plugin to work without installing or launching the desktop app; #6054 also explicitly rejects requiring the local daemon. With the bundled .mcp.json removed and the package validator now enforcing this local boundary, a fresh Codex plugin installation has no Cloud transport, so it cannot collect the brief, authenticate, or generateβthe primary flow fails before its first tool call. Please restore the production remote HTTPS Cloud MCP/OAuth path as the default plugin transport and reserve the local stdio MCP for explicitly selected Local Codex/BYOK modes, updating the manifest, package contract/validator, docs, and installation tests together. If the product boundary has intentionally changed, it needs a maintainer-approved scope decision rather than shipping as fulfillment of #6054.
mrcfps
left a comment
There was a problem hiding this comment.
@app/open-design-crew Thank you for rebuilding this into a focused local Vela-login sliceβthe CLI/MCP reuse and the config-path sanitization are clear and well covered. I reviewed all four changed files and verified the two focused suites, daemon typecheck, and workspace guard locally. I left one merge-safe safety-metadata correction for the new MCP tools.
π Powered by Looper Β· runner=reviewer Β· agent=codex Β· An autonomous AI dev team for your GitHub repos.| annotations: { ...WRITE_ANNOTATIONS, title: 'Sign in to Open Design Cloud' }, | ||
| }, | ||
| { | ||
| name: 'get_vela_login_status', | ||
| description: | ||
| 'Check whether Open Design Cloud (Vela/AMR) browser sign-in is complete. Does not expose Vela credentials.', | ||
| inputSchema: { type: 'object', properties: {}, additionalProperties: false }, | ||
| annotations: { ...READ_ANNOTATIONS, title: 'Check Open Design Cloud sign-in' }, |
There was a problem hiding this comment.
Non-blocking β mark these Cloud-auth tools as open-world interactions. Both definitions inherit openWorldHint: false, but their changed handlers cross the local daemon boundary: start_vela_login launches Vela device authorization and may open the user's browser, while get_vela_login_status calls /api/integrations/vela/status, whose logged-in path can invoke the Vela billing/account probe. MCP clients may use this annotation when deciding how to present consent or whether a tool is safe to run under a closed-world policy, so describing remote authentication/account access as local-only understates the effect. Please override both annotations with openWorldHint: true (while preserving the current read-only/write and idempotency hints), and assert that value for both tools in mcp-vela-login.test.ts so future shared-annotation refactors cannot regress it.
mrcfps
left a comment
There was a problem hiding this comment.
@app/open-design-crew Thank you for the focused rebuild and the careful coverage around the shared brief catalog, local confirmation store, MCP Apps resource, and Vela login surfaces. I reviewed all 10 changed files and verified the focused daemon/contracts suites, package typechecks, workspace guard, and full workspace typecheck locally. The earlier merge-safe Vela safety-annotation thread remains open; I added one new merge-safe MCP Apps capability-handling issue below.
π Powered by Looper Β· runner=reviewer Β· agent=codex Β· An autonomous AI dev team for your GitHub repos.| await request("ui/update-model-context", { | ||
| content: [{ type: "text", text: payload.summary }], | ||
| structuredContent: payload, | ||
| }); | ||
| await request("ui/message", { | ||
| role: "user", | ||
| content: [{ type: "text", text: prompt }], | ||
| }); |
There was a problem hiding this comment.
Non-blocking β honor the host capabilities before publishing the confirmation. The initialization result advertises updateModelContext and message independently, but this branch records only a boolean standardBridgeReady and then always awaits ui/update-model-context before sending ui/message. On a valid MCP Apps host that supports server tool calls and follow-up messages but omits context updates, confirm_brief has already made the draft immutable, this first request rejects, and the readable follow-up is never posted; retrying only repeats the same failure. The prompt sent by the next request already contains the full summary, so please retain result.hostCapabilities from ui/initialize, call each optional method only when its capability is advertised, and let a supported ui/message proceed even when context updates are unavailable. Add a bridge fixture where initialization returns serverTools + message without updateModelContext, then assert that submit still reaches ui/message after the successful tools/call. The capability split is defined in the MCP Apps host-capabilities contract: https://apps.extensions.modelcontextprotocol.io/api/interfaces/app.McpUiHostCapabilities.html
mrcfps
left a comment
There was a problem hiding this comment.
@app/open-design-crew Thank you for the substantial follow-upβthe headless MCP bootstrap wiring, focused tests, and data-root propagation are thoughtfully assembled. I reviewed all 20 changed files and verified the workspace guard, full workspace typecheck, and focused daemon, packaged, and contracts suites. One primary packaged lifecycle blocker remains: after Codex starts the windowless runtime, a normal Open Design launch exits after a no-op focus request, so the user cannot open the GUI until that headless owner is stopped. The two existing merge-safe Vela-annotation and MCP Apps capability threads also remain open.
π Powered by Looper Β· runner=reviewer Β· agent=codex Β· An autonomous AI dev team for your GitHub repos.| const request = normalizeDesktopSidecarMessage(message); | ||
| switch (request.type) { | ||
| case SIDECAR_MESSAGES.STATUS: | ||
| return { pid: process.pid, state: "running", url: webUrl, updatedAt: new Date().toISOString() }; | ||
| case SIDECAR_MESSAGES.SHUTDOWN: | ||
| setImmediate(() => { | ||
| void shutdown().finally(() => process.exit(0)); | ||
| }); | ||
| return { accepted: true }; | ||
| } |
There was a problem hiding this comment.
Blocking β let a normal app launch replace or surface the headless owner. This new desktop IPC handler reports state: "running" but handles only STATUS and SHUTDOWN. The normal packaged entry calls inspectExistingDesktopForLauncher; after confirming the daemon and web sidecars are healthy, that function sends SHOW and exits whenever the IPC request resolves. normalizeDesktopSidecarMessage accepts SHOW, so this switch falls through and the JSON IPC server still returns a successful response, which the launcher records as existing-focused even though this process intentionally created no BrowserWindow. As a result, once Codex auto-starts Open Design headlessly, clicking the installed Open Design app produces no UI until the headless runtime is stopped. Please make the windowless owner explicit and have the normal launcher shut it down and continue (or promote it into a windowful desktop) while retaining one owner for the sidecars. Add a lifecycle test that starts with a healthy headless STATUS, sends the normal launch's SHOW, and asserts that a windowful launch proceeds rather than exiting as focused.
mrcfps
left a comment
There was a problem hiding this comment.
@app/open-design-crew Thank you for the substantial, carefully tested follow-up across the local brief flow, packaged bootstrap, and secure BYOK profiles. I reviewed all 51 changed files and verified 440 focused tests, the workspace guard, and the full workspace typecheck. The existing primary packaged lifecycle blocker remains on this head: in apps/packaged/src/headless-runtime.ts, the windowless owner still accepts the normal launcher's SHOW request without surfacing a window, so a normal Open Design launch exits after Codex has bootstrapped the headless runtime. The existing MCP Apps capability-negotiation and Vela open-world annotation threads also remain. I added two merge-safe BYOK follow-ups below for keyless CLI parity and profile-metadata reconciliation. Thanks again for the thoughtful security boundary and broad test coverage. π
| if (!providerBaseUrl) { | ||
| return fail('--base-url must be an HTTP(S) URL without credentials, query, or fragment'); | ||
| } | ||
| if (!parsed.booleans.has('api-key-stdin')) { |
There was a problem hiding this comment.
Non-blocking β allow the CLI to save keyless BYOK profiles. The new shared request contract and credential service support requiresApiKey: false, and the web flow uses that path for a local Ollama endpoint, but this branch unconditionally rejects od byok save unless --api-key-stdin is present and then hard-codes requiresApiKey: true. That leaves a supported Local BYOK configuration reachable from the UI but not from the required CLI surface. Please add an explicit mutually exclusive keyless flag such as --no-api-key, skip stdin in that mode, send requiresApiKey: false, update the usage text, and cover both the keyless success path and conflicting flags in tools-byok-cli.test.ts.
| byokCredentialTail: undefined, | ||
| }; | ||
| } | ||
| return { |
There was a problem hiding this comment.
Non-blocking β reconcile the selected profile's metadata, not only its credential bit. The daemon response carries the authoritative protocol, baseUrl, model, and optional apiVersion, and the new CLI can update those fields while preserving the same profile id. This branch only flips byokCredentialConfigured/keyTail, so after such an update the browser can keep showing its stale provider and model while ProjectView sends only the id and the daemon runs with the newer profile metadata. That makes Settings and run analytics disagree with the runtime actually used. Please either project the matched profile's non-secret metadata into the active API config during hydration, or invalidate the selected id when the metadata differs and require explicit reconfirmation. Add a state-config test where the same profile id returns a changed base URL/model and assert the chosen reconciliation behavior.
mrcfps
left a comment
There was a problem hiding this comment.
@app/open-design-crew Thank you for the careful idempotency follow-upβthe durable request mapping, conflict response, recharge recovery shape, and focused tests are a strong step forward. I reviewed the full current changed range and verified the workspace guard, full workspace typecheck, and 478 focused daemon, packaged, web, and contracts tests. I found two current-head idempotency gaps around crash recovery and fingerprint completeness. The previously opened headless SHOW lifecycle thread also remains unresolved; this reviewer has reached the REQUEST_CHANGES cap, so Iβm leaving this follow-up as a COMMENT for maintainer judgment. Thanks again for the thoughtful implementation and broad coverage. π
| if (!runsLogDir || typeof id !== 'string' || !id) return null; | ||
| const statePath = path.join(runsLogDir, id, 'state.json'); | ||
| const state = readDurableRunState(statePath); | ||
| if (!state || state.id !== id || !TERMINAL_RUN_STATUSES.has(state.status)) return null; |
There was a problem hiding this comment.
Non-blocking follow-up β preserve the request mapping when the daemon dies mid-run. Startup records every durable clientRequestId, but this changed guard refuses to hydrate any saved queued or running state. createOrReuse then receives the mapped id, gets null from hydrateDurableRun, and falls through to create(meta). If the daemon is killed after an AMR run has started but before it writes a terminal state, retrying the same confirmed action after restart therefore creates a new local run and can initiate a second Cloud operationβthe exact duplicate this slice is intended to prevent. The new restart test waits for success before stopping the daemon, so it does not exercise this path. Please reconcile persisted non-terminal states on startup while retaining their request-id ownership (for example, materialize them as an interrupted/recovery-required terminal state or safely resume the same run id), and add a restart test that stops with state.json still running, retries the same request, and proves that no second agent/Cloud invocation starts.
| byokProfileId: meta.byokProfileId ?? null, | ||
| sessionMode: meta.sessionMode ?? null, | ||
| mediaExecution: meta.mediaExecution ?? null, | ||
| toolBundle: meta.toolBundle ?? null, |
There was a problem hiding this comment.
Non-blocking follow-up β fingerprint every execution-affecting request field. runRequestFingerprint hashes this hand-picked subset, but ChatRequest also carries fields that change the generated result or runtime behavior, including systemPrompt, attachments, commentAttachments, context, research, locale, byokMediaDefaults, and titleGeneration. Reusing one clientRequestId with a changed attachment or workspace context currently produces the same fingerprint, so createOrReuse silently returns the old run instead of the documented IDEMPOTENCY_CONFLICT; the tool description explicitly promises that a different payload with the same id is rejected. Please derive the fingerprint from the complete sanitized, normalized execution request (excluding only transport/recovery and analytics-only fields), or maintain an explicit exhaustive typed projection, and extend run-request-idempotency.test.ts with a matrix that changes each execution-affecting field and expects 409 while allowing analytics-only differences.
Scope-correction implementation update (2026-07-25)The corrected local-runtime implementation is now complete on this PR branch. Completed behavior:
Latest local validation:
Companion Vela validation:
Remaining work is release/maintainer-owned: review and merge, production Vela Note: installed odc 0.6.0 advertises PR body editing but its implementation only |
Fixes #6054
Why
Use case: Product wants Codex Desktop and Codex CLI users to install Open
Design support, confirm a brief, and generate a real Open Design artifact using
Cloud/Vela, Local Codex, or Local BYOK.
Pain: Open Design already has the product runtime, local MCP, projects,
runs, artifacts, agent adapters, and Vela CLI integration, but Codex users must
wire those pieces manually. The previous version of this branch also expanded
the task into a separate remote MCP, Cloud runtime, tenant store, and local
billing system. That duplicated existing ownership, obscured the real local
runtime prerequisite, and made the patch too large to review safely.
This branch has therefore been rebuilt from the latest
main. The correctedboundary is:
Vela remains the remote identity, wallet, charging, idempotency, and generation
owner. No remote Open Design MCP is introduced.
What users will see
When complete, Codex users will get an Open Design brief card, choose one of the
three explicit execution modes, and receive a real Open Design artifact.
path.
codex login.The first corrected slice now adds
od amr loginplus local MCPstart_vela_login/get_vela_login_status, reusing the existing Vela routesand bundled CLI resolution. Manual activation URL/code is surfaced when the
browser cannot open automatically.
Surface area
apps/weborapps/desktop(including Electron menu bar)od amr login/api/integrations/vela/*routes; no new HTTP contract in the current slicenexu-io/open-design-agent-plugins, not this repositoryThis checklist will be updated as the remaining slices land on this same PR.
Screenshots
No UI surface is present in the corrected first slice. Codex Desktop brief and
generated-artifact evidence will be attached after those flows are implemented
and exercised.
Bug fix verification
This is a feature and scope correction, not a standalone bug fix. The first
slice was still developed red-first:
apps/daemon/tests/amr-login-cli.test.tsapps/daemon/tests/mcp-vela-login.test.tsmain(6 failures) before the source change andpass on this branch (6 tests).
Validation
mise exec -- corepack pnpm --filter @open-design/daemon exec vitest run -c vitest.config.ts tests/amr-login-cli.test.ts tests/mcp-vela-login.test.ts --reporter=verboseβ 2 files / 6 tests passedmise exec -- corepack pnpm --filter @open-design/daemon typecheckβ passedmcp-runs,message-center-cli,integrations/vela.routes) β 3 files / 100 tests passedmise exec -- corepack pnpm guardβ passedFull workspace validation and real-host smoke remain pending until the
dependency-ordered implementation is complete.
Reviewer guide
Review the corrected implementation in this order as commits arrive:
start_run(agent: "amr").nexu-io/open-design-agent-plugins.Do not review for a remote MCP, a second Cloud project/runtime store,
subject-isolated local Cloud tenants, or a second billing ledger; those are no
longer part of this PR.
Remaining acceptance
latest
origin/main.od amr loginand local MCP Vela login/status tools.missing.
agent: "amr".codex login.exact Vela-owned blocker.
nexu-io/open-design-agent-pluginsin clean Codex CLI/Desktop homes.Codex Desktop evidence.
Unfinished work will continue on this same PR. Do not merge it yet.