Skip to content

feat(models): expose cel-js Environment to extensions via ctx.createCelEnvironment()#1406

Merged
stack72 merged 1 commit into
mainfrom
issue-376-cel-environment
May 19, 2026
Merged

feat(models): expose cel-js Environment to extensions via ctx.createCelEnvironment()#1406
stack72 merged 1 commit into
mainfrom
issue-376-cel-environment

Conversation

@adamhjk
Copy link
Copy Markdown
Contributor

@adamhjk adamhjk commented May 19, 2026

Summary

Exposes the cel-js Environment to extension model methods so they can evaluate their own CEL expressions over data they already hold — e.g. an SSH fleet model with a select method that takes a CEL selector (tags.role == "web" && region.startsWith("us-")) and filters hosts by it.

  • New createExtensionCelEnvironment() in src/infrastructure/cel/cel_evaluator.ts — returns a fresh, isolated cel-js Environment seeded with swamp's baseline arithmetic-overload registrations and { unlistedVariablesAreDyn: true }. It omits swamp's internal namespace types (file.contents, data.latest, …) so extensions get a clean baseline.
  • CelEvaluator's constructor now builds on the factory and layers the internal namespace types on top — no behavior change to swamp's internal CEL surface.
  • Exposed on MethodContext as ctx.createCelEnvironment(), plumbed via dependency inversion (CommonMethodContextDeps / CheckValidationContext) so the domain layer never imports the cel-js binding directly; application-layer callers (libswamp / CLI) supply the implementation. No new domain→infrastructure violations (DDD ratchet stays at 25).
  • @systeminit/swamp-testing mirrors the factory locally (packages/testing/_cel_environment.ts) — it publishes standalone to JSR and cannot import from src/. A parity + isolation drift test runs both implementations and fails on divergence. createModelTestContext wires ctx.createCelEnvironment() so extension unit tests work with no extra setup.
  • Extensions consume the Environment type via inference (the common case) or ReturnType<typeof ctx.createCelEnvironment>; no extension-author-facing package re-exports the type, matching how MethodContext itself is consumed today.

Scope is intentionally limited to model methods — reports, datastores, vaults, and drivers do not get createCelEnvironment in this change.

Resolves swamp-club#376

Test Plan

  • deno check — passes
  • deno lint — 1329 files, no issues
  • deno fmt --check — clean
  • deno run test — 5996 passed, 0 failed, 29 ignored
  • deno run compile — binary rebuilt successfully
  • New unit tests: factory arithmetic overloads, unlistedVariablesAreDyn, custom function registration, fresh-instance isolation, namespace-type exclusion
  • New drift + isolation test comparing the src/ and packages/testing/ factory implementations
  • New createModelTestContext wiring test
  • New integration test (integration/method_context_cel_env_test.ts) driving a synthetic extension through the production buildMethodContext path
  • Existing CelEvaluator regression tests pass unchanged

🤖 Generated with Claude Code

…elEnvironment()

Adds createExtensionCelEnvironment() in src/infrastructure/cel/cel_evaluator.ts
and exposes it on MethodContext as ctx.createCelEnvironment(), so extension
model methods can evaluate their own CEL expressions over data they hold
(e.g. selector predicates over a fleet of hosts). The factory returns a fresh,
isolated cel-js Environment seeded with swamp's baseline arithmetic-overload
registrations and { unlistedVariablesAreDyn: true }; it omits swamp's internal
namespace types so extensions get a clean baseline to register their own
functions, types, and operators on.

CelEvaluator's constructor now builds on the factory and layers the namespace
types on top. The factory is plumbed to MethodContext via dependency inversion
(CommonMethodContextDeps / CheckValidationContext) so the domain layer never
imports the cel-js binding directly — application-layer callers (libswamp / CLI)
supply the implementation. The testing package mirrors the factory locally
(packages/testing/_cel_environment.ts) because it publishes standalone to JSR
and cannot import from src/; a parity + isolation drift test guards the two
implementations against divergence.

Extensions consume the Environment type via inference (the common case) or
ReturnType<typeof ctx.createCelEnvironment> — no extension-author-facing package
re-exports the type, matching how MethodContext itself is consumed today.

