fix: real-toolchain-validated SDK emitter fixes (js-sdk, python-sdk, csharp-sdk)

[johnOC03]

Summary

A combined series of fixes across all three SDK emitters (js-sdk, python-sdk, csharp-sdk), each found and verified by actually running the real generated toolchain (npm install+vitest, poetry install+pytest, dotnet build+test) against a live broker — not just unit tests. Rebased cleanly onto current main (0 conflicts remaining after resolution); full local pre-push checklist (lint, all workspace typechecks, pipeline regeneration, npm test) passes with 994 passed / 12 skipped / 0 failed.

js-sdk

• Restored globalContextSeeds wiring and agent-connector semantics (seedBinding emits literal '<default>' for tenantIdVar, causing HTTP 409 on every tenant producer #342).
• Materialized real multipart/@@FILE fixture resolution and seed-binding support (previously silently dropped required fields).
• Fixed path-param URL derivation (previously always empty due to a step.pathParams dead-field bug — see repo history for the full class of this bug across emitters).
• Fixed the generated project's tsconfig.json lib setting so Error(message, { cause }) typechecks (ES2022.Error).

python-sdk

• Materialized real multipart/@@FILE fixture bytes instead of passing an unresolved marker string.
• Fixed buildPythonUrlExpression's path-param derivation (same class of bug as js-sdk).

csharp-sdk

• Aligned the emitter with the real, installed Camunda.Orchestration.Sdk 9.2.2 surface (via direct assembly reflection, not guesswork): strongly-typed key .AssumeExists(...) construction, correct request-DTO type names, bare-Task-returning method handling, RequireStringBinding for string? SDK parameters, and 41 missing body-type mappings for search operations.
• Fixed the CsharpOperationMap type to allow readonly entry arrays (needed for as const test fixtures).

Rebase / cleanup

• Deduped 3 configs/camunda-oca/ontology/semantics.json semantic-type entries that main had independently reclassified during the time this branch was in flight (AgentDefinitionKey, AgentHistoryItemKey, LoopIterationId) — deferred to main's already-reviewed classification rather than re-litigating.
• Kept main's superior scripts/e2e/run-oca.sh positive-suite-runner logic (proper PW_FAIL tracking) over this branch's now-superseded fix for the same bug.
• Kept this branch's Camunda.Orchestration.Sdk 9.2.2 pin in CamundaIntegrationTests.csproj (verified as the current, correct version this branch's csharp-sdk work was built/tested against) over main's stale 9.0.0.

Known blockers (not fixed here — flagged for follow-up)

Live-broker validation surfaced several pre-existing, cross-cutting issues that affect all three SDK targets roughly equally and are out of scope for an emitter-level fix in this PR:

1. Multi-tenancy disabled on the default local broker causes cascading failures. docker/docker-compose.yml's broker has no multi-tenancy-enabling env var, but the scenario planner/domain-semantics unconditionally populates a tenantId field on createDeployment (and other tenant-scoped ops). Confirmed directly: POST /deployments with a tenantId field → 400 INVALID_ARGUMENT "multi-tenancy is disabled"; the same call without it → 200. This cascades into most createTenant→createDeployment-chained scenarios across every SDK target.
   • Suggested fix (3-phase plan):
     • Phase 1: add an opt-in docker/docker-compose.multi-tenancy.yml overlay (mirroring the existing docker-compose.rbac.yml pattern) that enables multi-tenancy, so it can be exercised without changing the default broker's behavior.
     • Phase 2: decide, as a repo-level choice, among (a) leave the default broker as-is and treat tenant-scoped scenarios as multi-tenancy-overlay-only, (b) enable multi-tenancy by default, or (c) have the scenario planner suppress tenantId injection per-config when multi-tenancy is known to be disabled.
     • Phase 3: measurement hygiene — any before/after comparison against a live broker must fully recreate it (docker compose down -v && up -d; a plain restart does not reliably clear the H2 in-memory store) and report clean before/after counts.
2. Scenario dependency-chain gaps (path-analyser, not emitter-specific): several unassign*/delete* RBAC operations chain straight from create* without the intermediate assign* step, producing legitimate 404s. Same root cause likely affects correlateMessage/broadcastSignal scenarios missing a preceding createDeployment+createProcessInstance step.
3. waitUpToMs: 0 hardcoded for js-sdk's eventually-consistent calls — defeats the SDK's own consistency-retry mechanism, a likely contributor to timing-race NOT_FOUNDs on plain create-then-get chains.
4. csharp-sdk: 2 operations (getGlobalJobStatistics, getUsageMetrics) need query-parameter modeling in path-analyser (not just a request-body object) to compile — same gap affects js-sdk's getUsageMetrics. Also several response-shape/field-path-extraction gaps and polymorphic/oneOf request-body types remain unresolved for a handful of operations.

