Commit graph

15 commits

Author SHA1 Message Date
Marko Lahma 7328a0f14d
Adopt the Jint 4.15 host-integration surface: lazy type globals, enum names, and register-what-is-referenced (#7895)
* chore(javascript): update Jint to 4.15.3 and stop blocking on promises

`Engine.Evaluate(...).UnwrapIfPromise()` blocks the calling thread while the
engine's event loop drains, which is exactly the wrong thing to do inside an
`async` method — an expression that awaits a .NET `Task`, such as one calling
`getSecret()`, held a thread pool thread for the duration of the I/O.
`Engine.EvaluateAsync` awaits the returned promise instead, and takes the
cancellation token while it is at it.

The Jint version is moved from 4.4.2 to 4.15.3. `EvaluateAsync` arrived in
4.14.0, but the pin lands past 4.15.2 deliberately: once expressions genuinely
suspend and resume instead of draining the event loop on the calling thread,
they exercise the async suspension machinery 4.15.2 corrected — an `await` on a
right-hand side no longer stores the suspension sentinel, async generators and
`for await...of` preserve loop iteration state across a suspension, and a
suspension node is unwrapped correctly. Shipping the non-blocking change on an
earlier 4.14/4.15 would enable exactly the code paths those releases fixed.

One default changed along the way: since 4.14 `Interop.ArrayConversion` defaults
to `LiveView`, so a CLR array reaches script as a live view over the original
array rather than as a copy. That is observable — a script that sorts an array
would now reorder the workflow's own array, and the value round-trips back as
its original element type rather than as `object[]`. The evaluator therefore
pins the previous `Copy` behaviour so the upgrade is not a behavioural change;
hosts that prefer the live view can opt in through
`JintOptions.ConfigureEngineOptions`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179sA2T7HuRfRfSc2JirFik

* test(javascript): pin the array copy lane and adopt JsString.Create

The array audit. The parent commit fixes `ArrayConversion` to `Copy`, because
4.14 changed the default to `LiveView` and the two differ in behaviour a script
can observe. The existing tests assert what a copy *produces*, which a live view
also satisfies for a value nothing mutates; asking the engine how many
conversions of each kind it performed (4.15.1's interop conversion counters)
pins the lane itself. The second assertion is the more interesting one: an
ordinary evaluation converts no CLR array at all, because Elsa converts
collection-valued variables itself in `ObjectConverterHelper` long before Jint's
array lane could see them. That makes the `ArrayConversion` setting a narrow
compatibility pin rather than something every evaluation depends on.

`JsString.Create` (public since Jint 4.15.3) is adopted in
`JsonElementConverter`, where the string case was the only one still routed
through `JsValue.FromObject` — re-entering the whole conversion pipeline, the
registered object converters and this one included, to arrive at the same call
the number and boolean cases beside it already make directly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f

* perf(javascript): register .NET type globals lazily

A fresh Jint engine is built for every expression evaluation, and roughly twenty
.NET types are registered on it before the expression runs: the common types
(`DateTime`, `Guid`, `TimeSpan`, …) plus every non-primitive workflow variable
descriptor type. Each registration builds a `TypeReference`, which describes the
type through reflection. The overwhelming majority of expressions reference none
of them.

`Options.AddLazyGlobal` installs a global whose value is produced by a factory
the first time a script reads the name. Both type registration handlers now run
on `CreatingJavaScriptEngine` — which already carries the `Jint.Options` being
built — and register through it, so a type is only described if an expression
actually mentions it. A script that uses `Guid.NewGuid()` still sees `Guid`; a
script that uses none of them pays for none of them.

Types whose name cannot be written as a JavaScript identifier are skipped while
we are here, since no script can reach them. That covers constructed generic
types (``IDictionary`2``, which two different variable descriptors both claimed)
and array types (`Byte[]`).

Moving the registrations to engine construction has a consequence worth pinning
beyond the ordering: a global installed by the host through `configureEngine` is
no longer overwritten by the built-in registration of the same name. The lazy
global for `Guid` is already installed when that callback runs, and
`Engine.SetValue` goes through `[[Set]]` on the global object, which reads the
current value — running the factory once and discarding the result — before
replacing the descriptor. The end state is the host value, but only the test
says so.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f

* perf(javascript): register the common functions lazily

The same argument as the type globals, applied to the other bulk registration a
fresh engine pays for. `ConfigureEngineWithCommonFunctions` installs
twenty-seven functions on every engine — `getVariable`, `toJson`, `newGuid`, the
base64 helpers, the deprecated GUID pair — and each `engine.SetValue(name,
Delegate)` builds an interop function wrapper around the delegate: a JavaScript
function object, plus the signature metadata and invoker lookups Jint resolves
per delegate type. An expression such as `variables.Foo` or a comparison calls
none of them.

This was recorded in the PR as deferred, because a lazy version had to keep the
`NonEnumerable` flag `SetValue(string, Delegate)` applies — otherwise the
functions would start appearing in `Object.keys(globalThis)`, which is
observable. Jint 4.15.3's `Engine.Advanced.AddLazyGlobal` takes a `PropertyFlag`
and is the post-construction counterpart of the options-time API used for the
type globals, so the flag is passed explicitly and the port is a line per
function. The CLR delegate is now created inside the factory as well, so a
function nothing reads costs one closure rather than a closure, a delegate and a
wrapper.

The laziness is invisible, and the tests say so rather than leaving it implied.
`AddLazyGlobal` installs the property itself eagerly and defers only its value,
so `in`, `hasOwnProperty` and `Object.getOwnPropertyNames` answer immediately
without materialising anything; `Object.keys(globalThis)` still omits them; the
descriptor still reports writable, non-enumerable and configurable; two reads of
a function are the same value, which is what says the factory ran once and the
result was stored rather than recomputed per read; and a script can still
overwrite one.

Not separately measured. The measured table in the PR predates this commit and
was not re-run for it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f

* refactor(javascript): let Jint expose enums as names and type its converters

`EnumToStringConverter` turned every CLR enum crossing into JavaScript into
`Enum.ToString()`. Jint 4.15 does that with `Interop.EnumConversion`, so the
converter goes away.

It is not only fewer moving parts. The converter could only see values that
crossed the interop boundary, and a constant read off a registered enum type
does not: `LogPersistenceMode.Include` came back as the underlying number while
the same value held in a workflow variable came back as `"Include"`, so
`mode === LogPersistenceMode.Include` was always false. The built-in switch
covers both directions and they now agree. Values written back to the CLR keep
accepting the member name and the number, as they did before.

That fix changes what an expression using a constant *numerically* produces, and
the hazard worth calling out is persisted state: a workflow variable holding a
number written from a constant before the upgrade no longer compares equal to
that constant after it, which reaches in-flight and resumed instances rather
than only new ones. Typed conversions hold in both directions, so activity
inputs and typed variable reads are unaffected. Recorded in the 3.8.0 changelog.

The two remaining converters register through the overload that declares the CLR
types they handle. A converter that does not declare them has to be offered
every value crossing the boundary, which costs the engine its compiled
member-read and method-invoker lanes for every wrapped .NET object; declaring
`byte[]` and `JsonElement` keeps those lanes for everything that cannot produce
one.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f

* perf(javascript): register only the accessors an expression names

Every evaluation registers a getter and a setter for each variable in scope, a
getter for each workflow input, and a getter for each (activity, output) pair in
the enclosing container. The last of those is the expensive one: naming those
accessors walks every node of the workflow and resolves each against the
activity registry, so the cost of setting up an engine grows with the size of
the workflow rather than the size of the expression. Almost no expression uses
any of them.

Jint reports the free identifiers of a prepared program through
`Prepared<T>.ReferencedGlobals`, which is exactly the question being asked here:
an identifier the expression never mentions cannot be read from it. The
evaluator now prepares the script before configuring the engine — the parse was
already cached, so this only reorders work — and passes the set on
`EvaluatingJavaScript`. The accessor handler registers a name only if it is in
the set, and skips the workflow walk entirely when no identifier of the
`get{Output}From{Activity}` shape appears.

The filter is sound for an expression that names an accessor the way one is
meant to be named, and not for one that builds or reaches a name at run time.
Four such forms are detectable and each turns the filter off. A direct `eval`
call is reported as `HasDirectEvalCall`. An *indirect* `eval` call and the
`Function` constructor are deliberately not flagged as direct calls — Jint
reports the identifiers `eval` and `Function` in the set instead, and says so,
because that is the signal a host is meant to act on. Missing that signal is not
theoretical: `new Function('return getMyVariable()')()` would regress from
working to a `ReferenceError`, since Function-constructed code resolves only
against the global scope, and `var e = eval; e('getMyVariable()')` would fail the
same way. The fourth is a reference to `globalThis`, which reaches a global
without naming it. All four are pinned.

What stays undetectable is reaching the global object without naming it at all —
a sloppy-mode top-level `this`, or `[].constructor.constructor(…)`. An
expression written that way loses the generated accessor but not the data:
`getVariable(name)`, `getInput(name)` and `getOutputFrom(activityId, outputName)`
are always registered and reach the same values.

Note this does not replace the regular expression in
`ConfigureEngineWithVariables`, which extracts the member names in
`variables.Foo`. Those are not free identifiers and Jint deliberately does not
report them; the set only says whether `variables` itself is referenced.

The filtering is invisible from inside a script, which also makes it unprovable
from there, so two of the tests hold on to the engine and assert on the globals
directly.

While here, the cancellation token is registered as an engine constraint.
Passing it to `EvaluateAsync` only covers the awaiting part; Jint's own remarks
say the parameter cannot preempt the synchronous evaluation loop and point at
this constraint, so the token read as more coverage than it delivered. A
cancellation constraint is amortizable, so the interpreter keeps its tight-loop
fast path, and a default token registers nothing. #7891 supersedes this with the
full constraint set.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f

* perf(javascript): build marshalled variables in Jint's shaped representation

`ConvertToJsObject` built the JavaScript object a workflow variable is copied
into one `DefineOwnProperty` call at a time, with an explicit descriptor. That
lands the object in Jint's per-object property dictionary, so every marshalled
variable carries its own descriptors even though sibling variables and the
repeated nested payloads of one document present the same key set.

`JsObject.CreateFromEntries` defines the same writable, enumerable and
configurable properties but builds through the shared-layout path. Both wins
land inside a single evaluation — half the descriptor allocations, since the
explicit descriptor caused a second one inside `ValidateAndApplyPropertyDescriptor`,
and one shared layout across objects presenting the same keys. Nothing carries
across evaluations: layouts are interned per engine and per-node inline caches
live in per-engine handler trees that only engage on a second evaluation on the
same engine, which a fresh-engine-per-evaluation host never reaches.

Reaching the shared layout is silent: `CreateFromEntries` falls back to the
ordinary property dictionary whenever a key or a growth guard says the layout
cannot continue, and the object behaves identically either way. A test now asks
the engine whether the object actually got one. `Engine.Advanced.HasSharedShape`,
added in 4.15.3, is the part of that answer Jint documents as a contract — the
finer-grained `GetObjectRepresentation` names an internal representation that may
be renamed or subdivided in any release — and `CreateFromEntries` is one of its
three documented success cases. The assertion is not vacuous: against the
previous property-by-property build it is false, including for the
`CreateDataProperty` variant in #7892, because that object is created through
`Intrinsics.Object.Construct` rather than built as entries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f

* test(javascript): run the scripting suites with Jint's host-contract verifiers on

Jint has a set of verifiers for the extension points it *trusts* — the ones it
cannot afford to re-check on a hot path, so a violation is otherwise silent.
They used to be compiled out of Release, which made "run your suite against a
Debug Jint" the only way to reach them, and Jint ships Release-only. 4.15.3
moves them behind an AppContext switch read once at type initialization, so a
host that never sets it pays nothing (the JIT folds the guards away) and a test
host can turn them on against the shipped package.

The one Elsa is subject to today is the object-converter type declaration this
PR introduced. Registering a converter with `AddObjectConverter(converter,
handledTypes)` promises the engine the converter produces values only for those
types, and in exchange the compiled interop lanes are kept for every member that
cannot produce one. Nothing links that promise to the converter's own
`TryConvert` switch: a case added there and not added to the registration is
silently skipped on exactly the members the declaration excluded, and honoured
everywhere else. `ByteArrayConverter` and `JsonElementConverter` are consistent
today; this is what would report it if they drifted.

Elsa defines no `ObjectInstance` subclass, so the rest of the verifiers have
nothing to check here yet. They are a standing guard for the day a host handler
or a satellite module adds one.

Wired as a module initializer, because the switch has to be set before the first
use of any Jint type. Duplicated across the three suites that reference
`Elsa.Expressions.JavaScript` rather than shared: `Elsa.Testing.Shared.Integration`
would be the obvious home, but it is a published package, and a module
initializer there would flip a process-wide switch for every external consumer
of it as well. A one-line test pins that the initializer ran, since Elsa
satisfies the contracts it is subject to and nothing else would notice the
checks going away.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Sipke Schoorstra <sipkeschoorstra@outlook.com>
2026-08-17 01:44:03 +02:00
Sipke Schoorstra 2e712d367a
Add OpenTelemetry workflow instrumentation (#7514)
* Add OpenTelemetry workflow instrumentation

* Fix workflow telemetry metric tags

* Tighten telemetry test listeners

* Refine workflow telemetry boundaries

* Address telemetry review feedback

* Complete workflow telemetry coverage

* Address telemetry instrumentation review feedback

* Handle cancelled workflow telemetry

* Refine workflow activity telemetry tags

* Address telemetry review feedback

* Document OpenTelemetry extension coexistence

Agent-Logs-Url: https://github.com/elsa-workflows/elsa-core/sessions/33211c71-c3c9-424c-b7eb-a13ebd4713a3

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Address telemetry PR review comments

* Address telemetry review follow-ups

* Preserve workflow executing status transition order

* Address telemetry review feedback

* Refine workflow telemetry review fixes

* Address telemetry review feedback

* Address workflow instrumentation review feedback

* Fix faulted workflow telemetry tags

* Restrict workflow exception mutation

* Fix canceled activity telemetry status

* Clarify workflow exception access

* Cover HTTP trace context propagation

* Report cancelled workflow telemetry consistently

* Refine telemetry cancellation classification

* Record thrown workflow exceptions on context

* Tighten workflow telemetry exception handling

* Preserve first workflow exception

* Handle workflow cancellation separately

* Clarify workflow telemetry enum references

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
2026-05-22 01:17:05 +02:00
Sipke Schoorstra 541218a37f
Add ingress rate limiting hooks (#7512)
* Add ingress rate limiting hooks

* Fix ingress rate limiting middleware setup

* Harden rate limiter policy validation

* Preserve routed endpoints during rate limiting

* Address rate limiting review feedback

* Address rate limiting Copilot feedback

* Register rate limiter services for external policies

* Address rate limiting review comments

* Keep rate limiter service detection best effort

* Address rate limiting review comments

* Remove brittle rate limiter validation

* Address rate limiting review feedback

* Address rate limiting nullable review

* Address rate limiting review feedback

* Assign ingress rate limit policies when enabled

* Refine ingress rate limiting middleware cleanup

* Address rate limiting review feedback

* Align rate limiting review feedback

* Clarify rate limiting policy semantics

* Stabilize rate limiting exception tests

* Fix rate limiting endpoint matching default
2026-05-22 00:13:11 +02:00
Sipke Schoorstra e7dc936c67
Merge remote-tracking branch 'origin/main' into codex/security-identity-secret-hashing
# Conflicts:
#	doc/changelogs/3.6.0.md
#	src/modules/Elsa.Identity/README.md
2026-05-20 23:04:02 +02:00
Sipke Schoorstra e9d59bc5b1
[codex] Fail fast on default JWT signing keys (#7496)
* Fail fast on default JWT signing keys

* Address JWT signing key review feedback

* Refine JWT signing key validation feedback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Reject JWT signing keys with surrounding whitespace

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
2026-05-20 22:30:49 +02:00
Sipke Schoorstra e2587f74b5
Address identity validator compatibility feedback 2026-05-20 14:11:19 +02:00
Sipke Schoorstra 1da8709e2c
Distinguish refresh tokens from API access tokens (#7509)
* Separate access and refresh token use

* Address Greptile identity token feedback
2026-05-20 14:04:28 +02:00
Sipke Schoorstra 9415d220c0
Document identity validator constructor dependencies 2026-05-20 14:00:02 +02:00
Sipke Schoorstra 95640cbf7d
Address identity hasher review feedback 2026-05-20 13:47:25 +02:00
Sipke Schoorstra 304e990319
Harden identity secret generation and hashing 2026-05-20 13:20:46 +02:00
Sipke Schoorstra 3d8d3b7de2
Merge remote-tracking branch 'origin/release/3.6.1' 2026-04-20 15:08:10 +02:00
Sipke Schoorstra 1d3c00a01f
Update print statement to say 'Goodbye World' 2026-03-14 10:47:01 +01:00
Sipke Schoorstra 8be9b24f2a
Revise changelog for version 3.6.0
Updated breaking changes and upgrade notes for version 3.6.0, including package name changes, database migration requirements, and multitenancy ID conventions.
2026-03-10 12:44:18 +01:00
Copilot 7d32d95932
Add release notes for Elsa 3.6.0 (#7348)
* Initial plan

* Add release notes for Elsa 3.6.0

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
Co-authored-by: Sipke Schoorstra <sipkeschoorstra@outlook.com>
2026-03-05 13:57:21 +01:00
Copilot 9d1bf99950
doc: add Elsa 3.6.0 release notes as doc/changelogs/3.6.0.md (#7347)
* Initial plan

* doc: add 3.6.0 release notes to doc/changelogs/3.6.0.md

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
2026-03-05 13:41:40 +01:00