feat: accept and forward resolvedSymbolTable to the Rust session runtime

[seant-aws]

gated on OpenJobDescription/openjd-model-for-python#332

What was the problem/requirement? (What/Why)

The Rust session runtime cannot resolve {{ Job.Name }}, {{ Param.X }}, or step-level let bindings in EXPR templates because the worker never forwarded the resolvedSymbolTable that the service will provide. The _v1 session already accepts a resolved_symtab parameter on enter_environment and run_task — the worker just never passed it.

What was the solution? (How)

Whitelist resolvedSymbolTable on both StepDetails and EnvironmentDetails entity validators, store it on the dataclasses, and thread it through the action → Session → RuntimeABC chain:

• Rust runtime: parses the JSON into a SerializedSymbolTable via _reconstruct_serialized_symtab and forwards it to the _v1 session.
• Python runtime: accepts the kwarg and ignores it — the v0 session resolves EXPR natively.

The field is optional and absent from the service today. When absent, behavior is unchanged. When present, the Rust session can resolve symbols that previously failed silently.

exit_environment is not modified: openjd-sessions replays the enter-time resolved_symtab automatically.

What is the impact of this change?

None today — the field is not yet served. Once the service populates it, the Rust runtime gains resolution of Job.Name, Param.*, RawParam.*, and step-level let values in EXPR templates. The Python runtime is unaffected.

How was this change tested?

hatch run lint clean; full unit suite 3082 passed / 39 skipped.

New tests cover: entity validation (field accepted, non-string rejected), from_boto extraction (present vs absent), Rust runtime forwarding (present → parsed and forwarded, absent → not forwarded), and graceful degradation (malformed JSON → warning logged, proceeds without symtab).

Was this change documented?

No public API or configuration changes. The use of a private openjd API (_reconstruct_serialized_symtab) is called out in a code comment with the expected migration path.

Is this a breaking change?

No. The new parameters are keyword-only with None defaults on all signatures.

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

This adds a module-level import of SerializedSymbolTable from openjd.expr and starts passing a new resolved_symtab= kwarg to Session.enter_environment / Session.run_task, but pyproject.toml still pins openjd-sessions == 0.10.14 unchanged. Two things worth confirming before merge:

1. Does 0.10.14 actually export openjd.expr.SerializedSymbolTable with a from_json_str classmethod, and does its _v1 Session accept resolved_symtab? If either landed in a later release, this fails at import time (breaking the Rust runtime entirely, since the import is unconditional at module scope) or with TypeError: unexpected keyword argument on every env-enter / run-task.
2. The unit tests cannot catch this: mock_rust_session replaces OpenJDRustSession with a MagicMock, so it accepts any kwargs, and SerializedSymbolTable.from_json_str is patched. Only an e2e run against the real wheel would surface a signature mismatch — consider adding e2e coverage (or at least a real-SerializedSymbolTable parse test) alongside the pin bump.

[github-actions]

Swallowing all parse failures here is a risky default. If the service sent a pre-resolved symbol table, it presumably carries symbol values the worker cannot reconstruct locally; proceeding with resolved_symtab=None means the session resolves expression references from whatever it can derive on its own instead. The likely outcomes are (a) an obscure downstream unknown-symbol/resolution error whose real cause is this swallowed warning, or (b) worse, the action silently running with different values than the service intended.

Two suggestions:

• Consider letting the failure propagate (or raising a clear error) so the session action fails fast and visibly with an accurate cause, rather than degrading into an inconsistent resolution context.
• If graceful degradation really is intended, narrow the except Exception to the specific error from_json_str raises for malformed input. As written it also absorbs unrelated failures (e.g. a TypeError from a changed signature on a different openjd.expr version), turning a hard dependency mismatch into a silent per-action warning that quietly disables the feature in production.

[github-actions]

The Python (v0) runtime accepts resolved_symbol_table_json and drops it with no runtime signal. If the service ever sends resolvedSymbolTable because it contains values the worker cannot derive locally, a worker on the v0 runtime resolves those references from its own context instead — silently producing different values rather than failing. Worth at least a one-time logger.debug/warning when a non-None table is discarded, so this is diagnosable from logs instead of only from reading the source comment. (Same applies to the run_task override below.)

[github-actions]