Resolves swamp-club#376

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — This PR does not change any user-visible CLI surface. All changes are internal: the CEL factory refactor, dependency-inversion plumbing through MethodContext/CommonMethodContextDeps/CheckValidationContext, test infrastructure updates, and developer-facing documentation (skill reference, design doc, testing package README). No new commands, flags, output formats, error messages, or help text are introduced. Extension authors gain ctx.createCelEnvironment() as a programmatic API, but that has no CLI UX impact.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Well-structured feature. The dependency inversion via CommonMethodContextDeps keeps the domain layer free of runtime cel-js imports while the import type { Environment } in model.ts follows the same pattern as the existing CloudControlClient and Logger type-only imports on adjacent lines. Factory-per-call isolation, the mirrored testing-package implementation guarded by a drift test, and the integration test through the full buildMethodContextRawExecutionDriver → execute chain all demonstrate careful engineering.

Blocking Issues

None.

Suggestions

  1. ReDoS note in docs (.claude/skills/swamp-extension/references/model/api.md): The matchesRegex example uses new RegExp(pattern).test(value) with user-supplied pattern. For extension authors working with untrusted input, a brief caveat about ReDoS (or suggesting a bounded regex library) could save someone a production incident. Not blocking since this is example code and extensions are trusted, but worth considering.

  2. execution_service.ts domain→infrastructure import: The createExtensionCelEnvironment import at src/domain/workflows/execution_service.ts:89 sits alongside the pre-existing CelEvaluator infrastructure import, so this isn't a new violation — just noting it as a candidate if the DDD ratchet is ever tightened. The DefaultStepExecutor could receive the factory via its constructor (similar to how buildMethodContext gets it via CommonMethodContextDeps) to fully invert the dependency, but that's cleanup for another day.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial Review

Critical / High

None found.

Medium

  1. packages/testing/_cel_environment.ts — mirrored code as a drift vector. The factory is copy-pasted from src/infrastructure/cel/cel_evaluator.ts because the testing package publishes standalone to JSR. While there is a parity test (testing_package_compat_test.ts) that runs both factories against a representative set of expressions, the test set is finite. A future change that adds a new operator overload to only one copy would pass the existing parity expressions while producing silently different behavior for the untested operator. The drift guard is solid for what it tests — just be aware that new registrations need to be added to the parity expressions too, or the guard is blind to the delta.

  2. src/domain/workflows/execution_service.ts:89 — imports createExtensionCelEnvironment directly from infrastructure. The PR description claims "No new domain→infrastructure violations (DDD ratchet stays at 25)" and this is technically accurate — execution_service.ts already imports CelEvaluator from the same infrastructure module. Not a new violation, but worth noting: the existing pattern of the workflow execution service importing from infrastructure is the pre-existing issue, and this PR extends it.

Low

  1. Number(bigint) precision loss in arithmetic overloads (cel_evaluator.ts:203-240, _cel_environment.ts:39-75). The Number(b) / Number(a) conversions in the int + double, int - double, etc. overloads silently lose precision for bigint values outside Number.MAX_SAFE_INTEGER. This mirrors the pre-existing behavior in CelEvaluator (same operators, same conversions), and the standalone coerceBigInts function in the same file already documents this boundary. Unlikely in practice for the extension use-case (CEL integer literals from user expressions), but worth noting for completeness.

  2. Division-by-zero in double / int and int / double overloads. When b is 0n or 0, the division operators return Infinity or NaN — standard JS behavior, not a crash. Again, pre-existing behavior carried forward from the old CelEvaluator constructor.

Verdict

PASS. This is a clean, well-structured change. The factory extraction from CelEvaluator's constructor is behaviorally equivalent (same new Environment(...) + same operator registrations, with namespace types layered on top afterward — exactly matching the old order). The dependency injection through CommonMethodContextDeps / CheckValidationContext keeps the domain layer clean. All three production buildMethodContext call sites (run.ts, execution_service.ts, validation_service.ts) are correctly wired. The createCelEnvironment field is required (not optional) in both CommonMethodContextDeps and MethodContext, so TypeScript catches any omission. The testing package's mirrored factory has a runtime drift guard. The cel-js version is pinned identically (7.6.1) in both deno.json files. Test coverage is thorough — isolation, parity, arithmetic, custom function registration, and an integration test through the full buildMethodContextRawExecutionDriverexecute path.

@stack72 stack72 merged commit 88ab39c into main May 19, 2026
21 of 22 checks passed
@stack72 stack72 deleted the issue-376-cel-environment branch May 19, 2026 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants