feat(models): expose cel-js Environment to extensions via ctx.createCelEnvironment()#1406
Conversation
…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>
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 buildMethodContext → RawExecutionDriver → execute chain all demonstrate careful engineering.
Blocking Issues
None.
Suggestions
-
ReDoS note in docs (
.claude/skills/swamp-extension/references/model/api.md): ThematchesRegexexample usesnew RegExp(pattern).test(value)with user-suppliedpattern. 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. -
execution_service.tsdomain→infrastructure import: ThecreateExtensionCelEnvironmentimport atsrc/domain/workflows/execution_service.ts:89sits alongside the pre-existingCelEvaluatorinfrastructure import, so this isn't a new violation — just noting it as a candidate if the DDD ratchet is ever tightened. TheDefaultStepExecutorcould receive the factory via its constructor (similar to howbuildMethodContextgets it viaCommonMethodContextDeps) to fully invert the dependency, but that's cleanup for another day.
There was a problem hiding this comment.
Adversarial Review
Critical / High
None found.
Medium
-
packages/testing/_cel_environment.ts— mirrored code as a drift vector. The factory is copy-pasted fromsrc/infrastructure/cel/cel_evaluator.tsbecause 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. -
src/domain/workflows/execution_service.ts:89— importscreateExtensionCelEnvironmentdirectly from infrastructure. The PR description claims "No new domain→infrastructure violations (DDD ratchet stays at 25)" and this is technically accurate —execution_service.tsalready importsCelEvaluatorfrom 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
-
Number(bigint)precision loss in arithmetic overloads (cel_evaluator.ts:203-240,_cel_environment.ts:39-75). TheNumber(b)/Number(a)conversions in theint + double,int - double, etc. overloads silently lose precision for bigint values outsideNumber.MAX_SAFE_INTEGER. This mirrors the pre-existing behavior inCelEvaluator(same operators, same conversions), and the standalonecoerceBigIntsfunction 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. -
Division-by-zero in
double / intandint / doubleoverloads. Whenbis0nor0, the division operators returnInfinityorNaN— standard JS behavior, not a crash. Again, pre-existing behavior carried forward from the oldCelEvaluatorconstructor.
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 buildMethodContext → RawExecutionDriver → execute path.
Summary
Exposes the cel-js
Environmentto extension model methods so they can evaluate their own CEL expressions over data they already hold — e.g. an SSH fleet model with aselectmethod that takes a CEL selector (tags.role == "web" && region.startsWith("us-")) and filters hosts by it.createExtensionCelEnvironment()insrc/infrastructure/cel/cel_evaluator.ts— returns a fresh, isolated cel-jsEnvironmentseeded 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.MethodContextasctx.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-testingmirrors the factory locally (packages/testing/_cel_environment.ts) — it publishes standalone to JSR and cannot import fromsrc/. A parity + isolation drift test runs both implementations and fails on divergence.createModelTestContextwiresctx.createCelEnvironment()so extension unit tests work with no extra setup.Environmenttype via inference (the common case) orReturnType<typeof ctx.createCelEnvironment>; no extension-author-facing package re-exports the type, matching howMethodContextitself is consumed today.Scope is intentionally limited to model methods — reports, datastores, vaults, and drivers do not get
createCelEnvironmentin this change.Resolves swamp-club#376
Test Plan
deno check— passesdeno lint— 1329 files, no issuesdeno fmt --check— cleandeno run test— 5996 passed, 0 failed, 29 ignoreddeno run compile— binary rebuilt successfullyunlistedVariablesAreDyn, custom function registration, fresh-instance isolation, namespace-type exclusionsrc/andpackages/testing/factory implementationscreateModelTestContextwiring testintegration/method_context_cel_env_test.ts) driving a synthetic extension through the productionbuildMethodContextpathCelEvaluatorregression tests pass unchanged🤖 Generated with Claude Code