EnvironmentDetails now carries resolved_symbol_table_json, and it is forwarded on env enter — but ExitEnvironmentAction (actions/exit_env.py:60) and Session.exit_environment are untouched, so the environment’s onExit script resolves its expressions without the pre-resolved table. If the Rust session does not persist the symtab from enter_environment for the lifetime of that environment, onEnter and onExit for the same environment will resolve references from different contexts — an asymmetry that will be hard to debug.

Worth confirming which is true: if the session caches it, a brief comment saying so would prevent a future reader from “fixing” this; if it does not, exit_environment needs the same threading.

[github-actions]

Separate from the control-flow concern above: logging the raw exception from from_json_str can leak symbol-table contents into the agent log. JSON parse errors commonly quote the offending input fragment, and this table holds resolved job/task parameter values, which for some queues include sensitive parameter values. Since the log line fires exactly when the payload is unexpected, the odds of echoing content are highest in precisely the case you least control.

Safer to log only the exception type and, if useful, the byte offset / payload length rather than %s of the exception.

[github-actions]

Minor efficiency: StepDetails is cached per step, and run_task is invoked once per task, so the same resolved_symbol_table_json string is re-parsed for every task in the session. For a step with many tasks and a large resolved table this is repeated work on the hot path. Parsing once and memoizing (e.g. lazily on StepDetails, or keyed by step id in the adapter) would avoid it.

[github-actions]

The worker now takes a fully-resolved symbol table off the wire and hands it straight into the session's expression evaluator, with no check on which symbols it carries. Worth thinking about the trust boundary here, because the resolution namespace is not all service-owned:

• Job.*, Param.*, Task.Param.* are service-derived — a pre-resolved table is the whole point.
• Session.WorkingDirectory, Session.HasPathMappingRules, and especially Task.File.<name> are worker/session-derived. Task.File.* in particular resolves to paths the session materializes itself under files_directory at run time; the service cannot know those values.

If the _v1 session gives the pre-resolved table precedence over its own locally-derived symbols, a table containing a Task.File.X or Session.WorkingDirectory entry would redirect a script's file references to an arbitrary path, and the resulting read/write happens as the job user. That turns a malformed-or-mistaken service payload (or anything that can influence it) into a path-redirection primitive, with no worker-side guard.

Two things worth pinning down before this ships:

1. What is the documented precedence when a symbol appears both in resolved_symtab and in the session's own context? If the session's local values win for session-scoped symbols, a comment saying so here would settle it.
2. If precedence is not guaranteed, consider filtering the parsed table to the symbol prefixes the service legitimately owns (or rejecting tables that contain Session.* / Task.File.*) rather than forwarding it verbatim. Defence-in-depth is cheap here since the parse step already exists.

[github-actions]

New wire field, but session_events.py is untouched — worth checking whether that is the intended outcome for the agent log.

LOGGING_ALLOW_LIST["deadline:BatchGetJobEntity"]["res_log_body"] (session_events.py:161) enumerates the loggable stepDetails / environmentDetails keys, and _get_loggable_parameters (session_events.py:206) replaces any key not in that map with *REDACTED*. Since resolvedSymbolTable was not added there, it currently logs as resolvedSymbolTable: "*REDACTED*" — which is the safe default, and correct given the table holds resolved job/task parameter values (the same reasoning the existing comments give for excluding template and jobDetails.parameters).

So no leak today. The ask is just to make it deliberate rather than incidental: adding an explicit "resolvedSymbolTable": False, # resolved parameter values; sensitive entry alongside the existing exclusions documents the decision and prevents a future "*": True-style change or allow-list refactor from silently opting this field in.

[github-actions]

Test-coverage gap that matters for the negative case: the run_task path has no equivalent of test_enter_environment_graceful_degradation_on_malformed_json.

Both call sites go through the same _parse_resolved_symtab helper, so the parsing logic is covered — but the forwarding of None after a parse failure is only asserted for enter_environment. Since run_task builds resolved_symtab in its own separate statement (rust.py:408) and passes it as its own kwarg (rust.py:416), a future edit that drops or reorders that on the run_task path would leave the suite green. A parallel test_run_task_graceful_degradation_on_malformed_json is a few lines and closes it.

Separately, worth adding the plain-None case for both paths: nothing asserts that resolved_symbol_table_json=None (the overwhelmingly common path today, before the service ships the field) results in resolved_symtab=None rather than, say, a stray call to from_json_str(None). The enter_environment delegation test at line 262 pins resolved_symtab=None, but only because the fixture never sets it — an explicit test would state the intent.

