Skip to content

feat(core): thread workflow context (ctx.*) into node scope + post-merge roadmap - #23

Merged
cemililik merged 4 commits into
mainfrom
development
Jun 15, 2026
Merged

feat(core): thread workflow context (ctx.*) into node scope + post-merge roadmap#23
cemililik merged 4 commits into
mainfrom
development

Conversation

@cemililik

@cemililik cemililik commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Closes the highest-value open engine gap from deferred-tasks.md: the authored context: namespace is now resolved at run start and threaded to every node, so a bare ctx.key resolves instead of reading undefined. Plus the post-1.R/1.Q roadmap update. Engine-only (@relavium/core, zero platform imports); 632 core tests green, format:check + lint + typecheck + build clean, Leakwatch clean; put through an adversarially-verified multi-agent review (1 real HIGH found + fixed).

ctx.* threading (commit a9a3671 + review fix 0db2a91)

The gap: the condition/transform/merge_fn sandbox scope, the AgentRunner prompt scope, and the human-gate message_template/assignee all bound ctx: {} — the workflow context: was never resolved/threaded, so a bare ctx.key JS read saw undefined (a silent mis-route risk), and a {{ctx.key}} template resolved to undefined.

  • NodeExecContext.ctx — the new seam field: the resolved, frozen ctx.* namespace threaded to every node ({} when the workflow declares no context:).
  • WorkflowEngineDeps.resolverCapabilities — the engine resolves context: values (which may {{inputs.*}} / read_file) at run start through the same purity seam the handlers use.
  • Run-start eager-once resolution (#resolveContextOrFail): right after run:started, before any node — a resolution failure closes the run run:failed{validation} (never runs nodes against a partial context); a cancel mid-resolve settles run:cancelled.
  • Cross-process resume re-resolves the context (it is deliberately not carried in the checkpoint) via the unified beginResume (replaced kick()).
  • Consumers read ctx.ctx: buildExpressionScope, the AgentRunner's resolvePrompt, and (the review-caught HIGH) the human-gate handler.
  • No new ADR — the eager-once context is already specified (workflow-yaml-spec context: + resolveContext); this wires it.

Review trail

A 4-lens Sonnet review (correctness/concurrency, purity/non-negotiables, security — can a secret leak via ctx.*?, tests/contracts) with refute-by-default verification found one real HIGH: the human-gate handler was the one scope site the change missed (ctx: {}), so {{ctx.key}} in a gate's message_template/assignee silently failed. Fixed (ctx: ctx.ctx) with a regression test that the prior ctxFor would have passed. Security lens confirmed no new secret-leak path (parse-time taint on context-value/node-text sinks + the input-handler masking still hold).

Tests (+9)

A transform reading a bare ctx.key; engine e2e (context resolved + threaded); a no-context: workflow → ctx: {}; a read_file context value via an injected capability; a context-resolution failure → run:failed{validation} before any node (with run:started-first ordering); a cancel racing resolution → run:cancelled; cross-process resume re-resolving so post-gate nodes see ctx.*; resume-path re-resolution failure → run:failed{validation}; and the human-gate {{ctx.key}} resolution.

Post-merge roadmap (commit b229555)

Marks 1.R + 1.Q ✅ Done (PR #22) across phase-1-engine-and-llm.md / current.md / CLAUDE.md, advances the next-workstream pointer to 1.S (node retry), and re-points the relevant deferred-tasks.md items (the ctx-threading item is checked off here; the structuredClone-transport obligation is re-pointed as dormant since ctx is re-resolved, not transported).

🤖 Generated with Claude Code

Summary by Sourcery

Thread the resolved workflow context: namespace (ctx.*) through the engine so all node types and agents can read it, and update roadmap/docs to mark checkpoint/resume and human gate workstreams as complete with follow-up items captured.

New Features:

  • Add workflow context: resolution at run start and resume, exposing a frozen ctx.* namespace on NodeExecContext for use by expressions, agents, and human-gate templates.

Bug Fixes:

  • Ensure human-gate message_template and assignee correctly resolve {{ctx.*}} values from the threaded workflow context instead of an empty namespace.
  • Classify failures during workflow context resolution as run:failed{validation} while treating mid-resolution cancellations as run:cancelled.
  • Re-resolve workflow context on cross-process resume so post-gate nodes see ctx.*, failing the run with validation if re-resolution is not possible.

Enhancements:

  • Extend the workflow engine to accept resolver capabilities for resolving context: values (including read_file) once per run.
  • Refine the resume path via a new beginResume entry that re-resolves context before applying gate decisions or driving downstream work.
  • Clarify the execution model docs to describe eager-once context resolution and non-checkpointing of ctx.*.
  • Adjust roadmap/deferred-tasks notes to mark workflow-context threading as completed and document that ctx is re-resolved rather than checkpointed.

Documentation:

  • Update roadmap and status documents (current, phase-1, CLAUDE guide, deferred tasks) to reflect completed 1.Q/1.R work and the remaining 1.S/1.AC workstreams, plus Phase-2 follow-ups for checkpoint/resume and human gates.

Summary by CodeRabbit

Release Notes

  • New Features

    • Workflow context (ctx.*) is resolved once per run and consistently available across expressions, transforms, agent prompts, and human-gate message fields.
    • During cross-process resume, ctx.* is re-resolved so post-gate nodes see up-to-date values.
  • Documentation

    • Updated execution-model and roadmap materials to reflect context-resolution timing and failure behavior, plus checkpoint/resume and human-gate completion.
  • Tests

    • Expanded engine and node-handler test coverage, including prompt/template interpolation for {{ctx.*}} and resume/gate edge cases.

cemililik and others added 3 commits June 15, 2026 01:58
Post-merge roadmap + status update now that checkpoint/resume (1.R) and the human
gate (1.Q) have merged.

- phase-1-engine-and-llm.md: ✅ Done markers on §1.Q and §1.R; top status block
  records the PR #22 landing and the remaining 1.m4 lane (1.S, 1.AC).
- current.md: status narrative + next-workstream pointer advanced to node retry
  (1.S); last-updated 2026-06-15.
- CLAUDE.md: status paragraph + detailed status reflect 1.R/1.Q landed, 1.S next.
- deferred-tasks.md: re-point the now-landed-context items — the structuredClone
  `ctx`-transport obligation moves off 1.R (the checkpoint carries no resolved ctx)
  to the ctx-threading work; mid-tool-loop resume noted as Phase-2 (1.R resumes at
  gate boundaries only); the ctx-threading fold-into-1.Q/1.R window noted closed
  (now its own task). New "Checkpoint/resume + human gate (1.R/1.Q) follow-ups"
  section captures the three confirmed Phase-2 deferrals (gate-timer re-arm on
  rehydration, content-hash workflow-snapshot identity guard, cross-process
  gate-resolve TOCTOU → store-level uniqueness).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Close the highest-value open engine gap (deferred-tasks): the authored `context:`
namespace was never resolved/threaded, so the sandbox scope (condition/transform/
merge_fn) and the AgentRunner prompt bound `ctx: {}` — a bare `ctx.key` JS read saw
`undefined` (a silent mis-route risk).

- NodeExecContext gains `ctx: Readonly<Record<string,string>>` — the resolved,
  frozen workflow-context namespace threaded to every node.
- WorkflowEngineDeps gains `resolverCapabilities?` — the engine resolves `context:`
  values (which may `{{inputs.*}}` / `read_file`) at run start through the same
  purity seam the handlers use.
- The engine resolves the context ONCE right after `run:started` (the spec's
  eager-once context — `#resolveContextOrFail` over `resolveContext`), before any
  node runs; a resolution failure closes the run with `run:failed{validation}` (never
  runs nodes against a partial context), and a cancel mid-resolve settles
  `run:cancelled`. The resolved map is threaded via `NodeExecContext.ctx`.
- Cross-process resume RE-RESOLVES the context (it is deliberately NOT carried in the
  checkpoint): the resume drive is unified into `beginResume`, which resolves context
  then either kicks (gate already resolved) or applies the decision.
- Consumers: `buildExpressionScope` and the AgentRunner's `resolvePrompt` now read
  `ctx.ctx` (was `{}`).
- No new ADR — the eager-once context is already specified (workflow-yaml-spec
  `context:` + `resolveContext`); this wires it.

Tests (+5): a transform reading a bare `ctx.key`; engine e2e — context resolved +
threaded; a context-resolution failure → `run:failed{validation}` before any node;
a `read_file` context value via an injected capability; and a cross-process resume
re-resolving the context so post-gate nodes see `ctx.*`.

Docs: execution-model.md §2 (the run-start eager-once context step); the
NodeExecContext.ctx contract; deferred-tasks ctx-threading item checked off and the
structuredClone-transport obligation re-pointed (dormant — ctx is re-resolved, not
transported).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adversarial review of the ctx-threading diff caught a real HIGH: the
`human_in_the_loop` handler was the one scope-building site the change missed —
it still bound `ctx: {}`, so a `{{ctx.key}}` in a gate's `message_template` /
`assignee` (a documented template field, workflow-yaml-spec §context) resolved to
`undefined` and failed the gate with `validation`. Now reads `ctx.ctx`, matching
`buildExpressionScope` and the AgentRunner's `resolvePrompt`.

- human-gate.ts: `ctx: ctx.ctx` (+ corrected the scope comment).
- execution-model.md §2: human-gate `message_template` / `assignee` added to the
  list of `ctx.*` consumers (the doc had mirrored the broken state).
- Tests (+4): a gate `{{ctx.key}}` in message_template/assignee resolves (ctxFor
  gains a `ctx` option — it would otherwise pass with the bug); a no-`context:`
  workflow threads `ctx: {}`; a cancel racing context resolution settles
  `run:cancelled` (not `run:failed{validation}` — the previously-untested
  #resolveContextOrFail cancel branch); context RE-resolution failure on the resume
  path closes the run `run:failed{validation}` without applying the decision; and
  the context-failure test now pins `run:started` precedes `run:failed`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Reviewer's Guide

Threads the resolved workflow context (ctx.*) into the engine’s node execution scope by eagerly resolving the workflow context: map at run start (and on resume), wiring it through NodeExecContext into expression, agent, and human-gate scopes, and updates roadmap/architecture docs to reflect completed 1.Q/1.R and the ctx-threading behavior.

Sequence diagram for ctx.* resolution and threading on start and resume

sequenceDiagram
  participant Engine as WorkflowEngine
  participant Exec as RunExecution
  participant Resolver as resolveContext
  participant Handler as NodeExecutor
  participant Scope as buildExpressionScope
  participant Agent as resolvePrompt
  participant Gate as runHumanGate

  Engine->>Exec: begin()
  Exec->>Resolver: resolveContext(workflow, inputs, resolverCapabilities, signal)
  alt context resolved
    Exec-->>Exec: #resolvedContext = frozen ctx map
    Exec->>Exec: #schedule()
    Exec->>Handler: execute(NodeExecContext{ctx: #resolvedContext})
    Handler->>Scope: buildExpressionScope(ctx)
    Scope-->>Handler: ExpressionScope{ctx}
    Handler-->>Engine: node:completed
    Engine-->>Engine: emit run:completed
  else resolution fails and cancelling
    Exec-->>Engine: settle(run:cancelled)
  else resolution fails (validation)
    Exec-->>Engine: settle(run:failed{validation})
  end

  rect rgb(235,235,245)
    Engine->>Exec: resumeFromCheckpoint()
    Exec->>Exec: beginResume(gateId, decision, gateAlreadyResolved)
    Exec->>Resolver: resolveContext(workflow, inputs, resolverCapabilities, signal)
    alt context re-resolved
      Exec-->>Exec: #resolvedContext = frozen ctx map
      alt gateAlreadyResolved
        Exec->>Exec: #schedule()
      else apply decision
        Exec->>Exec: resume(gateId, decision)
      end
      Exec->>Handler: execute(NodeExecContext{ctx: #resolvedContext})
      Handler->>Agent: resolvePrompt(ctx)
      Handler->>Gate: runHumanGate(ctx)
    else re-resolution fails
      Exec-->>Engine: settle(run:failed{validation})
    end
  end
Loading

File-Level Changes

Change Details Files
Add eager-once workflow context resolution and thread ctx.* into all node execution scopes, including resumes.
  • Extend WorkflowEngineDeps and WorkflowEngine to accept and store resolverCapabilities, defaulting to an empty capability set.
  • Introduce RunExecution.#resolvedContext and #resolverCapabilities, populating them via a new #resolveContextOrFail method that calls resolveContext with workflow, inputs, resolver capabilities, and abort signal.
  • Call #resolveContextOrFail from begin() immediately after run:started but before scheduling nodes, settling the run as run:failed{validation} or run:cancelled on failure.
  • Replace kick() with beginResume(...) that re-resolves context on resume and then either resumes from a gate decision or schedules remaining work when the gate was already resolved.
  • Thread the resolved context into each node execution via NodeExecContext.ctx when calling the node executor.
packages/core/src/engine/engine.ts
docs/architecture/execution-model.md
Expose ctx.* on NodeExecContext and consume it in expression scopes, agent prompts, and human gate templates.
  • Extend NodeExecContext to include a readonly ctx map of resolved context values and plumb it through all test helpers that construct a node-exec context.
  • Update buildExpressionScope to pass through ctx.ctx instead of binding ctx to an empty object, so condition/transform/merge_fn expressions can read ctx.key.
  • Update the AgentRunner’s resolvePrompt and the human-gate handler to build RunScope with ctx: ctx.ctx, so {{ctx.*}} in prompts and gate message_template / assignee resolve correctly.
  • Add targeted unit tests for transform handlers and the human-gate handler to assert that bare ctx.key and {{ctx.key}} resolve from the threaded context.
packages/core/src/engine/node-executor.ts
packages/core/src/engine/node-handlers/scope.ts
packages/core/src/engine/agent-runner.ts
packages/core/src/engine/node-handlers/human-gate.ts
packages/core/src/engine/node-handlers/node-handlers.test.ts
packages/core/src/engine/node-handlers/human-gate.test.ts
packages/core/src/engine/agent-runner.test.ts
Add engine-level tests that cover context resolution, error handling, cancellation races, and cross-process resume behavior for ctx.*.
  • Add a new WorkflowEngine — workflow context (ctx.*) resolution test suite that asserts context is resolved once at run start and threaded to nodes, with ctx.* echoed from a transform node.
  • Test that workflows without a context: block receive ctx: {} and still complete successfully.
  • Verify that context-resolution failures produce run:failed with validation error code before any node starts, while cancel-during-resolution leads to run:cancelled.
  • Assert that read_file in context values runs through injected resolverCapabilities.readFile and that cross-process resumes re-resolve context so post-gate nodes see the same ctx.*.
  • Verify that context re-resolution failure on resume prevents gate decisions from being applied and closes the run with run:failed{validation}.
packages/core/src/engine/engine.test.ts
Update roadmap and status documentation to mark 1.Q/1.R as done and document ctx-threading and checkpoint/resume follow-ups.
  • Mark the workflow-context threading task as completed and clarify that ctx is re-resolved (not checkpointed), leaving structuredClone transport as a dormant obligation unless ctx is ever persisted.
  • Clarify the behavior and scope of checkpoint/resume and human-gate work, adding a dedicated section for Phase-2 checkpoint/resume + human-gate follow-ups.
  • Update phase roadmap, current roadmap, and CLAUDE status docs to mark 1.Q (human gate) and 1.R (checkpoint/resume) as done, describe their behavior, and point the next workstream to 1.S (node retry).
  • Document in the execution-model overview that the engine now resolves workflow context once after run:started and re-resolves on resume, and that this ctx.* namespace is threaded to nodes.
docs/roadmap/deferred-tasks.md
docs/roadmap/phases/phase-1-engine-and-llm.md
docs/roadmap/current.md
docs/architecture/execution-model.md
CLAUDE.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f8c6880a-1fc3-469c-b672-8d878fd0f142

📥 Commits

Reviewing files that changed from the base of the PR and between 0db2a91 and ded5f97.

📒 Files selected for processing (3)
  • packages/core/src/engine/agent-runner.test.ts
  • packages/core/src/engine/engine.test.ts
  • packages/core/src/engine/engine.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/src/engine/engine.test.ts

📝 Walkthrough

Walkthrough

Adds eager resolution of the workflow context: block into a frozen ctx.* namespace immediately after run:started, threads it through NodeExecContext to every node handler, expression scope, human-gate template, and agent-runner prompt. On cross-process resume, ctx.* is re-resolved rather than restored from checkpoint. Roadmap and architecture docs are updated to record completion of 1.Q/1.R and document the new lifecycle.

Changes

ctx.\ Workflow Context Threading*

Layer / File(s) Summary
NodeExecContext.ctx contract and handler scope threading
packages/core/src/engine/node-executor.ts, packages/core/src/engine/node-handlers/scope.ts, packages/core/src/engine/node-handlers/human-gate.ts, packages/core/src/engine/agent-runner.ts
Adds readonly ctx: Readonly<Record<string, string>> to NodeExecContext; updates buildExpressionScope, human-gate interpolation scope, and resolvePrompt to use ctx.ctx instead of {}.
Engine eager resolution and beginResume wiring
packages/core/src/engine/engine.ts
Adds resolverCapabilities to WorkflowEngineDeps; RunExecution stores #resolvedContext, resolves it eagerly in begin() with cancel vs. validation error discrimination, re-resolves on cross-process resume via new beginResume, and passes #resolvedContext into NodeExecContext at every node dispatch.
ctx.\ resolution, threading, and resume tests*
packages/core/src/engine/engine.test.ts, packages/core/src/engine/node-handlers/human-gate.test.ts, packages/core/src/engine/node-handlers/node-handlers.test.ts, packages/core/src/engine/agent-runner.test.ts
New engine-level integration test suite (7 cases) covering initial resolution, pre-node validation failure, empty context, cancellation race, resolver-capability read_file, and resume re-resolution success/failure; test harness helpers updated to carry ctx through makeCtx/ctxFor.

Roadmap and Architecture Documentation Updates

Layer / File(s) Summary
Architecture doc and roadmap status updates
docs/architecture/execution-model.md, CLAUDE.md, docs/roadmap/current.md, docs/roadmap/phases/phase-1-engine-and-llm.md
Adds ctx.* resolution lifecycle section to execution-model; marks 1.Q and 1.R as ✅ done (PR #22, 2026-06-15) across CLAUDE.md, current.md, and phase-1 doc; shifts next-workstream focus to node retry (1.S) and pre-egress budget governor (1.AC).
Deferred-tasks update
docs/roadmap/deferred-tasks.md
Marks ctx checkpoint-transport obligation dormant, aligns mid-tool-loop resume item with 1.R gate-boundary behavior, closes 1.P ctx-threading follow-up, adds three Phase-2 deferred items for timer re-arming, workflow-identity guardrails, and concurrent gate-resolve TOCTOU.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant WorkflowEngine
  participant RunExecution
  participant resolveContext
  participant NodeHandler

  Caller->>WorkflowEngine: start(workflow, inputs, resolverCapabilities)
  WorkflowEngine->>RunExecution: new RunExecution(resolverCapabilities)
  RunExecution->>RunExecution: emit run:started
  RunExecution->>resolveContext: `#resolveContextOrFail`(workflow.context)
  alt cancellation races resolution
    resolveContext-->>RunExecution: AbortError
    RunExecution-->>Caller: run:cancelled
  else resolution validation failure
    resolveContext-->>RunExecution: ValidationError
    RunExecution-->>Caller: run:failed{validation}
  else resolved successfully
    resolveContext-->>RunExecution: ctx map → `#resolvedContext`
    RunExecution->>NodeHandler: execute(NodeExecContext{ctx: `#resolvedContext`})
    NodeHandler-->>RunExecution: outcome
    RunExecution-->>Caller: run:completed
  end

  Caller->>WorkflowEngine: resumeFromCheckpoint(runId, decision)
  WorkflowEngine->>RunExecution: beginResume(decision)
  RunExecution->>resolveContext: `#resolveContextOrFail` (re-resolve, not from checkpoint)
  alt re-resolution failure
    resolveContext-->>RunExecution: error
    RunExecution-->>Caller: run:failed{validation}
  else re-resolved
    resolveContext-->>RunExecution: ctx map → `#resolvedContext`
    RunExecution->>NodeHandler: execute(NodeExecContext{ctx: `#resolvedContext`})
    RunExecution-->>Caller: run:completed
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • HodeTech/Relavium#22: This PR directly builds on the checkpoint/resume and human-gate suspend/resume infrastructure landed in PR #22, adding ctx.* re-resolution in beginResume and updating roadmap entries to record PR #22's completion.
  • HodeTech/Relavium#18: Both PRs extend NodeExecContext with new fields (toolPolicy there, ctx here) and update AgentRunner's execution context shape.
  • HodeTech/Relavium#15: This PR wires ResolverCapabilities and resolveContext into the engine to populate ctx.*, building directly on the interpolation/resolver infrastructure introduced in PR #15.

Poem

🐇 A context resolved with a hop and a bound,
ctx.* now threaded all the way down!
Gate templates and prompts read the map that was frozen,
Re-solved on resume — no stale data chosen.
The roadmap turns green: 1.Q and 1.R done,
Three deferred to Phase Two — the rabbit hops on! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main feature: threading workflow context (ctx.*) into node scope, which is the core implementation accomplishment, plus a post-merge roadmap update.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch development

Comment @coderabbitai help to get the list of available commands and usage tips.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="packages/core/src/engine/engine.ts" line_range="180-181" />
<code_context>
   readonly #bus: RunEventBus;
   readonly #onSettled: (runId: string) => void;
+  readonly #resolverCapabilities: ResolverCapabilities;
+  /** The resolved workflow `context:` (`ctx.*`), folded once at run start (or re-resolved on resume). */
+  #resolvedContext: Readonly<Record<string, string>> = {};

   readonly #abort: AbortControllerLike;
</code_context>
<issue_to_address>
**issue (bug_risk):** `#resolvedContext` is declared `readonly` but later reassigned, which will break type-checking

For `#` private fields, `readonly` is enforced by TypeScript, so this reassignment will fail at compile time. Either remove `readonly` from `#resolvedContext`, or keep it immutable by resolving the context into a local variable and passing that down instead of mutating the field.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread packages/core/src/engine/engine.ts

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request implements workflow-context (ctx.*) threading into expression and agent scopes. The WorkflowEngine now resolves the workflow context: map once at run start (using injected resolverCapabilities such as readFile) and threads the frozen ctx.* namespace to all nodes via NodeExecContext.ctx. Additionally, the context is re-resolved on cross-process resume rather than being persisted in checkpoints. This allows expressions, agent prompts, and human-gate templates to access the resolved context values directly. Comprehensive unit and integration tests have been added to verify these behaviors, including failure paths and cancel races. I have no feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/core/src/engine/agent-runner.test.ts (1)

115-130: ⚡ Quick win

Add an agent prompt regression for {{ctx.*}}.

The helper now satisfies the required field, but hardcoding ctx: {} leaves the AgentRunner part of this PR’s ctx-threading contract untested. Let the helper accept a workflow context and add one prompt interpolation assertion.

Proposed test helper and regression
 function ctxFor(
   vertex: PlanVertex,
   inputs: Record<string, unknown> = DEFAULT_INPUTS,
   runOutputs: ReadonlyMap<string, unknown> = NO_OUTPUTS,
+  workflowCtx: Readonly<Record<string, string>> = {},
 ): {
   ctx: NodeExecContext;
   events: NodeStreamEvent[];
 } {
@@
       vertex,
       runOutputs,
       inputs,
-      ctx: {},
+      ctx: workflowCtx,
it('resolves {{ctx.*}} into the agent user prompt', async () => {
  const { provider, req } = reqCapturingProvider();
  const exec = createAgentNodeExecutor(deps(provider));
  const { ctx } = ctxFor(
    vertexFor({
      kind: 'agent',
      node: agentNode({ prompt_template: 'Summarize {{ctx.topic}}: {{inputs.text}}' }),
      resolvedAgent: AGENT,
    }),
    { text: 'the body' },
    NO_OUTPUTS,
    { topic: 'weather' },
  );

  await exec.execute(ctx);

  const userMsg = req()?.messages.find((m) => m.role === 'user');
  const part = userMsg?.content[0];
  expect(part?.type === 'text' ? part.text : undefined).toBe('Summarize weather: the body');
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/engine/agent-runner.test.ts` around lines 115 - 130, The
ctxFor helper function currently hardcodes ctx: {} in the returned
NodeExecContext, which leaves the AgentRunner's context-threading contract
untested for workflow context variables. Modify the ctxFor function signature to
accept an optional fourth parameter for workflow context, replace the hardcoded
ctx: {} assignment with the provided context parameter (defaulting to an empty
object if not provided), and add a new test case that validates the agent prompt
template correctly interpolates {{ctx.*}} variables alongside {{inputs.*}}
variables. The test should verify that a prompt template like 'Summarize
{{ctx.topic}}: {{inputs.text}}' resolves to 'Summarize weather: the body' when
passed context and inputs appropriately.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/engine/engine.ts`:
- Around line 374-383: The beginResume method currently resolves the context
before validating whether the provided gateId is actually pending, which can
cause a valid paused run to be terminally settled as run:failed{validation}
instead of properly handling an invalid resume request. Move the validation of
the gateId parameter (checking whether it is actually pending on the run) to
occur BEFORE the context resolution check in `#resolveContextOrFail`, so that
invalid resume attempts fail fast with the appropriate unknown_gate or
run_not_paused error and delete the half-initialized execution, rather than
settling the run as failed when context resolution fails.

---

Nitpick comments:
In `@packages/core/src/engine/agent-runner.test.ts`:
- Around line 115-130: The ctxFor helper function currently hardcodes ctx: {} in
the returned NodeExecContext, which leaves the AgentRunner's context-threading
contract untested for workflow context variables. Modify the ctxFor function
signature to accept an optional fourth parameter for workflow context, replace
the hardcoded ctx: {} assignment with the provided context parameter (defaulting
to an empty object if not provided), and add a new test case that validates the
agent prompt template correctly interpolates {{ctx.*}} variables alongside
{{inputs.*}} variables. The test should verify that a prompt template like
'Summarize {{ctx.topic}}: {{inputs.text}}' resolves to 'Summarize weather: the
body' when passed context and inputs appropriately.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f2b963b3-ccf2-4df1-b296-2c82560da376

📥 Commits

Reviewing files that changed from the base of the PR and between 7013d49 and 0db2a91.

📒 Files selected for processing (14)
  • CLAUDE.md
  • docs/architecture/execution-model.md
  • docs/roadmap/current.md
  • docs/roadmap/deferred-tasks.md
  • docs/roadmap/phases/phase-1-engine-and-llm.md
  • packages/core/src/engine/agent-runner.test.ts
  • packages/core/src/engine/agent-runner.ts
  • packages/core/src/engine/engine.test.ts
  • packages/core/src/engine/engine.ts
  • packages/core/src/engine/node-executor.ts
  • packages/core/src/engine/node-handlers/human-gate.test.ts
  • packages/core/src/engine/node-handlers/human-gate.ts
  • packages/core/src/engine/node-handlers/node-handlers.test.ts
  • packages/core/src/engine/node-handlers/scope.ts

Comment thread packages/core/src/engine/engine.ts
… review)

Address the PR #23 review.

- beginResume now validates the gate FIRST (non-kick path) — a bad gateId throws
  unknown_gate / run_not_paused (extracted into #assertGatePending, shared with
  resume()) BEFORE the side-effectful context re-resolution. Previously an invalid
  resume request could terminally settle a resumable run as run:failed{validation}
  when context resolution happened to fail; now it fails fast and the caller drops
  the run from #runs (a retry with the correct gateId stays possible). Test: a wrong
  gateId on a run whose context would fail rejects with unknown_gate and persists no
  new events (context resolution never runs).
- agent-runner.test.ts: ctxFor gains a context param; new test asserts the agent
  prompt interpolates {{ctx.*}} alongside {{inputs.*}}
  ('Summarize {{ctx.topic}}: {{inputs.text}}' -> 'Summarize weather: the body') —
  the AgentRunner's ctx-threading contract, previously untested.

Skipped: the "#resolvedContext is readonly" comment — a misread. The field is NOT
`readonly` (only its value type is `Readonly<Record<…>>`); the reassignment
typechecks (CI green). No change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@cemililik
cemililik merged commit 48a9f27 into main Jun 15, 2026
7 checks passed
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.

1 participant