fix: resolve #793 — Two workflow primitives gaps: role-based context loss and no convergence loops#833
Conversation
…based context loss and no convergence loops Fixes AgentWorkforce#793 Signed-off-by: Dinh Truong (SlncTrZ) <46520299+SlncTrZ@users.noreply.github.com>
…based context loss and no convergence loops Fixes AgentWorkforce#793 Signed-off-by: Dinh Truong (SlncTrZ) <46520299+SlncTrZ@users.noreply.github.com>
…based context loss and no convergence loops Fixes AgentWorkforce#793 Signed-off-by: Dinh Truong (SlncTrZ) <46520299+SlncTrZ@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR introduces workflow execution primitives and two template implementations. It adds TypeScript type definitions for workflow steps and context management, implements a feature-dev template that passes context through sequential planning/development/review phases, and provides a review-loop factory function for iterative review-and-fix workflows. ChangesWorkflow Primitives and Templates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/templates/feature-dev.ts (1)
27-33: ⚡ Quick winAvoid
this-bound method dispatch inrun.If
runis called as a detached function,thiscan be undefined. Prefer explicit object references for stable behavior.Proposed fix
- const plan = await this.planner(ctx); + const plan = await featureDevTemplate.planner(ctx); ctx.set('plan', plan); - const development = await this.developer(ctx); + const development = await featureDevTemplate.developer(ctx); ctx.set('development', development); - const review = await this.reviewer(ctx); + const review = await featureDevTemplate.reviewer(ctx);🤖 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 `@src/templates/feature-dev.ts` around lines 27 - 33, The run method uses this-bound calls (this.planner, this.developer, this.reviewer) which break if run is detached; change run to capture the functions from the instance first or call them via a stable object reference instead of relying on this at call time—e.g., within run assign const planner = this.planner.bind(this) or const { planner, developer, reviewer } = this and then call planner(ctx), developer(ctx), reviewer(ctx), and continue to set ctx with the returned plan/development/review values; update references to planner, developer, reviewer and ctx.set accordingly.
🤖 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 `@src/templates/review-loop.ts`:
- Around line 3-11: Change ReviewStepOutput to a discriminated union so a
failing review must include feedback: replace the current single interface with
a union such that when passed is true feedback is optional, and when passed is
false feedback is required; update any references to ReviewStepOutput (and
ensure AddressFeedbackInput still matches where used) so callers construct the
correct shape for pass vs fail cases.
- Around line 30-33: The loop dereferences ctx.getStepOutput(reviewStep) without
guarding for undefined; change the retrieval to const reviewOut =
ctx.getStepOutput(reviewStep) as ReviewStepOutput | undefined and update the
condition to use optional chaining (e.g., return !(reviewOut?.passed)) so the
loop handles a missing review output consistently with the finalize callback.
---
Nitpick comments:
In `@src/templates/feature-dev.ts`:
- Around line 27-33: The run method uses this-bound calls (this.planner,
this.developer, this.reviewer) which break if run is detached; change run to
capture the functions from the instance first or call them via a stable object
reference instead of relying on this at call time—e.g., within run assign const
planner = this.planner.bind(this) or const { planner, developer, reviewer } =
this and then call planner(ctx), developer(ctx), reviewer(ctx), and continue to
set ctx with the returned plan/development/review values; update references to
planner, developer, reviewer and ctx.set accordingly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a474deb-526b-40ab-9e50-8e52a2e62fe0
📒 Files selected for processing (3)
src/templates/feature-dev.tssrc/templates/review-loop.tssrc/workflow/types.ts
| export interface ReviewStepOutput { | ||
| passed: boolean; | ||
| feedback?: string; | ||
| } | ||
|
|
||
| export interface AddressFeedbackInput { | ||
| code: string; | ||
| feedback: string; | ||
| } |
There was a problem hiding this comment.
Enforce feedback when review fails.
ReviewStepOutput allows passed: false with no feedback, but the next step requires feedback: string. Encode this in the type to prevent invalid loop state.
Proposed fix
-export interface ReviewStepOutput {
- passed: boolean;
- feedback?: string;
-}
+export type ReviewStepOutput =
+ | { passed: true }
+ | { passed: false; feedback: string };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface ReviewStepOutput { | |
| passed: boolean; | |
| feedback?: string; | |
| } | |
| export interface AddressFeedbackInput { | |
| code: string; | |
| feedback: string; | |
| } | |
| export type ReviewStepOutput = | |
| | { passed: true } | |
| | { passed: false; feedback: string }; | |
| export interface AddressFeedbackInput { | |
| code: string; | |
| feedback: string; | |
| } |
🤖 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 `@src/templates/review-loop.ts` around lines 3 - 11, Change ReviewStepOutput to
a discriminated union so a failing review must include feedback: replace the
current single interface with a union such that when passed is true feedback is
optional, and when passed is false feedback is required; update any references
to ReviewStepOutput (and ensure AddressFeedbackInput still matches where used)
so callers construct the correct shape for pass vs fail cases.
| while: (ctx: StepContext) => { | ||
| const reviewOut = ctx.getStepOutput(reviewStep) as ReviewStepOutput; | ||
| return !reviewOut.passed; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @agentworkforce/relay, is LoopStep's whilepredicate evaluated before the first iteration, and what doesStepContext.getStepOutput(step) return when that step has not executed yet?
💡 Result:
I couldn’t find any authoritative source in the AgentWorkforce/relay repo or its published docs that defines (1) the evaluation timing of LoopStep’s while predicate (before first iteration vs after) or (2) what StepContext.getStepOutput(step) returns prior to that step executing. The search results I retrieved only cover high-level workflow/run concepts and a generic note about output availability when dependsOn is missing (i.e., output “won’t be available yet”), but they do not mention LoopStep, while, StepContext, or getStepOutput semantics for the “not executed yet” case. If you can share the exact file/class name where LoopStep and StepContext are implemented in your version of @agentworkforce/relay (or paste the relevant code), I can answer precisely.
Citations:
- 1: https://github.com/AgentWorkforce/relay/blob/main/packages/sdk/README.md
- 2: https://agentrelay.dev/docs/reference-workflows
- 3: https://github.com/AgentWorkforce/skills/blob/main/skills/writing-agent-relay-workflows/SKILL.md
- 4: https://github.com/AgentWorkforce/relay
- 5: https://github.com/AgentWorkforce
🏁 Script executed:
find . -type f -name "*.ts" -o -name "*.tsx" | head -20Repository: AgentWorkforce/relay
Length of output: 934
🏁 Script executed:
# Search for LoopStep and StepContext definitions
rg "class LoopStep|interface LoopStep|type LoopStep" -A 10Repository: AgentWorkforce/relay
Length of output: 1414
🏁 Script executed:
# Search for StepContext definition
rg "class StepContext|interface StepContext|type StepContext" -A 10Repository: AgentWorkforce/relay
Length of output: 553
🏁 Script executed:
# Search for getStepOutput method
rg "getStepOutput" -B 2 -A 5Repository: AgentWorkforce/relay
Length of output: 6625
🏁 Script executed:
# Look at the review-loop.ts file context
cat -n src/templates/review-loop.tsRepository: AgentWorkforce/relay
Length of output: 1598
🏁 Script executed:
# Find LoopStep implementation (class, not just interface)
rg "class LoopStep|export.*LoopStep.*=" -A 20Repository: AgentWorkforce/relay
Length of output: 46
🏁 Script executed:
# Check for LoopStep in node_modules or dist
find . -name "*loop*" -type f | grep -E "\.(ts|js)$"Repository: AgentWorkforce/relay
Length of output: 508
🏁 Script executed:
# Look at the imports in review-loop.ts to find where LoopStep comes from
head -5 src/templates/review-loop.tsRepository: AgentWorkforce/relay
Length of output: 210
🏁 Script executed:
# Search for when conditions/while loops are evaluated in execution logic
rg "while.*condition|condition.*while|evaluateLoop|executeLoop" -B 3 -A 5Repository: AgentWorkforce/relay
Length of output: 1870
🏁 Script executed:
# Search for LoopStep implementation (likely in relay package)
rg "export.*class LoopStep" -A 30Repository: AgentWorkforce/relay
Length of output: 46
🏁 Script executed:
# Look for LoopStep in the relay package specifically
fd ".*loop.*" packages/ --type f -e ts -e tsx | head -20Repository: AgentWorkforce/relay
Length of output: 104
🏁 Script executed:
# Check what's exported from the main relay package
cat packages/relay/src/index.ts 2>/dev/null || cat packages/sdk/src/index.ts 2>/dev/null | grep -i loopRepository: AgentWorkforce/relay
Length of output: 46
🏁 Script executed:
# Search for usage of while predicate in loop execution
rg "\.while\s*\(" -B 5 -A 10Repository: AgentWorkforce/relay
Length of output: 46
🏁 Script executed:
# Look at the actual LoopStep type more carefully - check all files with "loop" in workflow
rg "while.*StepContext|condition.*StepContext" -B 2 -A 5Repository: AgentWorkforce/relay
Length of output: 1021
🏁 Script executed:
# Look at ralph-loop example for LoopStep usage
cat packages/sdk/src/examples/ralph-loop.tsRepository: AgentWorkforce/relay
Length of output: 12710
🏁 Script executed:
# Check for LoopStep tests
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "LoopStep" 2>/dev/nullRepository: AgentWorkforce/relay
Length of output: 48
🏁 Script executed:
# Look for StepContext interface extensions or actual implementation
rg "getStepOutput" -B 5 -A 3Repository: AgentWorkforce/relay
Length of output: 7255
🏁 Script executed:
# Check package.json to understand relay dependencies
cat package.json | grep -A 5 "@agentworkforce"Repository: AgentWorkforce/relay
Length of output: 46
🏁 Script executed:
# Look at how ctx parameter is used in loops - check the actual LoopStep implementation in relay module
find . -path ./node_modules -prune -o -name "*.ts" -type f -exec grep -l "class.*LoopStep\|interface.*LoopStep" {} \;Repository: AgentWorkforce/relay
Length of output: 140
🏁 Script executed:
# Search for LoopStep class/implementation in relay module
rg "new LoopStep" -B 5 -A 15Repository: AgentWorkforce/relay
Length of output: 3891
🏁 Script executed:
# Look for where LoopStep is actually defined/implemented
find . -path ./node_modules/@agentworkforce -name "*.ts" -o -path ./node_modules/@agentworkforce -name "*.js" | head -20Repository: AgentWorkforce/relay
Length of output: 46
🏁 Script executed:
# Check what's in `@agentworkforce/relay` node_modules
ls -la node_modules/@agentworkforce 2>/dev/null || echo "Module not installed or not in node_modules"Repository: AgentWorkforce/relay
Length of output: 108
🏁 Script executed:
# Look at the actual StepContext definition more carefully - maybe it's extended somewhere
rg "interface StepContext|type StepContext" -A 15Repository: AgentWorkforce/relay
Length of output: 832
🏁 Script executed:
# Check for implementations that show when while/condition is evaluated
rg "executeLoop|runLoop|loopBody|while.*steps" -B 3 -A 10Repository: AgentWorkforce/relay
Length of output: 4517
🏁 Script executed:
# Search for where StepContext is extended with getStepOutput method
rg "StepContext.*getStepOutput|getStepOutput.*StepContext" -B 5 -A 5Repository: AgentWorkforce/relay
Length of output: 46
🏁 Script executed:
# Look for Step generic type definition to understand the actual types involved
rg "type Step<|interface Step<" -A 5Repository: AgentWorkforce/relay
Length of output: 46
🏁 Script executed:
# Check if there's a compiled version or dist folder with implementation
find . -name "*.d.ts" -path "*/relay/*" | head -10Repository: AgentWorkforce/relay
Length of output: 46
🏁 Script executed:
# Look at line 36 more carefully - it already uses | undefined, check what's different
sed -n '28,41p' src/templates/review-loop.tsRepository: AgentWorkforce/relay
Length of output: 648
🏁 Script executed:
# Search for how ctx parameter is used in other while/condition predicates
rg "\.while\s*:\s*\(|condition:\s*\(" -A 5 -B 2Repository: AgentWorkforce/relay
Length of output: 439
🏁 Script executed:
# Look at the full LoopStep interface definition and search for any implementation hints
cat src/workflow/types.tsRepository: AgentWorkforce/relay
Length of output: 552
🏁 Script executed:
# Search for tests or usage examples that show when while/condition is evaluated
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "LoopStep\|while.*condition" 2>/dev/nullRepository: AgentWorkforce/relay
Length of output: 48
🏁 Script executed:
# Check if there's documentation or comments about loop execution timing
rg "before.*iteration|first.*iteration|while.*evaluated" -i -B 2 -A 2Repository: AgentWorkforce/relay
Length of output: 46
🏁 Script executed:
# Look at the finalize function - it suggests the context after loop completes, so while would be evaluated after first iteration
rg "finalize" -B 3 -A 5Repository: AgentWorkforce/relay
Length of output: 14284
Guard loop condition against missing review output.
ctx.getStepOutput(reviewStep) is dereferenced without checking for undefined. The finalize callback (line 36) already applies defensive coding for the same method with | undefined, indicating the output can be absent. Apply the same guard to maintain consistency.
Proposed fix
while: (ctx: StepContext) => {
- const reviewOut = ctx.getStepOutput(reviewStep) as ReviewStepOutput;
- return !reviewOut.passed;
+ const reviewOut = ctx.getStepOutput(reviewStep) as ReviewStepOutput | undefined;
+ return reviewOut ? !reviewOut.passed : true;
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| while: (ctx: StepContext) => { | |
| const reviewOut = ctx.getStepOutput(reviewStep) as ReviewStepOutput; | |
| return !reviewOut.passed; | |
| }, | |
| while: (ctx: StepContext) => { | |
| const reviewOut = ctx.getStepOutput(reviewStep) as ReviewStepOutput | undefined; | |
| return reviewOut ? !reviewOut.passed : true; | |
| }, |
🤖 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 `@src/templates/review-loop.ts` around lines 30 - 33, The loop dereferences
ctx.getStepOutput(reviewStep) without guarding for undefined; change the
retrieval to const reviewOut = ctx.getStepOutput(reviewStep) as ReviewStepOutput
| undefined and update the condition to use optional chaining (e.g., return
!(reviewOut?.passed)) so the loop handles a missing review output consistently
with the finalize callback.
1 participant
Summary
fix: resolve #793 — Two workflow primitives gaps: role-based context loss and no convergence loops
Problem
Severity:
High| File:src/workflow/types.tsThis file currently defines core types (WorkflowStep, Workflow, etc.). We need to extend it to support structured context propagation and the new loop primitive. The existing WorkflowStep interface is modified to accept and return StepContext instead of plain strings. A new LoopStep interface is added to model iterative execution.
Solution