feat: connection config — OpenAI-compatible LLM endpoint and Temporal namespace/TLS/API-key auth - #113
Open
dawidaksamski wants to merge 18 commits into
Open
feat: connection config — OpenAI-compatible LLM endpoint and Temporal namespace/TLS/API-key auth#113dawidaksamski wants to merge 18 commits into
dawidaksamski wants to merge 18 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.
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 nothing in the code names an external host. 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 through a unit-tested option builder. Defaults preserve today's plaintext local-dev Temporal behaviour exactly.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")ai_not_configured(permanent, one attempt); adapt route returns 501TEMPORAL_NAMESPACEplus optionalTEMPORAL_TLS(tri-state),TEMPORAL_API_KEY,TEMPORAL_TLS_CA_PATH/_CERT_PATH/_KEY_PATH, in backend and worker; contradictory combinations rejected when the connection opensx-temporal-envanchor feeds everyTEMPORAL_*variable to backend and worker, so the two cannot drift./tls(git-ignored, excluded from the image build context,TEMPORAL_TLS_DIRto override) mounted read-only into both containers at/etc/workflowbuilder/tlsdocker-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, passes pushed tags asRUNTIME_IMAGE/WEB_IMAGEOPENROUTER_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)Documentation covers all surfaces: both app READMEs, the deploy README, the docs-site quick start (new "secured or external Temporal" section with Temporal Cloud and mTLS examples) and the root README. The earlier claim that "AI Studio works with stub responses out of the box" was never true and is gone; the stack now genuinely runs without an LLM and the docs describe what happens.
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.Verification
Automated: backend 117, worker 32, execution-core 168,
@workflowbuilder/temporal83 tests pass; lint, typecheck and prettier are clean for every touched workspace; the docs site builds. 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.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; a PEM dropped there is git-ignored and absent from the image build context (probe build fails toCOPYit)..env.exampleand an empty environment start; a.envwith onlyOPENROUTER_API_KEYis refused in both modes; a correctly renamed one starts.Not verified: a real TLS/mTLS handshake against a secured cluster or Temporal Cloud. Neither CI nor a dev machine has one; the option shapes are pinned by tests and checked against both SDKs' types.
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 builder attaches (ENOENT vs EACCES on a PEM path).ai-studio_temporal-db-datavolume behind; docs should adddocker volume rm.docker inspect); file-based*_API_KEY_PATHvariants are a follow-up.envkeys equalling variable names; a drift test between the twotemporal-connection.tscopies; a few comments to trim; the worker README's structure tree omits the new files; the pre-existingWB-229id in the compose header.To consider: the duplicated connection builder
buildTemporalConnectionOptionsexists twice, verbatim apart from one type import (apps/backend/src/engine/temporal-connection.ts,apps/execution-worker/src/engines/temporal/temporal-connection.ts), with mirrored tests. Deliberate: the two SDKs typeapiKeydifferently, there is no shared config package, and a new workspace costs tsconfig, knip and lockfile wiring. Against it: this is now the largest duplicated block in the repo and it is security-relevant; if the copies drift, backend and worker could disagree about whether TLS is on, and the mirrored tests are the only thing catching that. Recommendation: leave it for this PR and revisit if a third consumer appears, or add the drift test listed above as a cheap middle ground.