[github-actions]

Scope mismatch worth confirming: the resolved table arrives on stepDetails, but it is consumed once per task.

JobEntities caches the raw stepDetails payload in _entity_record_map keyed by step id (job_entities.py:132), and step_details() rebuilds StepDetails from that same cached dict on every call (job_entities.py:372). So all tasks in a step — including tasks dispatched in later UpdateWorkerSchedule cycles within the same session — share one resolved_symbol_table_json, sourced from whenever the step was first fetched.

That is fine if the service only ever puts step-and-above-scoped symbols in it (Job.*, Param.*). It is a correctness bug if the table can contain task-scoped entries such as Task.Param.<name>, since task_parameter_values on line 86 varies per task while the table beside it would not — task 2 would resolve {{Task.Param.Frame}} to task 1's value, silently, and the two sources would disagree.

Worth confirming which contract the service guarantees. If task-scoped symbols are excluded by design, a short comment here saying the table is step-scoped would protect the invariant; if they are not, the table needs to be per-task rather than carried on the cached step entity.

[github-actions]

I checked the upstream implementation, and the "graceful degradation" this helper promises only covers half the failure modes.

In openjd-model 0.11.4, SerializedSymbolTable.from_json_str validates only JSON well-formedness. From the binding’s own docstring (rust-bindings/src/expr/symbol_table.rs:283):

the contents are validated lazily: a well-formed JSON document whose entries are not valid symbol table entries is accepted here and rejected by to_symtab.

So there are two distinct malformed-payload classes, and this try only catches one:

1. Syntactically invalid JSON → ValueError raised here → caught, warned, None. This is the case the new unit test exercises.
2. Valid JSON, invalid entries (e.g. [{"name": 1}], an unknown type, a bad PATH value, or even {}/"x" instead of an array) → from_json_str succeeds. The failure surfaces later, inside enter_environment / run_task when the runner calls to_symtab — outside this try, and past convert_runtime_crashes.

Case 2 is the more likely one in practice: a service-side schema drift or version skew produces well-formed JSON with unexpected entry shapes far more often than it produces truncated JSON. And it fails in the least graceful way available — a raw exception from deep inside the session at env-enter/task-run time, with no resolvedSymbolTable mention in the message to point at the cause.

Worth deciding which contract you actually want, since right now it is neither:

• If degradation is the goal, call to_symtab() inside the try as a validation probe (discarding the result) so contents are checked where you can still fall back to None.
• If fail-fast is the goal, drop the except here — but then say so, because the current docstring claims otherwise.

Either way it would be good to have a test with a payload that is valid JSON but not a valid table, since that is the gap the current suite cannot see.

[github-actions]

The -> Any return annotation is worth tightening to SerializedSymbolTable | None.

Every other converter in this module returns a precise type — _to_rust_path_mapping_rule -> RustPathMappingRule (line 195), _to_v0_action_status -> ActionStatus (line 218), _to_v0_action_state -> ActionState (line 88) — so this one is the odd one out, and the type is already imported at line 16 and known statically.

It matters more than style here, because Any is the one annotation that silences the check you most want on brand-new cross-library plumbing. With check_untyped_defs = true and openjd-model shipping a real .pyi (src/openjd/_openjd_rs.pyi:2036 declares from_json_str(cls, json: str) -> SerializedSymbolTable), mypy is in a position to verify that what this helper produces matches what _v1.Session accepts for resolved_symtab. Any propagates to both call sites (lines 355 and 408) and makes the kwarg unchecked at each — so a future signature change upstream, or a wrong value threaded in here, type-checks clean and only fails at runtime.

Given the prior discussion on this PR about version-skew risk on exactly this kwarg, keeping mypy able to see the type seems worth the one-line change.

[github-actions]

This Field line is the highest-blast-radius change in the PR, and nothing tests it.

validate_object rejects unknown keys wholesale (validation.py:42-47): any key not in the declared set raises ValueError: Unexpected fields: .... So before this line existed, the mere presence of resolvedSymbolTable in a stepDetails response would fail validation and take down every step in the job — not degrade, fail. This line is what makes the whole feature safe to roll out, and it is load-bearing in the deployment direction that actually happens first: the service starts sending the field to workers already in the field.

