logs.gokuls.in

Fixes the last rough edge in wrkflw's expression evaluator: toJSON(env). Prior PRs (#97–#101) landed bare-context support for toJSON(steps), toJSON(needs), toJSON(github), toJSON(secrets), and toJSON(matrix). toJSON(env) was the outlier — it used an is_user_env_var() prefix heuristic that incorrectly excluded user-declared vars with runner-style names (most notably GITHUB_TOKEN).

TODO item 6 from the bare context serialization plan.

Root cause

env_context is a single flat HashMap<String, String> that mixes user-declared env with runner-seeded vars (GITHUB_*, RUNNER_*, INPUT_*, WRKFLW_*, CI, MATRIX_*). toJSON(env) should dump only the user slice — but no key-level filter can separate the two populations after they've been merged. Users writing the canonical pattern env: { GITHUB_TOKEN: \${{ secrets.GITHUB_TOKEN }} } would silently lose their token from toJSON(env) output.

The fix

Separate the two populations at the source:

• Adds user_env: &'a HashMap<String, String> to ExpressionContext, read by the toJSON(env) / bare-env resolve arm.
• env_context remains the union used for dotted lookups (env.FOO, github.X).
• user_env is maintained in parallel through every scope where user-declared env enters the system:

1. Workflow: seeded empty, populated from resolved workflow.env (with or_insert precedence so it can't shadow runner vars).

2. Job: clone workflow user_env, layer container.env + job.env. Runner-internal GITHUB_JOB stays in env_context only.

3. Matrix jobs: same as job-level but skip MATRIX_* / MATRIX_CONTEXT — those are runner-internal and belong to toJSON(matrix).

4. Step: clone job user_env, layer resolved step.env.

5. \$GITHUB_ENV writes: apply_step_environment_updates mirrors env-file writes into user_env, so a step writing echo FOO=bar >> \$GITHUB_ENV sees FOO in the next step's toJSON(env). \$GITHUB_PATH updates stay out of user_env (PATH is not a user-declared env var).

Commits

• f4a8978 — scaffolding: adds the field, threads it through every construction site with a heuristic bridge so the tree stays green. No behavior change.
• 905f029 — replaces bridges with real tracking through the workflow→job→step merges, wires \$GITHUB_ENV writes into user_env, deletes is_user_env_var() and its test.

Scope decisions (flagged in the code)

• Reusable workflows (run_called_workflow) start with an empty user_env — parent workflow.env doesn't leak across reusable-workflow boundaries, and the called workflow's own workflow.env isn't currently merged into child_env (pre-existing gap, not fixed here). Job-level merges still populate user_env correctly downstream.
• GitLab pipelines don't carry workflow-level env:, so that path also starts empty.
• Sibling bug in toJSON(github) — a user-declared GITHUB_CUSTOM will still appear in toJSON(github) masquerading as a runner var. Deferred to a follow-up; this PR stays focused on toJSON(env).

Test plan

• cargo test -p wrkflw-executor --lib — 398 pass (1 deleted: is_user_env_var_filters_all_github_context_keys in environment.rs tested the now-gone heuristic).
• cargo build --workspace — clean.
• cargo clippy --workspace --tests -- -D warnings — clean.
• cargo fmt --all --check — clean.

New / updated tests

• Added tojson_env_includes_user_var_with_internal_prefix — user-declared GITHUB_CUSTOM appears in toJSON(env) (inverted from the prior tojson_env_excludes_user_var_with_internal_prefix, which asserted the bug as if it were a feature).
• Added tojson_env_includes_user_declared_github_token — the canonical GITHUB_TOKEN case: user-declared token appears; process-inherited GITHUB_SHA doesn't.
• Updated tojson_env_returns_object and tojson_env_empty_when_only_internal_vars to construct ExpressionContext directly with distinct env_context vs user_env populations.
• Updated apply_updates_merges_env_and_path to verify \$GITHUB_ENV writes mirror into user_env and \$GITHUB_PATH writes don't.

Fifth and final in the bare-context series after #96 (env), #97 (steps), #98 (needs), #99 (github), #100 (secrets).

• Adds a bare-matrix arm in ExpressionContext::resolve so ${{ toJSON(matrix) }} returns the matrix combination as a JSON object instead of null. Each value flows through the existing yaml_value_to_expr helper, so bare and dotted (matrix.os) forms agree on per-value shape.
• One intentional asymmetry with the other bare-context arms: matrix_combination = None (non-matrix job) returns ExprValue::Null, not an empty Object. None encodes "no matrix context exists" — what real GHA exposes for jobs without a matrix strategy. Some(empty) still renders as {}, preserving the Some/None distinction the field's Option<...> type already carries.
• Drops the stale // TODO: support other bare contexts: matrix comment. Nothing remains.

Test plan

• cargo test -p wrkflw-executor --lib expression:: — 96 tests pass, including 10 new ones
• cargo test -p wrkflw-executor — full suite (398 tests) passes
• cargo clippy -p wrkflw-executor --all-targets -- -D warnings — clean
• cargo build --workspace — compiles

New tests

• tojson_matrix_returns_object — populated matrix → JSON object with expected keys
• tojson_matrix_no_matrix_returns_null — None → "null" (pins the Null-on-None asymmetry)
• tojson_matrix_empty_combination — Some(empty) → {} (pins the Some-vs-None distinction)
• tojson_matrix_mixed_value_types — string/number/bool preserved natively, not stringified
• tojson_matrix_sorted_keys — lexicographic key ordering
• fromjson_tojson_matrix_produces_parseable_json — round-trip
• bare_matrix_is_truthy / bare_matrix_when_none_is_null — bare truthiness in both states
• bare_matrix_does_not_shadow_dotted_access — regression guard for matrix.<key>
• tojson_matrix_yaml_sequence_falls_through_to_string — pins YAML-string fallback for non-scalar values

Fourth in the series after #96 (env), #97 (steps), #98 (needs), #99 (github). Same bug, same fix pattern — ExpressionContext::resolve had no bare-secrets match arm, so resolving the bare identifier fell through to _ => ExprValue::Null and toJSON(secrets) serialized as "null".

secrets_context is already &HashMap<String, String> in the exact shape real GHA exposes. The fix clones its entries into ExprValue::String and wraps them in ExprValue::Object — simpler than #99's github arm (no prefix strip / exclusion list) and simpler than #97/#98 (no nested outputs sub-object).

Plaintext values by design

Secret values surface in plaintext in toJSON(secrets), matching real GHA. The common fromJSON(toJSON(secrets)) pipe-through-an-action pattern depends on the exact original values surviving the round-trip. Masking remains a log-boundary concern handled by wrkflw_secrets::SecretMasker (already wired in engine.rs) — pulling it into the evaluator would break the round-trip, diverge from GHA, and duplicate a concern with one correct home.

A dedicated test (tojson_secrets_returns_values_in_plaintext) pins that decision so any future switch is deliberate.

Tests

Mirrors the #96–#99 suites:

• tojson_secrets_returns_object — populated map, exact keys/values.
• tojson_secrets_empty_context — empty map → {}, not null.
• tojson_secrets_sorted_keys — alphabetical ordering.
• tojson_secrets_preserves_special_characters — quotes, backslashes, PEM-style newlines.
• fromjson_tojson_secrets_produces_parseable_json — round-trip preserves values.
• bare_secrets_is_truthy — Object semantics in if: contexts.
• bare_secrets_does_not_shadow_dotted_access — regression guard for secrets.NAME.
• tojson_secrets_returns_values_in_plaintext — pins the no-masking-at-this-layer decision.

Also drops secrets from the lingering // TODO: support other bare contexts comment. Only matrix remains.

Test plan

• cargo test -p wrkflw-executor — 388 passing, 0 failing
• cargo clippy -p wrkflw-executor --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean

Third in the bare-context-serialization series after #96 (env), #97 (steps), #98 (needs). Same disease, same pattern.

toJSON(github) was returning null because ExpressionContext::resolve() had no bare-github arm — the identifier fell through to the catch-all _ => ExprValue::Null, which toJSON dutifully serialized.

The fix mirrors the bare-env arm: there's no dedicated github_context field because env_context is already the source of truth for GITHUB_* vars (the dotted-access arm reads github.sha → GITHUB_SHA from there). So the bare case inverts that mapping: iterate env_context, filter keys starting with GITHUB_, strip the prefix, lowercase, wrap as ExprValue::Object.

• New "github" if parts.len() == 1 arm in resolve().
• Trimmed the adjacent TODO comment — github is done, secrets and matrix are still next.
• 8 new unit tests: populated object, empty env, no-GITHUB-prefix, sorted keys, bare-github truthiness, dotted-access regression guard, special characters, fromJSON/toJSON round-trip.

Known limitation (unchanged)

Does not produce a nested event sub-object. Real GHA parses $GITHUB_EVENT_PATH as a full JSON payload; wrkflw approximates it via flat env vars. This matches the existing dotted-access behavior — out of scope for this PR.

Test plan

• cargo build -p wrkflw-executor — clean
• cargo test -p wrkflw-executor --lib expression::tests — 74/74 pass (8 new)
• cargo clippy -p wrkflw-executor --all-targets -- -D warnings — clean
• cargo fmt --all -- --check — clean

Bare needs context now resolves to a nested ExprValue::Object of { <job_id>: { outputs: { ... }, result: "success" | "failure" | ... } } — so toJSON(needs) returns the shape GitHub Actions provides instead of "null".

Same pattern as #97 (toJSON(steps)) and #96 (toJSON(env)): the resolve() method had no match arm for the bare identifier and fell through to Null. Added the arm, merged the key sets of needs_context and needs_results so a job shows up even if only one of the two maps has an entry for it, and omitted result when not recorded (mirroring how the steps arm omits outcome/conclusion).

Only real shape difference from steps: a single result string instead of the (outcome, conclusion) pair. Dropped needs from the lingering TODO comment — github, secrets, matrix still to go.

Part of the bare-context serialization series tracked in the local TODO list (item 2 of 6).

Test plan

• cargo test -p wrkflw-executor --lib expression:: — 66/66 pass (7 new tojson_needs_* / bare_needs_is_truthy)
• cargo test -p wrkflw-executor — 368/368 pass
• cargo clippy -p wrkflw-executor --all-targets -- -D warnings — clean
• Tests mirror the toJSON(steps) suite: populated, empty, sorted keys, one-sided populations, special-character escaping