feat: connection config — OpenAI-compatible LLM endpoint and Temporal namespace/TLS/API-key auth - #113
Open
dawidaksamski wants to merge 27 commits into
Open
feat: connection config — OpenAI-compatible LLM endpoint and Temporal namespace/TLS/API-key auth#113dawidaksamski wants to merge 27 commits into
dawidaksamski wants to merge 27 commits into
Conversation
dawidaksamski
marked this pull request as ready for review
September 2, 2026 07:23
dawidaksamski
requested review from
librowski,
lukasz-jazwa,
piotrblaszczyk and
szymon-t-sc
as code owners
September 2, 2026 07:23
dawidaksamski
force-pushed
the
feat/WB-596-connection-config
branch
from
September 3, 2026 11:01
d348521 to
cf4f5de
Compare
OPENROUTER_API_KEY is no longer read. As an unconditional fallback for AI_API_KEY it was sent as a bearer token to whatever AI_BASE_URL pointed at, so an old .env plus a repointed endpoint leaked the OpenRouter credential. There are no external deployments to keep compatible; rename the variable instead. AI_BASE_URL and AI_MODEL lose their code and compose defaults too, so nothing in the code points outside the network. The OpenRouter values live in .env.example only. AI is configured when all three AI_* vars are set; otherwise the worker boots and names the missing ones, AI Agent nodes fail with ai_not_configured, and the adapt route returns 501.
Missing AI configuration cannot recover on retry, yet the plain NodeExecutionError was retried once and lost its code crossing the activity boundary — node_failed carried only the message. Thrown as PermanentNodeExecutionError it stops on the first attempt and the code survives via the classified-error envelope. Adds a test through a real Temporal dev server asserting the node_failed code, a single attempt, and the workflow's failure type; the unclassified path is pinned alongside as the contrast.
The apps already read TEMPORAL_TLS_CA_PATH / _CERT_PATH / _KEY_PATH, but compose passed none of them and the docs told users to edit the manifest. Both services now take every TEMPORAL_* variable from one shared YAML block, so they cannot drift, and mount ./tls (override via TEMPORAL_TLS_DIR) read-only at /etc/workflowbuilder/tls. The directory ships empty with a .gitignore so PEMs never reach git.
Setting TEMPORAL_ADDRESS to an operated cluster or Temporal Cloud still started temporal and temporal-db, and the apps' depends_on edges let that unused stack block them. The bundled cluster, its volume, its debug UI and the start-order edges now live in docker-compose.override.yml, applied by default; COMPOSE_FILE=docker-compose.yml in .env leaves it out, so the apps depend only on app-db. The debug UI is documented as showing the bundled cluster only.
loadEnv only stubbed the values a case supplied, so variables inherited from the shell leaked into the fresh module and cases asserting "unset" tested whatever the runner happened to carry. Every variable env.ts reads is now unset before each import, derived from the module's own keys so a new one cannot be missed, and restored afterwards.
The standalone quick start covered AI_BASE_URL and keyless startup but none of the Temporal connection variables. Adds the namespace, TLS, API-key and mTLS table with the same semantics as the backend README, plus Temporal Cloud and private-CA examples. Also corrects the LLM section, which still described a built-in OpenRouter default, and moves the env snippets to the dotenv grammar the highlighter actually has.
dawidaksamski
force-pushed
the
feat/WB-596-connection-config
branch
from
September 4, 2026 11:50
cf4f5de to
168c5d9
Compare
…the VM The workflow drove a VM-local docker-compose.yml with -f, which disables the automatic override and ignores COMPOSE_FILE — after the bundled cluster moved into docker-compose.override.yml the demo VM would have run without Temporal. The deploy step now copies both compose files from the repo on every run, executes compose from /app/ai-studio, and passes the pushed tags as RUNTIME_IMAGE / WEB_IMAGE, so one compose file serves local builds and the VM.
The build context is the repo root and .dockerignore excluded only .env files, so PEMs dropped into deploy/ai-studio/tls per the mTLS docs were copied into the runtime image by `COPY . .`. The directory is now excluded; the files reach the containers through the read-only mount only.
The examples carried the placeholder `sk-or-...`, which envOptional treats as a configured key: a verbatim copy skipped the boot warning and sent requests to OpenRouter with a bogus token, surfacing a provider 401 instead of the documented ai_not_configured / 501 paths. The value is now empty and the key format lives in the comment.
The Full Stack Demo section still described AI_BASE_URL as defaulting to OpenRouter and listed a two-variable setup, contradicting the code and the docs site. It now mirrors the docs page: three variables, pre-filled by setup:env, no built-in default.
Compose no longer passes the retired variable, so a pre-rename .env came up with AI silently off and only a warn-level log to explain it. A compose-level guard now fails interpolation with a message naming the rename and the two new variables; the README and .env.example carry the upgrade note.
The troubleshooting row claimed the backend exits on a contradictory TEMPORAL_* setup; it connects on first use, so it boots, passes its healthcheck and fails on the first Play. Also aligns wording across the READMEs, .env.example files and docs page with the code: no built-in LLM default, any credential implies TLS, provider-neutral phrasing.
Only deploy/ai-studio/tls was dockerignored, so TEMPORAL_TLS_DIR=./certs with a key inside the checkout was copied into the runtime image by COPY . . on a local build. Exclude deploy/ as a whole (re-including only deploy/ai-studio/nginx, the one file the Dockerfile copies) and *.pem/*.key/*.crt/*.cer/*.p12/ *.pfx repo-wide. Document that TEMPORAL_TLS_DIR supports exactly ./tls or a directory outside the checkout. Verified with control files: none of the in-repo locations reach the build context; nginx/default.conf and a repo-root positive control do.
The deploy step only exported RUNTIME_IMAGE / WEB_IMAGE for its own shell, so any later compose command on the VM (worker restart after a model change, the debug profile) fell back to the local ai-studio-* build names. Rewrite the two image lines in /app/ai-studio/.env on every deploy instead, leaving the rest of the file untouched, and drop the export so the deploy itself runs off the persisted values. Verified locally: after a simulated deploy against a stale .env, a fresh process with no inherited variables resolves both deployed tags via `docker compose config --images`; other .env lines and mode are unchanged.
… end to end The connection builder tests only compared the options object built from fake certificate bytes; nothing showed the backend's gRPC client or the worker's native transport would complete or refuse a real handshake. Add a shared test harness in apps/tools (throwaway CA with server and client leaves, a TLS-terminating proxy in front of the Temporal dev server, and an HTTP/2 endpoint that records bearer tokens) and drive both builders through it: private CA, mutual TLS, an untrusted server CA, a client certificate from the wrong CA, an API key inside the TLS session, and work in a non-default namespace. No Docker or Temporal Cloud needed.
Backend and worker carried identical validation, TLS inference and
certificate reading, kept aligned only by a "keep in sync" comment, plus
duplicated defaults for the address and namespace. The option types were
the excuse, but both SDKs accept a plain apiKey string and the same tls
shape, so a narrow shared contract fits both uncast.
Move all of it into a private source-only workspace,
@workflow-builder/temporal-connection, behind a single temporalConfig()
returning { connection, namespace }; each app hands the connection object
to its SDK's connect call. Reading and validation still happen where they
did: in the backend's first-connection factory and before the worker
starts. An empty TEMPORAL_ADDRESS or TEMPORAL_NAMESPACE now falls back to
the default like the other variables already did.
The validation matrix lives once in the new package, and so do the TLS
connection tests and their harness, a describe.each over both SDK
transports; handing the built options to both connect calls there is the
compile-time proof of assignability. The app suites are pure unit tests
again and apps/tools is untouched. The workspace is added to the CI
execution job's filter list, knip and CLAUDE.md.
…ons apart The rule that AI_API_KEY, AI_BASE_URL and AI_MODEL must be set together lived three times: the backend's adapt route, the worker's executor factory and its startup warning. Both apps also normalised empty values on their own. Add a private source-only workspace, @workflow-builder/ai-config, whose aiConfig() returns a complete config or the names of the missing variables, and make all three sites read it. What each app does when AI is unavailable is unchanged and stays in the app: the backend answers 501 after authorization and the guard, the worker boots and fails an AI Agent node with permanent ai_not_configured only when a run reaches it. The package README is the canonical description of the contract; both app READMEs and .env.example files point at it. The first-run guides named a template that does not exist and invited Play before the LLM section; they now say every bundled template has AI Agent nodes and what to expect without an LLM.
Several docs said an internal AI_BASE_URL keeps all traffic in your network, or that nothing in the code points outside it. The optional web-search tool calls Tavily's API whenever TAVILY_API_KEY is set, a node enables search and the model invokes the tool, regardless of where the model runs. Say "model requests stay inside it" instead, note that the Tavily key must stay unset if nothing may call out, and remind readers that Temporal and the database go wherever their addresses point. Wording only; no behaviour changed.
Remove the file header on the AI agent executor factory that repeated what its name and body already say. The comments explaining the real contracts stay: TEMPORAL_TLS is tri-state on purpose, and the AI configuration error is deferred to node execution so a keyless worker still boots. Rename the certificate reader readPemFile so call sites say what kind of file they read.
dawidaksamski
removed this pull request from stack #115
September 9, 2026 15:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the reference stack's outbound connections configurable without code changes, and makes the code itself point nowhere by default.
The backend and worker build their model through the generic
@ai-sdk/openai-compatibleprovider driven byAI_API_KEY,AI_BASE_URLandAI_MODEL. None of the three has a built-in value: the OpenRouter endpoint and model live in.env.exampleonly, so any OpenAI-compatible endpoint works, including one inside the customer's network, and model requests stay whereverAI_BASE_URLpoints. AI is configured only when all three are set; otherwise the stack fails closed. The worker boots keyless and runs every other node type, an AI Agent node reached without configuration fails withai_not_configured(aPermanentNodeExecutionError, so it is not retried and its code survives the Temporal activity boundary), and the backend's adapt route returns 501. Both processes acceptTEMPORAL_NAMESPACEplus optional TLS, mTLS and API-key configuration, validated fail-loud when the connection opens. Defaults preserve today's plaintext local-dev Temporal behaviour exactly.The rules both processes must agree on live once each, in two private source-only workspaces:
@workflow-builder/temporal-connection(temporalConfig()→ connection options and namespace) and@workflow-builder/ai-config(aiConfig()→ a complete LLM config or the names of the missing variables). What each app does with an unavailable result stays in the app.Closes the first subtask of WB-596 (WB-600).
What changed
@openrouter/ai-sdk-provider→@ai-sdk/openai-compatible, driven byAI_BASE_URL; no built-in endpoint or modelAI_API_KEYreplacesOPENROUTER_API_KEY, which is no longer read anywhere (see "Upgrading")@workflow-builder/ai-config: one parser for the all-or-nothingAI_*trio, empty counts as unset; read by the adapt route, the executor factory and the worker's startup warningai_not_configured(permanent, one attempt); adapt route returns 501 after authorization and the execution guard@workflow-builder/temporal-connection:TEMPORAL_ADDRESS,TEMPORAL_NAMESPACE, tri-stateTEMPORAL_TLS,TEMPORAL_API_KEY,TEMPORAL_TLS_CA_PATH/_CERT_PATH/_KEY_PATHread and validated once; backend calls it in its first-connection factory, worker before it starts polling; contradictory combinations rejected therex-temporal-envanchor feeds everyTEMPORAL_*variable to backend and worker, so the two cannot drift./tls(git-ignored) mounted read-only into both containers at/etc/workflowbuilder/tls;TEMPORAL_TLS_DIRmay be./tlsor a directory outside the checkout, nothing else in-repo.dockerignoreexcludesdeploy/as a whole (re-including onlydeploy/ai-studio/nginx) plus*.pem/*.key/*.crt/*.cer/*.p12/*.pfxrepo-wide, so no supported certificate location can reachCOPY . .docker-compose.override.yml(applied by default);COMPOSE_FILE=docker-compose.ymlin.envleaves it out-f, so the override andCOMPOSE_FILEapply), ships both compose files to the VM on every deploy, and rewritesRUNTIME_IMAGE/WEB_IMAGEin the VM's.envwith the pushed tags so later compose commands on the VM keep using themOPENROUTER_API_KEYis still set, naming the rename and the two new variablesaiand@ai-sdk/openai-compatiblemoved into the pnpm catalog; their majors are coupled (v2↔aiv6,v3↔aiv7).node-forgeand@temporalio/testingare dev dependencies oftemporal-connectiononlyDocumentation covers all surfaces: both app READMEs (linking to the two package READMEs as the canonical description of each contract), the deploy README, the docs-site quick start (new "secured or external Temporal" section with Temporal Cloud and mTLS examples) and the root README. Three claims were corrected along the way: "AI Studio works with stub responses out of the box" was never true and is gone; the first-run guides named a template that does not exist and invited Play before the LLM section, and now say every bundled template has AI Agent nodes and what to expect without an LLM; and "no request leaves your network" is narrowed to model requests, since the optional Tavily web search calls out whenever its key is set.
Upgrading an existing deployment
In
.env, renameOPENROUTER_API_KEYtoAI_API_KEYand addAI_BASE_URLandAI_MODEL(values in.env.example). Compose fails fast with a message while the old name is present, so a stale.envcannot come up with AI silently off. The demo VM's.envneeds this before the next deploy, and the deploy workflow now overwrites the VM's compose files with the repo's, so any VM-local customisation must be ported to the repo first. The deploy also rewrites the two image lines in the VM's.envon every run; everything else in that file is left as is. If you setTEMPORAL_TLS_DIR, it must be./tlsor a path outside the checkout.Verification
Automated: backend 101, worker 18, execution-core 168,
@workflowbuilder/temporal83,temporal-connection27,ai-config9 tests pass; lint, typecheck, prettier and knip are clean for every touched workspace; the docs site builds. The app suites are pure unit tests again; the env tests unset every variable the module reads before each import, so they are independent of the runner's environment.Through a real boundary:
packages/temporal/test/error-boundary.test.tsruns graphs against a real Temporal dev server and asserts that a permanent throw stops after one attempt with its code innode_failedand in the workflow's failure type, while an unclassified throw retries per the profile and is reported exactly as before.packages/temporal-connection/test/tls.test.tsruns the same matrix on both SDK transports: private CA, mutual TLS, an untrusted server CA (rejected, handshake failure recorded on the server side), a client certificate from the wrong CA (rejected), a synthetic API key observed asBearerinside the TLS session, and work in a non-default namespace (the client's workflow lands in that namespace and not indefault; a worker polls it over TLS and completes a workflow). The negative cases are controlled pairs of the positive ones, differing only in which CA is trusted or which client certificate is presented. Handing the built options to both SDKs' connect calls there is the compile-time proof that the shared contract fits both without a cast.Against Docker:
docker compose config): default includes the bundled cluster and edges, external mode has notemporal*services and the apps depend only onapp-db;TEMPORAL_*is identical between backend and worker in both.host.docker.internal), workerRUNNINGwith zero restarts in each.tls/mount is visible and write-rejected inside a running container. Non-secret control files placed at every in-repo certificate location (./tls, a sibling./certsasTEMPORAL_TLS_DIRwould allow, a non-certificate filename underdeploy/, a.keyin a directory outsidedeploy/) were all absent from a probe build of the repo-root context, while a repo-root positive control anddeploy/ai-studio/nginx/default.confarrived..env.exampleand an empty environment start; a.envwith onlyOPENROUTER_API_KEYis refused in both modes; a correctly renamed one starts..envrewrite, extracted from the workflow file and run against a stale.env(other settings, old tags mid-file, mode 600, no trailing newline): a fresh process with no inherited variables resolves both deployed tags viadocker compose config --images; every other line and the file mode are unchanged.Not verified:
TEMPORAL_TLS=trueagainst a publicly trusted certificate (the OS-trust-store path Temporal Cloud uses). Neither CI nor a dev machine has one; the local TLS tests use a private CA for every case.Known, deferred
Found by an internal multi-perspective review and left out deliberately; none blocks the change.
packages/temporalclient factory caches a rejected connection promise until restart (pre-existing; this PR moves config validation into that factory). Fix: reset the promise on rejection.onErrorlogserror.messageonly, dropping thecausethe PEM reader attaches (ENOENT vs EACCES on a certificate path).ai-studio_temporal-db-datavolume behind; docs should adddocker volume rm.docker inspect); file-based*_API_KEY_PATHvariants are a follow-up.docker compose up -d --buildas "deploy a new version"; on the VM, with registry tags now pinned in.env, that would build locally under the registry name.envkeys equalling variable names; the pre-existingWB-229id in the compose header.Shared configuration packages
Review of an earlier revision flagged the connection builder as the largest duplicated, security-relevant block in the repo. The rationale for duplicating it (the two SDKs type
apiKeydifferently) did not hold: both accept a plain string and the sametlsshape, so a narrow shared contract fits both uncast.@workflow-builder/temporal-connectionand@workflow-builder/ai-configfollow the@workflow-builder/typespattern: private,exportspointing at.tssource, no build step, consumed by the apps through tsx and included in the Docker--prodinstall as workspace dependencies. Neither touchesexecution-core(engine plumbing, not the execution model) nor the published@workflowbuilder/temporalAPI. Sharing the parsers keeps the two readings of each rule identical; it cannot make two independently configured processes agree on the values, which is why both.env.examplefiles keep the full variable list.