Role-binding layer: runner + prompts -> Actor/Critic (Task 13)#4
Conversation
Implements Milestone 2, Task 13 from the loop-engine spec: a vendor-neutral seam that pairs any AgentRunner with role prompt templates to produce working Actor/Critic roles, wired through configuration. - prompts.ts: actor (draftPlan/build/selfReview) and critic (plan/task review) prompt templates with the rubric encoded, plus actor JSON wire contracts. - runnerRoles.ts: RunnerActor parses structured plan/build output (retry-once then RunnerRoleError); RunnerCritic and actor selfReview pass raw text through so the engine owns parsing + rubric enforcement. - jsonExtract.ts: shared last-valid-JSON scanner; criticParser refactored onto it so actor and critic outputs parse identically. - runnerFactory.ts + mockRunner.ts: profile->runner dispatch (http deferred to Task 14 with a clear error) and a deterministic runner for tests. - config.ts: runner profiles (LOOPWRIGHT_RUNNERS) + role bindings (LOOPWRIGHT_ACTOR_RUNNER/_CRITIC_RUNNER); roleBindings.createRoles resolves them and fails fast on misconfiguration, with per-role model override. Adds 24 tests (78 total); typecheck clean.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/adapters/prompts.ts`:
- Around line 169-202: The plan contract and schema disagree:
ACTOR_PLAN_CONTRACT requires each task to have at least one machine-checkable
verifyCommand, but TaskSpecSchema.verifyCommands currently defaults to [] (no
.min(1)), which lets runMechanicalGate([]) vacuously pass; fix by enforcing the
contract in the schema or aligning the contract to behavior. Update
TaskSpecSchema.verifyCommands to require at least one entry (e.g., use
z.array(...).min(1) or add an equivalent refinement/validation step that throws
when verifyCommands is empty) so TaskSpecSchema, ACTOR_PLAN_CONTRACT, and
runMechanicalGate behavior are consistent; alternatively if you prefer changing
the prompt, edit ACTOR_PLAN_CONTRACT to allow empty verifyCommands and document
that mechanical gate treats empty lists as skipped. Reference symbols to change:
TaskSpecSchema.verifyCommands, ACTOR_PLAN_CONTRACT, and consider
runMechanicalGate behavior if you opt to keep empty arrays.
🪄 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: 7e15be86-eca6-4172-8079-843dc1608098
📒 Files selected for processing (12)
.kiro/specs/loop-engine/tasks.mdsrc/adapters/prompts.tssrc/adapters/roleBindings.tssrc/adapters/runnerRoles.tssrc/config.tssrc/engine/criticParser.tssrc/engine/jsonExtract.tssrc/runners/agentRunner.tssrc/runners/mockRunner.tssrc/runners/runnerFactory.tstest/roleBindings.test.tstest/runnerRoles.test.ts
| const ACTOR_PLAN_CONTRACT = | ||
| 'Reply with ONLY a JSON object (no prose, no markdown fences) matching:\n' + | ||
| '{"tasks":[{"id":string,"title":string,"description":string,' + | ||
| '"acceptanceCriteria":[string,...],"verifyCommands":[string,...],' + | ||
| '"dependencies":[string,...]}],"notes":string}\n' + | ||
| "Rules: ids are unique and short; every task has at least one acceptance " + | ||
| "criterion AND at least one machine-checkable verifyCommand (build/test/lint) " + | ||
| "so its done-state is verifiable; dependencies reference other task ids only."; | ||
|
|
||
| const ACTOR_BUILD_CONTRACT = | ||
| 'Reply with ONLY a JSON object (no prose, no markdown fences) matching:\n' + | ||
| '{"diff":string,"touchedFiles":[string,...],"summary":string}\n' + | ||
| "`diff` is a unified diff of your changes; `touchedFiles` lists the paths it " + | ||
| "changes; `summary` is one or two sentences. Do not include secrets."; | ||
|
|
||
| export const DEFAULT_ACTOR_PROMPTS: ActorPromptTemplates = { | ||
| system: | ||
| "You are the Actor in an actor-critic engineering loop: the high-volume " + | ||
| "worker that decomposes goals into verifiable tasks, builds each task, and " + | ||
| "fixes issues from feedback. You are vendor-neutral and reply with strict " + | ||
| "JSON when asked. Favor small, verifiable units of work.", | ||
|
|
||
| draftPlan(goal, blockers) { | ||
| const revision = | ||
| blockers && blockers.length > 0 | ||
| ? `\n\nThis is a REVISION. The critic blocked the previous plan; ` + | ||
| `resolve every blocker below:\n${renderFindings(blockers)}` | ||
| : ""; | ||
| return ( | ||
| `Decompose this goal into a plan of dependent tasks.\n\nGOAL: ${goal}` + | ||
| revision + | ||
| `\n\n${ACTOR_PLAN_CONTRACT}` | ||
| ); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify TaskSpecSchema validation requirements for verifyCommands
rg -A 10 'export const TaskSpecSchema' src/schemas/plan.tsRepository: inhaq/loopwright
Length of output: 418
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find ActorPlanOutputSchema definition and how it validates TaskSpecSchema
rg -n --hidden --glob '!**/node_modules/**' "ActorPlanOutputSchema|TaskSpecSchema" src | head -n 200
# Show the ActorPlanOutputSchema block (up to ~80 lines around matches)
rg -n --hidden --glob '!**/node_modules/**' -C 5 "ActorPlanOutputSchema" src/schemas/plan.ts src/schemas/**/*.ts 2>/dev/null || true
# Show where the plan output is parsed/validated (zod .parse / .safeParse)
rg -n --hidden --glob '!**/node_modules/**' "\.parse\(|safeParse\(" src | rg -n "ActorPlanOutputSchema|plan|draftPlan|ActorPlan" | head -n 200Repository: inhaq/loopwright
Length of output: 993
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print ActorPlanOutputSchema definition
sed -n '1,140p' src/adapters/prompts.ts
# Print PlanSchema/related schemas in src/schemas/plan.ts
sed -n '1,120p' src/schemas/plan.ts
# Print relevant part of src/schemas/artifact.ts (imports TaskSpecSchema)
sed -n '1,120p' src/schemas/artifact.ts
# Print where ActorPlanOutputSchema and PlanSchema.parse are called
sed -n '1,160p' src/adapters/runnerRoles.tsRepository: inhaq/loopwright
Length of output: 13510
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where mechanicalGate is produced and where verifyCommands is used
rg -n --hidden --glob '!**/node_modules/**' "verifyCommands|mechanicalGate|testCommands" src | head -n 200
# Specifically look for iteration/execution of verifyCommands
rg -n --hidden --glob '!**/node_modules/**' "verifyCommands.*(map|forEach|for \\(|reduce|push|command)" src
# Show the most relevant implementation files around mechanical gate execution
rg -n --hidden --glob '!**/node_modules/**' "mechanicalGate" src | head -n 50Repository: inhaq/loopwright
Length of output: 1895
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' src/engine/loop.ts
# Print mechanical gate implementation (file name guessed from import path)
ls -la src/engine | rg -n "mechanicalGate" || true
fd mechanicalGate src/engine -t f -x echo
# Show the mechanical gate runner file
MECH_FILE=$(fd mechanicalGate src/engine -t f | head -n 1)
echo "Using: $MECH_FILE"
sed -n '1,260p' "$MECH_FILE"Repository: inhaq/loopwright
Length of output: 13303
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for verifyCommands in critic-related files
rg -n --hidden --glob '!**/node_modules/**' "verifyCommands" src/adapters src/engine src/schemas | head -n 200
# Show critic prompt section for plan review (likely DEFAULT_CRITIC_PROMPTS draftPlan/review)
rg -n --hidden --glob '!**/node_modules/**' "critic.*plan|PlanSchema|plan draft|draft" src/adapters/prompts.ts src/adapters/runnerRoles.ts src/adapters | head -n 200
# If prompts.ts includes critic contract mention verifyCommands, print around it
rg -n --hidden --glob '!**/node_modules/**' "acceptanceCriteria|dependencies|verifyCommands" src/adapters/prompts.ts | head -n 200Repository: inhaq/loopwright
Length of output: 3914
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --glob '!**/node_modules/**' "verifyCommands" src/adapters src/engine src/schemas | head -n 200
rg -n --hidden --glob '!**/node_modules/**' "critic.*plan|PlanSchema|plan draft|draftPlan|review.*plan" src/adapters/prompts.ts src/adapters/runnerRoles.ts src/adapters | head -n 200
rg -n --hidden --glob '!**/node_modules/**' "acceptanceCriteria|dependencies|verifyCommands" src/adapters/prompts.ts | head -n 200Repository: inhaq/loopwright
Length of output: 3521
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect critic prompt template(s) in prompts.ts around where verifyCommands appears
sed -n '120,320p' src/adapters/prompts.ts
# Inspect critic schema (normalization + blockers/rubric enforcement)
sed -n '1,260p' src/schemas/critic.ts
# Inspect critic response parsing
rg -n "parseCriticResponse|REPAIR_HINT|criticParser" -S src/engine
sed -n '1,260p' src/engine/criticParser.ts
# Search for logic that checks missing verifyCommands explicitly
rg -n --hidden --glob '!**/node_modules/**' "verifyCommands.*(block|fail|reject)|verifyCommands.*min\\(|machine-checkable|DoD" src | head -n 200Repository: inhaq/loopwright
Length of output: 12003
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the rest of runTask switch handling of review.kind, especially "unavailable"/"malformed"/"paused"
rg -n "switch \\(review\\.kind\\)" -n src/engine/loop.ts
sed -n '240,460p' src/engine/loop.ts
# Inspect plan review stage too (where critic.review kind "plan")
sed -n '300,420p' src/engine/loop.tsRepository: inhaq/loopwright
Length of output: 9040
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "UNVERIFIED|unverified|UNVERIFIED_BY_CRITIC" src/engine src/domain src | head -n 200
sed -n '1,260p' src/domain/stateMachine.tsRepository: inhaq/loopwright
Length of output: 4963
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "UNVERIFIED|unverified|UNVERIFIED_BY_CRITIC" src/engine src/domain src | head -n 200
sed -n '1,260p' src/domain/stateMachine.tsRepository: inhaq/loopwright
Length of output: 4963
Fix contract/schema mismatch for verifyCommands in plan tasks.
ACTOR_PLAN_CONTRACT (src/adapters/prompts.ts) says every task must have at least one machine-checkable verifyCommand, but TaskSpecSchema.verifyCommands (src/schemas/plan.ts) allows [] (z.array(...).default([]) with no .min(1)). Since runMechanicalGate([]) returns { passed: true } (src/engine/mechanicalGate.ts), an empty verifyCommands makes the mechanical “done-state” vacuously pass and relies entirely on the critic’s semantic checks rather than validation.
Consider enforcing verifyCommands length in the schema (or adding a refinement where the plan is validated), or updating the prompt contract to match the actual behavior.
🤖 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/adapters/prompts.ts` around lines 169 - 202, The plan contract and schema
disagree: ACTOR_PLAN_CONTRACT requires each task to have at least one
machine-checkable verifyCommand, but TaskSpecSchema.verifyCommands currently
defaults to [] (no .min(1)), which lets runMechanicalGate([]) vacuously pass;
fix by enforcing the contract in the schema or aligning the contract to
behavior. Update TaskSpecSchema.verifyCommands to require at least one entry
(e.g., use z.array(...).min(1) or add an equivalent refinement/validation step
that throws when verifyCommands is empty) so TaskSpecSchema,
ACTOR_PLAN_CONTRACT, and runMechanicalGate behavior are consistent;
alternatively if you prefer changing the prompt, edit ACTOR_PLAN_CONTRACT to
allow empty verifyCommands and document that mechanical gate treats empty lists
as skipped. Reference symbols to change: TaskSpecSchema.verifyCommands,
ACTOR_PLAN_CONTRACT, and consider runMechanicalGate behavior if you opt to keep
empty arrays.
This pull request was created by @kiro-agent on behalf of @inhaq 👻
Comment with /kiro fix to address specific feedback or /kiro all to address everything.
Learn about Kiro Web
Summary
Implements Milestone 2, Task 13 from
.kiro/specs/loop-engine— the role-binding layer that turns a vendor-neutralAgentRunnerplus role prompt templates into workingActor/Criticroles, wired through configuration. This is the seam that lets any backend (the existingCliRunner, the upcomingHttpRunner) drive the Milestone 1 loop with zero engine changes.What's included
draftPlan/build(with prior-failure context fed forward) / degradedselfReview, plus the JSON wire contracts for plan and build output.LOOPWRIGHT_RUNNERS(JSON) and role bindings viaLOOPWRIGHT_ACTOR_RUNNER/LOOPWRIGHT_CRITIC_RUNNER;createRoles()resolves them and fails fast on misconfiguration (naming available profile ids), with an optional per-role model override.Design notes
RunnerActorparses structured plan/build output and retries once with a corrective nudge before throwingRunnerRoleError. The critic review and the actor's self-review return raw text so the engine keeps sole ownership of parsing + rubric enforcement.engine/jsonExtract.ts) and refactoredcriticParseronto it, so actor and critic outputs parse through one code path.runnerFactorydispatches profilekind→ runner;httpthrows a clear "not implemented yet (Task 14)" error rather than degrading silently. Added a deterministicMockRunnerfor tests.Testing
npm run typecheckclean.npm test: 78 passing (24 new acrosstest/runnerRoles.test.tsandtest/roleBindings.test.ts), covering parse/retry/quota handling, feedback-in-prompt, raw passthrough, factory dispatch, config parsing, binding errors, and two end-to-end runs that drive the real loop (plan review + per-task fix cycle) to a verified GREEN.Not in scope
HttpRunner(Task 14) and the real end-to-end run against live backends (Task 15).Summary by CodeRabbit
New Features
Documentation
Tests