The gap is that the test_input_validation_success parametrization right above (test_step_details.py:14-63) was not extended — it still covers only dependencies and extensions permutations. The two new tests added in this PR both exercise from_boto, not validate_entity_data, and from_boto uses a plain .get() that never consults the field list. So if this line were dropped or its key misspelled, every new test in this PR still passes while the agent hard-rejects every step entity from a field-sending service.

Two cheap parametrize cases would close it, in both this file and environment_details.py:

• a valid payload including "resolvedSymbolTable": "[]" in test_input_validation_success
• "resolvedSymbolTable": 123 (or a dict) in test_input_validation_failure, pinning the expected_type=str half

The second is worth having on its own: expected_type=str is the only guard rejecting a non-string here, and a dict payload would otherwise flow into _parse_resolved_symtab and hit from_json_str with a non-str.

[github-actions]

This module-level from openjd.expr import SerializedSymbolTable couples the Python adapter importability to the Rust binding, which the surrounding code goes out of its way to treat as optional.

openjd.expr is the Rust extension namespace — rust.py:72 says so explicitly ("openjd.expr.PathFormat is a non-constructable Rust enum"), and rust.py imports it alongside openjd._openjd_rs. Meanwhile _factory.py imports each adapter lazily inside its own try/except ImportError, with the documented rationale: "the adapter module for kind cannot be imported (e.g. the Rust binding is unavailable on this platform)."

That guard exists for PythonSessionRuntime too. So on any host where the openjd._openjd_rs extension will not load — an unsupported arch/libc, a wheel that fell back to sdist, a stripped-down install — the effect of this line is:

• before: RUST fails with NotImplementedError, PYTHON still works, and the no-hint default in select_runtime() (SessionRuntimeKind.PYTHON, _select.py:47) keeps the worker running.
• after: both adapters raise NotImplementedError: PythonSessionRuntime adapter is not available, create_session_runtime fails for every kind, and the except (ValueError, NotImplementedError, OSError) in scheduler.py fails every action of every session. The worker stays up but runs nothing.

So the fallback path now inherits the exact failure mode it was structured to survive, and it does so to obtain one optional display string.

Worth confirming which is actually true here, since the fix differs:

1. If openjd.sessions 0.10.14 already imports openjd.expr transitively (plausible given the "pythonexpr" hint naming in _select.py), then the binding is already a hard requirement of the v0 path and this line changes nothing — in which case a one-line note saying so would stop a future reader from worrying, and the _factory.py comment is the misleading part.
2. If it does not, move the import inside _extract_job_name (or guard it), so a missing binding degrades to job_name=None instead of taking down the Python runtime.

Note the new tests cannot distinguish these: patch.object(python_module, "OpenJDSession", autospec=True) runs only after python.py has already imported successfully.

[github-actions]

This peek performs a blocking BatchGetJobEntity service call on the scheduler thread, and that is a new behaviour for this code path — worth weighing before merge.

The docstring on the new accessor says "Entity resolution results are cached by JobEntities, so the later dequeue issues no additional service request." That is true of the second call, but the first one is this one, and at this point in _create_new_sessions the cache is empty for the entity in question. JobEntities.request() (job_entities.py:124-146) finds no entity_record.data, calls cache_entities([identifier]) synchronously, and blocks on the API round trip with retries. So the cost is not moved — it is moved earlier, onto a thread that previously never did entity resolution.

Two consequences:

1. The warming step is bypassed. Entity resolution deliberately happens on the session thread: Session.run() calls _warm_job_entities_cache() first (session.py:319-364), which batches every action identifier into one request and swallows failures with an INFO log. That design keeps per-entity latency off the scheduler. This peek resolves one entity ahead of it, on the wrong thread, in its own single-identifier request.

2. It is inside the for session_spec in ... loop. With N new sessions assigned in one UpdateWorkerSchedule response, this serialises N blocking calls before any session starts. The scheduler thread is what heartbeats — _sync is called from the run() loop at scheduler.py:315 and drives updated_session_actions/cancellation/shutdown. A slow or retrying BatchGetJobEntity here delays the next heartbeat for all sessions, not just the new one.

