fix: resolve namespace-qualified references in step-level with parame…#317
Merged
fix: resolve namespace-qualified references in step-level with parame…#317
Conversation
eskenazit
force-pushed
the
fix/step-with-namespace-resolution
branch
2 times, most recently
from
4e4e630 to
a6ecc29
Compare
eskenazit
requested changes
Apr 14, 2026
src/test/java/io/naftiko/engine/aggregates/AggregateStepNamespaceIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/main/java/io/naftiko/engine/aggregates/AggregateFunction.java
Outdated
Show resolved
Hide resolved
jlouvel
force-pushed
the
fix/step-with-namespace-resolution
branch
from
a6ecc29 to
342bf86
Compare
Contributor
Author
|
@eskenazit Comments addressed. Could you review again? |
…ters OperationStepExecutor.executeCallStep() passed null as namespace to mergeWithParameters(), so namespace-qualified references like shipyard-tools.voyageId in step-level with blocks were treated as literal strings instead of being resolved to the caller's argument. Propagate the expose namespace from ToolHandler, ResourceHandler, and ResourceRestlet into the step executor. Apply the same fix to AggregateFunction, which also passed null for both function-level and step-level with blocks. Closes #290
Move setExposeNamespace before the orchestrated/simple-call branch so both paths receive the namespace. Add a constructor overload to OperationStepExecutor that accepts the namespace, and update handlers to use it instead of the two-step construct-then-set pattern. Refactor integration test to use a parameter-capturing executor for deterministic assertions.
jlouvel
force-pushed
the
fix/step-with-namespace-resolution
branch
from
11fbf64 to
7f1ab55
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Closes #290
What does this PR do?
Fixes namespace-qualified references (e.g.
shipyard-tools.voyageId) in step-levelwithblocks not being resolved during orchestrated step execution.Root cause:
OperationStepExecutor.executeCallStep()passednullas the namespace tomergeWithParameters(), so thenamespace.paramresolution branch was never entered — values were treated as literal strings instead of being resolved to the caller's argument.Fix (production — 6 files):
exposeNamespacefield +setExposeNamespace()setter; pass it tomergeWithParametersinexecuteCallStep()andfindClientRequestFor(ServerCallSpec, ...)exposeNamespaceon the executor after constructionexposeNamespaceon the executor after constructionexposeNamespaceon the executor after constructionnamespacefield to constructor; pass it tomergeWithParametersfor function-levelwith; setstepExecutor.setExposeNamespace(namespace)before orchestrated executionAggregateFunctionconstructorTests
Unit tests (3):
OperationStepExecutorTest.mergeWithNullNamespaceShouldNotResolveQualifiedReference— documents that null namespace keeps literal stringsOperationStepExecutorBranchTest.executeStepsShouldResolveNamespaceQualifiedReferencesInStepWith— captures parameters via subclass to verify step-level resolutionAggregateFunctionTest.executeShouldResolveNamespaceQualifiedReferencesInFunctionWith— mock-mode function with namespace-qualifiedwithresolves to caller's valueIntegration tests (2):
StepWithNamespaceIntegrationTest— loads MCP capability YAML, verifies namespace-qualified reference in stepwithresolves through full executor chainAggregateStepNamespaceIntegrationTest— loads aggregate capability YAML, executes function with orchestrated steps, verifies namespace resolution in HTTP URLTest fixtures (2):
withaggregate-step-with-namespace.yaml— aggregate function with namespace-qualified stepwithAll 463 tests pass (0 failures, 5 skipped).
Checklist
mainAgent Context (optional)