None of these are regressions from this PR — they were present before this branch's work began and are documented in detail (with repro steps) for follow-up.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate generation, runtime, rendering, E2E, and regression-test issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (12)

materializer/src/csharp-sdk/emitter.ts:23

• The request-type table is still incomplete for body-bearing operations that are present in the C# operation map. For example, the planner emits updateUserTask (and the same applies to createAuthorization/createRole and the CRUD update operations), but none has an entry here; resolveRequestTypeName() then returns undefined and the non-empty body path throws No published C# request DTO mapping found during generation. Add the verified DTO mappings for every body operation before generating the full C# suite.

const CSHARP_REQUEST_TYPE_BY_OPERATION: Record<string, string> = {
  createDeployment: 'DeploymentRequest',
  createUser: 'UserRequest',
  createTenant: 'TenantCreateRequest',
  createGroup: 'GroupCreateRequest',
  createMappingRule: 'MappingRuleCreateRequest',

materializer/src/js-sdk/emitter.ts:181

• initSpecSalt() is still emitted at module scope, but every generated operation file imports the same support/seeding module whose _specSalt is global. Vitest loads multiple files before running their tests, so the last module imported overwrites the salt for all earlier suites; seed values then depend on import order and lose the per-operation isolation this helper is meant to provide. Emit the initialization inside each generated it body immediately before that scenario's seed calls, as the Python emitter now does.

  if (needsSeeding) {
    // Mixed into the seed so parallel vitest workers (sharing one TEST_SEED)
    // don't draw the same sequence for the same binding name.
    lines.push(`initSpecSalt(${JSON.stringify(operationId)});`);
    lines.push('');

materializer/src/js-sdk/emitter.ts:400

• RequestStep.extract.fieldPath is emitted from the OpenAPI response fields (for example, id), but the SDK call returns an envelope whose payload is under response.data. This assignment therefore reads response.id for generated plans, leaving prerequisite bindings undefined and breaking later path/body inputs. Normalize to response.data before applying the planner accessor, and only assign when the extracted value is not undefined so an absent field cannot erase an earlier binding.

        lines.push(`      ctx['${extract.bind}'] = ${responseVar}${accessor};`);

materializer/src/js-sdk/emitter.ts:452

• The witness predicate is applied to the raw SDK response wrapper, whose REST payload is under .data, while WitnessPredicate.path is the top-level wire-body field (for example, state). As written, the predicate reads response.state instead of response.data.state, so eventual waits retry until timeout even after the witness reaches the expected state.

    `            const v = (body as Record<string, unknown>)[${JSON.stringify(w.predicate.path)}];`,

materializer/src/js-sdk/sdk-mapping.ts:24

• Falsy but valid JSON body templates (false, 0, '', or null) are rendered as {}. Since the request plan permits arbitrary JSON values, this silently changes scalar payloads; use an undefined check for absence and preserve the literal value (and update the caller's truthiness check accordingly).

  if (!bodyTemplate) return '{}';

materializer/src/js-sdk/sdk-mapping.ts:32

• Only whole-string ${...} values are converted to context lookups here, while the planner recognizes embedded and multiple placeholders in a string. A body value like prefix-${nameVar}-${idVar} therefore remains a literal in generated JavaScript even though both bindings are collected and seeded, producing an incorrect request.

      const bindingMatch = value.match(/^\\?\$\{([^}]+)\}$/);
      if (bindingMatch) return `ctx['${bindingMatch[1]}']`;
      return JSON.stringify(value);

materializer/src/python-sdk/emitter.ts:142

• The Python emitter now keeps planner binding names in their original camelCase (ctx.get('tenantIdVar')), but this doc block still says placeholders become ctx.get('snake_var') and the example above uses id_var. That contradicts the generated code and the regression assertions, so contributors following the documentation can reintroduce the casing bug; update the example/docstring to use idVar/tenantIdVar (or change the implementation consistently).

      // ctx keys are the planner's original binding variable names (e.g.
      // tenantIdVar) — must match the ctx.set(...) calls emitted for
      // scenario.bindings verbatim, so no casing transform here (#354).
      return `ctx.get('${whole[1]}')`;

materializer/src/python-sdk/emitter.ts:138

• This matcher only substitutes a placeholder when it occupies the entire string. The planner collects embedded and multiple ${...} references, so values such as prefix-${nameVar}-${idVar} are seeded but emitted literally and reach the broker unresolved. Render all placeholder matches as a concatenation of literals and context values instead of limiting substitution to whole-string values.

    const whole = /^\$\{([^}]+)\}$/.exec(value);
    if (whole) {

materializer/src/python-sdk/emitter.ts:556

• This unconditionally stores get_nested_value(...) in the context. The helper returns None both for a missing response field and for a JSON null, so a missing optional field can erase a previously seeded or extracted binding and make a later request send null. Match the other emitters' extraction contract by distinguishing a missing path and leaving the existing binding untouched.

          `    ctx.set('${extract.bind}', get_nested_value(${responseDataVar}, '${extract.fieldPath}'))`,

materializer/src/python-sdk/emitter.ts:165

• The renderPythonValue call is preceded by a falsy check that treats valid scalar JSON bodies (false, 0, '', and null) as if no body existed, so this path emits {} instead. Use bodyTemplate === undefined as the no-body check and preserve all other JSON values.

  return renderPythonValue(bodyTemplate);

scripts/e2e/run-hub.sh:66

• By adding curl to this live-step condition, the script now supports curl-only execution, but the resource-fixture block below still runs only when step run is selected. STEPS=curl therefore reaches curl_compare.py without the valid keys its negative requests need and can report 404/403 instead of the expected validation responses. The fixture setup must also run for curl-only mode.
  tests/regression/generated-suites-typecheck.test.ts:59
• The PR changes the Python and C# emitters and their generated-project scaffolding, but this regression harness adds a typecheck only for js-sdk. A generated Python syntax/import or C# compile regression can therefore pass the repository test suite even though these are the real-toolchain paths this PR is fixing; add equivalent generated-output checks for both targets (or a separate CI-backed harness).

• Files reviewed: 35/37 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved critical JavaScript response-path issues and additional seed-binding and C# mapping defects block approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

materializer/src/js-sdk/sdk-mapping.ts:32

• seedBindings.ts deliberately recognizes placeholders embedded anywhere in a string and supports multiple placeholders, but this renderer only substitutes a whole-string match. A template such as proc-${processInstanceKeyVar}-${tenantIdVar} is emitted with the literal placeholder text, causing the generated request to send unresolved bindings instead of their seeded values; use an embedded replacement path here and add the same regression coverage as the Python emitter.

materializer/src/csharp-sdk/emitter.ts:104

• This key-type map is incomplete for the pinned 9.2.2 SDK: methods using {tenantId} take TenantId, and methods using {username} take Username, but both names fall through to RequireStringBinding below. Generated calls such as tenant membership operations then pass strings to strongly typed SDK parameters and fail to compile; add these value-object mappings (and audit the remaining SDK signatures) instead of treating every unmapped path parameter as a string.

  processInstanceKey: 'ProcessInstanceKey',
  resourceKey: 'ResourceKey',
  userTaskKey: 'UserTaskKey',
  variableKey: 'VariableKey',
};

materializer/src/js-sdk/emitter.ts:61

• emitCtxSeeding writes literal scenario bindings verbatim, so planner values such as proc_${RANDOM} are generated into the JS suite as a quoted literal instead of a runtime-generated identifier. The scenario generator marks these as placeholders consumed by the test runtime; route the RANDOM token through the generated seeding helper (and add a regression case) rather than passing it unchanged.

    bindings: scenario.bindings,
    seedBindings: scenario.seedBindings,
    globalContextSeeds,

materializer/src/python-sdk/emitter.ts:159

• scenarioGenerator emits literal bindings such as proc_${RANDOM} and jobType_${RANDOM} (path-analyser/src/scenarioGenerator.ts:443-449). This interpolation treats RANDOM as a missing context key and falls back to an empty string, producing proc_/jobType_; these literals should be expanded through seed_binding('RANDOM') (with the corresponding import) just as the C# emitter handles SeedBinding("RANDOM").

  for (const match of value.matchAll(EMBEDDED_PLACEHOLDER_RE)) {
    rendered += escapeLiteral(value.slice(lastIndex, match.index));
    rendered += `{ctx.get('${match[1]}') or ''}`;
    lastIndex = match.index + match[0].length;

• Files reviewed: 35/37 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain in token and placeholder rendering, generated-output validation, and specification-pin justification.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

configs/camunda-oca/spec-pin.json:4

• This changes the camunda-oca baseline to a different upstream spec commit and content hash, but the PR description presents only SDK-emitter fixes and does not document a spec bump or the resulting invariant/output changes. Because this changes the contract that all generated scenarios are validated against, please remove it from this emitter PR or explicitly justify the pin bump and include the required regenerated/invariant review.

  "specRef": "6ea2724e1e03f26508d07748b66a7ca54e9c595b",
  "expectedSpecHash": "sha256:798dfd63863ec3bfa266ccc46e0695082ec9aa1ed45923bd9fb0cb130d4f85db"

materializer/src/js-sdk/emitter.ts:406

• When the SDK call unexpectedly succeeds, expect.fail(...) throws inside this try and is immediately caught as if it were an HttpSdkError; the test then reports sdkError.status as undefined instead of the intended “expected N but request succeeded” failure. Move the explicit failure outside the try/catch (or return after handling the SDK error) so unexpected success is not misclassified.

  if (isErrorExpected) {
    lines.push('      try {');
    lines.push(
      `        const ${responseVar} = await call${stepNum}(input${stepNum}, consistency${stepNum});`,
    );
    lines.push(`        expect.fail('Expected ${expectedStatus} but request succeeded');`);
    lines.push('      } catch (error) {');
    lines.push('        const sdkError = error as HttpSdkError;');
    lines.push(`        expect(sdkError.status).toBe(${expectedStatus});`);
    lines.push('      }');

materializer/src/python-sdk/emitter.ts:164

• The embedded-placeholder branch has the same ${RANDOM} problem: proc_${RANDOM} becomes an f-string expression reading an absent RANDOM context key and therefore loses the process/job type suffix. Preserve this token as escaped literal f-string text rather than evaluating it as a binding.

  for (const match of value.matchAll(EMBEDDED_PLACEHOLDER_RE)) {
    rendered += escapeLiteral(value.slice(lastIndex, match.index));
    // `is not None`, not `or`: a falsy-but-bound value like 0 or False must
    // not collapse to '' (Copilot PR #574 review, same class as the
    // path-param fix in buildPythonUrlExpression above).
    rendered += `{ctx.get('${match[1]}') if ctx.get('${match[1]}') is not None else ''}`;

scripts/e2e/run-hub.sh:72

• This new comment is grammatically incomplete and leaves the inline code span unterminated, so it does not clearly document what header is intentionally omitted.
  tests/regression/generated-suites-typecheck.test.ts:59
• This test now assumes every active config emits a JS SDK suite, but configs/camunda-hub/codegen/emitters.json:1-3 enables only Playwright. With CONFIG=camunda-hub, getSdkOutDir(..., 'js-sdk')/tsconfig.json is absent and this test throws before checking any suite. Gate this entry on the active config's enabled-emitter registry, consistent with the config-driven target layout.
  tests/regression/generated-suites-typecheck.test.ts:59
• This new generated-suite guard covers only TypeScript. The PR also changes Python request rendering and several C# DTO/path/return-type mappings, where emitter string tests can pass while the emitted project still fails poetry/dotnet compilation. Add equivalent generated Python syntax/type and C# build checks (or an explicitly scoped CI job) so those real-toolchain regressions are guarded rather than validated only manually.

• Files reviewed: 35/37 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain around generated literal escaping, falsy request bodies, fixture resolution, and exact status validation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (10)

Previously missed (1) — in code that hasn't changed since the last review.

materializer/src/python-sdk/emitter.ts:228

• The scalar JSON branch calls renderPythonBody for false, 0, and '', but the preceding truthiness check converts all of them to {}. RequestStep.bodyTemplate is unknown and the C# emitter already handles scalar bodies, so this drops the documented request payload for valid primitive schemas; only an absent (undefined) template should take the no-body path.

 * Substitutes placeholders like "${varName}" with ctx.get('var_name').

materializer/src/csharp-sdk/emitter.ts:564

• The new bare-Task branch treats any non-throwing completion as success, but these SDK methods only enforce the broad 2xx range and expose no response status. Consequently a step expecting one exact status (for example 204) cannot detect a different successful status. Use a status-returning SDK path for these operations or explicitly model that exact status is unavailable instead of silently passing.

    // to compile with CS0815. A successful (non-throwing) completion is the
    // pass condition for these calls, so there is nothing to assert or
    // extract from.

materializer/src/csharp-sdk/emitter.ts:601

• This generated assertion claims to check the planner's exact status, but the installed SDK methods return DTOs after EnsureSuccessStatusCode() and do not expose the HTTP response. AssertExpectedStatus therefore falls back to accepting any 2xx, so an expected 201/204 can pass after an unexpected 200/202. The call must use a status-aware SDK response (or the exact-status limitation must be modeled explicitly).

    body.push(`        AssertExpectedStatus(${varName}, ${step.expect.status});`);

materializer/src/js-sdk/emitter.ts:415

• The successful branch forwards the request but never checks step.expect.status. Because the loose SDK client only throws for non-2xx responses, an expected 201/204 can silently pass when the server returns a different 2xx status. Emit an exact-status assertion using a status-aware SDK response before recording extracts.

      `      const ${responseVar} = await call${stepNum}(input${stepNum}, consistency${stepNum});`,

materializer/src/js-sdk/sdk-mapping.ts:24

• This helper is also used for each multipart field value, so a valid falsy scalar such as false or 0 is rendered as {} instead of the scalar. Because request templates are typed as unknown and the sibling emitters support scalar values, use an undefined-only no-body check here (and preserve null as null) so multipart fields and scalar JSON bodies retain their wire type.

  if (!bodyTemplate) return '{}';

materializer/src/python-sdk/emitter.ts:186

• The newly added embedded-placeholder branch interpolates varName directly into a single-quoted Python string literal. A spec-derived binding containing ', \, or a newline makes the generated test invalid or allows source injection; render the key with renderPythonStringLiteral(varName) in both ctx.get(...) calls.

        parts.push(renderPythonStringLiteral(literalBuffer));

materializer/src/python-sdk/emitter.ts:255

• fieldName is another spec-derived value inserted into a single-quoted Python literal without escaping. The new dictionary-assignment path will emit invalid source for a property such as owner'sName; use renderPythonStringLiteral(fieldName) here, consistently with the multipart fix.

  const deferred: { fieldName: string; binding: string }[] = [];

materializer/src/python-sdk/emitter.ts:262

• This deferred branch has the same code-generation bug as the inline branch: fieldName is interpolated into a Python string literal without escaping. When an omit-when-unbound field has a non-identifier name, the emitted suite can fail to parse or assign the wrong key.

    inlineEntries.push(`'${fieldName}': ${renderPythonValue(fieldValue)}`);

materializer/src/python-sdk/materialize-support.ts:251

• The generated project vendors its fixtures specifically so resolution is independent of cwd, but these two candidates search the caller's working directory before the vendored/config fixture roots. Running pytest from the repository (or another directory containing the same relative name) can therefore silently load an unrelated file instead of the fixture selected by the generator. Remove the direct relative-path lookups and resolve only from the generated/configured fixture roots.

    candidates = [
        Path(relative_path),
        Path.cwd() / relative_path,
        Path.cwd() / 'fixtures' / relative_path,

tests/codegen/js-sdk-emitter.test.ts:319

• This assertion locks in a missing status contract: the planner supplies expect.status, but a successful loose-client call can return 200 for an expected 201/204 without throwing. Generated JS suites therefore accept the wrong successful status and continue to extract data. Remove the negative assertion and make the generated success path verify the exact status (or use an SDK API that exposes it).

• Files reviewed: 35/37 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

One critical fixture-path safety issue and three moderate request-handling issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

materializer/src/js-sdk/emitter.ts:473

• The loose client returns a response wrapper whose payload is under data (the emitter already emits response?.data for extraction), but this new witness predicate reads state/other planner paths directly from the wrapper. Every eventual wait therefore evaluates undefined === expected and times out even after the API returns the desired payload. Apply the predicate to body.data (or unwrap the response before passing it to awaitEventually).

    `            const v = (body as Record<string, unknown>)[${JSON.stringify(w.predicate.path)}];`,

materializer/src/python-sdk/emitter.ts:235

• The body renderer's falsy guard above treats every falsy template as absent. Since this path now intentionally renders any non-undefined request body, valid top-level JSON bodies such as false, 0, '', or null reach this return but are emitted as {} instead of their planned value. Change the guard to an undefined check and let renderPythonValue handle scalar/null bodies.

  return renderPythonValue(bodyTemplate);

• Files reviewed: 35/37 changed files
• Comments generated: 1
• Review effort level: Lite