Since JobEntities is constructed a few lines above (scheduler.py:927) and is passed to both the queue and the session, one alternative that keeps the current thread model: have the adapter pull the table lazily at first use on the session thread, rather than the scheduler pre-resolving it at construction. If the value really must be known at Session.__init__ time (it must, for job_name= on the v0 session — see python.py:66), it is worth saying so here in a comment, and bounding the cost explicitly, because the blocking call is not obvious at this call site.

[github-actions]

Peeking only self._actions[0] makes the session-scoped job_name depend on which action the service happens to put first, and there is a common case where it silently comes back None.

The else: return None branch covers SYNC_INPUT_JOB_ATTACHMENTS, and that action type is routinely the first action in a session — it is the input-sync that precedes task runs, and list_all_action_identifiers (line 167) treats it as a first-class queue member. So for any queue whose front action is an attachment download/upload, peek returns None, and PythonSessionRuntime is constructed with job_name=None for the entire session — including all the later TASK_RUN actions whose stepDetails did carry a resolvedSymbolTable. The value is not merely late; it is never picked up, because _config.resolved_symbol_table_json is consumed once at construction (python.py:66) and Session.__init__ runs before any action executes.

The result is a feature that works or does not work based on action ordering, with no signal either way: the logger.warning in the except block does not fire, since this is the non-exceptional else path.

Job.Name is job-scoped, so any queued action that has an associated entity can supply it. Two options that remove the ordering dependence:

• Scan for the first action in self._actions that has a table rather than inspecting only index 0 (still one entity resolution in the common case, since ENV_ENTER/TASK_RUN are usually at the front).
• Or source it from jobDetails instead, which the scheduler has already resolved at this point (job_details at scheduler.py:940) and which is unambiguously job-scoped — no peeking, no extra request, no ordering dependence.

At minimum, worth a logger.debug on the else branch so "no table because the front action was an attachment sync" is distinguishable in logs from "the service sent no table."

[github-actions]

This %s-of-exception can leak resolved symbol-table contents into the agent log, and the exposure here is broader than the equivalent line in the Rust adapter.

The table holds resolved job/task parameter values — the same data session_events.py deliberately redacts from BatchGetJobEntity response logging, and the reason template / jobDetails.parameters are excluded from LOGGING_ALLOW_LIST. So it should not reappear verbatim in a warning.

What makes this line riskier than _parse_resolved_symtab in rust.py:249 is that it wraps two calls, not one:

• from_json_str(json_str) — raises on malformed JSON; such errors conventionally quote the offending input fragment.
• .to_symtab() — this is the content validation step, and its errors are about specific entries. An error naming the entry it rejected is very likely to carry that entry name and value, i.e. exactly a resolved parameter value.

Because to_symtab() is inside the try here, the higher-signal-for-leaking failure mode is the one this line reports. And the warning fires precisely when the payload is unexpected — the case whose contents you control least.

Suggest logging only type(e).__name__ (plus payload length or entry count if useful for diagnosis) rather than the exception text. The unit test at test_construction_passes_none_when_resolved_table_is_malformed_json only asserts warning was called once, so tightening the message will not break it.

[github-actions]

Two things about this docstring claim are worth correcting, because both invite a wrong conclusion at the call site.

"Entity resolution results are cached by JobEntities, so the later dequeue issues no additional service request." This holds only when the peek succeeds. On failure it is the opposite of true. cache_entities creates an EntityRecord up front (_create_entity_records, job_entities.py:209-215) and only assigns .data on success; on the DeadlineRequestWorkerNotFound / DeadlineRequestUnrecoverableError path it continues (job_entities.py:231-235), leaving the record with data is None and error is None. request() then falls through both checks and raises the "Should be impossible" RuntimeError (job_entities.py:157-160). That is swallowed by the new except Exception below — so the peek returns None, and the later dequeue re-enters request(), finds data is None, and calls cache_entities again. The failing case therefore costs two blocking service calls, not one, both on the scheduler thread.

"a failure must not break session creation. The subsequent dequeue surfaces the real error through the normal action-failure path." The second sentence is only true for the terminal-error case. If the peek failed for a transient reason and the dequeue retry succeeds, no error is ever surfaced — the session runs to completion with job_name=None, and the only trace is this logger.warning. That is the intended graceful degradation, but the comment describes it as though the error always resurfaces, which would lead a future reader to assume silent-None sessions cannot happen.

Also, the warning drops the exception entirely. Given that this swallows except Exception — including UnsupportedSchema and validation ValueErrors from validate_entity_data, which are permanent and will fail the action moments later — recording at least type(e).__name__ would make the two situations distinguishable in logs without echoing free text.

[github-actions]

This precedence rule is worth reconsidering: in practice the two branches read the same entity, so the fallback almost never engages — and where it does engage, the preference is arguably backwards.

Both tables come from environment_details(environment_id=...) for the same environmentId. JobEntities caches per entity key (_entity_key returns the bare environmentId, job_entities.py:~127), and dequeue at session_queue.py:452 passes that same cached EnvironmentDetails into ExitEnvironmentAction. So resolved_symbol_table_json (from the exit action) and active_env.resolved_symbol_table_json (from the enter action) are the same string from the same cached dict — the ternary reduces to a no-op whenever the enter and exit happen within one session, which is the normal case.

The fallback only matters in the asymmetric cases, and there the choice deserves a second look:

• Cache miss between enter and exit. If the record was re-fetched (e.g. cache_entities failed mid-session and request() retried), the exit-time payload could differ from the enter-time one. The comment says the exit action wins — but for environment teardown, resolving onExit against the same symbols onEnter saw is usually the safer invariant, which is the argument for preferring active_env, not the action.
• details=None. ExitEnvironmentAction now defaults details to None (exit_env.py:32), so None reaching this parameter does not mean "the service omitted a table" — it can equally mean "this action was constructed without details." The stored active_env value is the more trustworthy source in exactly that case, and the ternary does handle it, which is good. But it means the comment rationale ("the service omits one") does not match the condition being tested.

Given that ActiveEnvironment.resolved_symbol_table_json is documented as "Retained so environment teardown resolves the same symbols the enter action used" (session.py:104), preferring the action-supplied table over the retained one directly contradicts that field docstring. Worth reconciling the two — either flip the precedence to match the stated intent, or update the field docstring.

[github-actions]

RustSessionRuntime.__init__ never reads this field — I checked every reference, and the only consumer is PythonSessionRuntime (python.py:66). The Rust adapter gets its table per-action via the resolved_symbol_table_json kwarg on enter_environment / exit_environment / run_task instead.

That makes the config-level plumbing dead weight on the path the feature is actually built for. Because queue.peek_resolved_symbol_table_json() at scheduler.py:1230 is called unconditionally — before the Session constructor and with no reference to the runtime_kind resolved twenty lines earlier — every RUST session pays a blocking BatchGetJobEntity round trip on the scheduler thread for a value that is then threaded into SessionRuntimeConfig and dropped on the floor.

So the cost lands on precisely the sessions that do not benefit, while the sessions that do benefit (PYTHON) are the ones where the v0 runtime cannot use the table for anything except the Job.Name display string.

Cheapest fix is to gate the peek on the selected kind, since runtime_kind is already in scope at that point:

resolved_symbol_table_json = (
    queue.peek_resolved_symbol_table_json()
    if runtime_kind is SessionRuntimeKind.PYTHON
    else None
)

That removes the extra request from the Rust path entirely, and makes the narrow purpose of this field — which this docstring already describes accurately as "Adapters that seed session-scoped symbols at construction (e.g. job_name for the classic Python session)" — visible at the call site too.

[github-actions]

Sourcing job_name from the resolved symbol table introduces an unvalidated string into the v0 session, and it is worth knowing where that value lands before shipping.

Job.Name is a user-controlled field — it is whatever the job submitter typed. In the v0 openjd.sessions.Session, job_name is not inert: it feeds the session log banner and, depending on version, working-directory/log naming. So the questions are the ordinary ones for any user string crossing into path or log construction:

1. Length and content. Nothing here bounds the string or rejects control characters. _extract_job_name checks only isinstance(value, str). If the v0 session incorporates job_name into a filesystem path, a name containing /, .., or a NUL is worth confirming as handled upstream rather than assumed. If it only reaches log output, embedded newlines still allow log-line forgery in the agent log — a name containing \n followed by a plausible-looking log prefix.
2. Where the value comes from now. Note this is a new trust path, not a relabelling of an existing one. Previously the worker never took Job.Name from the wire at all on this code path; now it does, via a field (resolvedSymbolTable) whose entries the worker does not otherwise inspect. The value bypasses the expected_type=str check that guards the outer field, because that check validates the JSON string, not the Job.Name entry inside it.

Given the peek already parses the table anyway, a length cap and a rejection of control characters (or at minimum a comment recording that upstream openjd.sessions sanitises job_name before any path or log use) would close this cheaply. If upstream already validates, saying so here is still useful — it is not evident from this call site, and the isinstance check suggests validation was considered the caller responsibility.

[github-actions]

Making details optional here is worth a second look, because the only production caller always has the value and the default silently disables the feature.

There is exactly one non-test construction site — session_queue.py:452 — and by that point environment_details has already been resolved (it is fetched a few lines above for both ENV_ENTER and ENV_EXIT, and EnterEnvironmentAction takes it as a required kwarg). So the = None default is not serving any real caller; it exists only for the tests in test_exit_env.py that construct the action bare.

The cost of the default is that the failure is silent. Forget the kwarg at some future call site and exit_environment receives resolved_symbol_table_json=None, the onExit script resolves against a different symbol context than onEnter did, and nothing logs or raises. Contrast EnterEnvironmentAction, where omitting details is a TypeError at construction.

Given EnterEnvironmentAction._details is required and non-optional, making this one required too (details: EnvironmentDetails) would keep the two symmetric, let start() drop the if self._details is not None else None ternary, and turn a silent degradation into an immediate error. The three tests that build the action without details would each need the kwarg added — a small change that also makes them exercise the shape production actually uses.

If the optionality is deliberate — e.g. anticipating a future caller that genuinely has no details — a comment naming that caller would help, since today the signature suggests a case that does not exist.

[github-actions]

This comment refers to the authoring process rather than the code — "(concurrent agent adds _details field)" describes how the field came to exist, not anything a reader of the test needs. Worth dropping it, along with the now-unnecessary # type: ignore[attr-defined]: _details is a declared class attribute on ExitEnvironmentAction (exit_env.py:29), so mypy resolves it without the suppression.

More substantively, this assertion is weaker than the thing the test is named for. assert result._details is env_details checks that the object was stored on a private attribute; it does not check that the table reaches the session. Since ExitEnvironmentAction.start() is what forwards it (resolved_symbol_table_json=self._details.resolved_symbol_table_json), a refactor that renames the attribute or drops the forwarding in start() leaves this test green.

Asserting through the public behaviour instead would close that — call result.start(session=mock_session, executor=...) and assert mock_session.exit_environment received resolved_symbol_table_json=table_json. That is the pattern the tests in test_exit_env.py already use, and it makes table_json (currently defined but only reachable via the private attribute) actually load-bearing in this test.

[github-actions]

This except Exception is the wrong width for what the try actually calls, and the consequence is worse here than in the Rust adapter because there is no decorator behind it.

to_symtab() is the Rust content-validation step (the binding rejects invalid entries there, not in from_json_str). This repo already has a documented policy for Rust code that can fail abnormally — convert_runtime_crashes in _abc.py:30, whose own docstring exists because of "a Rust panic crossing the PyO3 boundary as a BaseException". Every openjd.sessions._v1 call in rust.py is wrapped in it, including the two _parse_resolved_symtab call sites.

This one is not, and it cannot be:

• _extract_job_name runs from PythonSessionRuntime.__init__ (line 66), before any adapter method exists to decorate.
• A BaseException from to_symtab() therefore escapes __init__, escapes create_session_runtime, and escapes Session.__init__.
• The scheduler catches only except (ValueError, NotImplementedError, OSError) at scheduler.py:1251, so it propagates out of _create_new_sessions — which is called from the scheduler run() loop. That is the thread that heartbeats, so the failure mode is not "this session degrades to job_name=None", it is the worker stopping.

The trigger is service-controlled and is exactly the payload class the graceful-degradation story is meant to absorb: well-formed JSON whose entries are not valid symbol-table entries. from_json_str accepts those, and to_symtab() is where they blow up.

Two options:

• Catch BaseException here (this helper is pure parsing with a None fallback, so there is nothing to lose by being total), or
• do not call to_symtab() in __init__ at all — resolve job_name lazily, or add SessionRuntimeCrashError/BaseException to the scheduler's construction guard.

Worth noting the current tests cannot see this: test_construction_passes_none_when_resolved_table_is_malformed_json patches nothing on the Rust side and feeds "{not json", which fails in from_json_str — the one path that does raise a normal Exception. A case that reaches to_symtab() would pin the real behaviour.