Milestone 2: HttpRunner + end-to-end session (Tasks 14, 15)#5
Merged
Conversation
Task 14 - HttpRunner: vendor-neutral runner for OpenAI-compatible chat endpoints behind the AgentRunner interface. Profile-driven (baseUrl, path, model from profile, env-referenced API key + headers, temperature/maxTokens, extraBody), with HTTP-status + body-pattern quota detection, AbortController timeout, bounded output capture, and strict option validation. Transport is injectable for testing; transport failures resolve to a diagnostic result rather than throwing. Registered in runnerFactory (http is now built). Task 15 - session.runGoal: headless end-to-end entrypoint wiring config -> createRoles -> plan review -> per-task loop, executing tasks in dependency order and skipping dependents of unsatisfied prerequisites. Adds src/run.ts CLI (npm start -- "goal") driven entirely by config. Adds 12 tests (91 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 |
![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
🤖 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/runners/httpRunner.ts`:
- Line 63: HttpRunner's options schema currently allows any string for
quota.pattern so invalid regexes slip past construction and only throw inside
run(); change construction to validate and precompile quota.pattern: in the
constructor (where HttpRunnerOptionsSchema.parse(...) is called) attempt to new
RegExp(this.opts.quota.pattern, "i") if a pattern is present, store the compiled
RegExp on the instance (e.g., this.quotaPatternRegex) and surface any syntax
error so construction fails fast; then update HttpRunner.run() (which currently
does new RegExp(this.opts.quota.pattern, "i") on non-ok responses) to use the
precompiled this.quotaPatternRegex (and skip matching if undefined) so runtime
exceptions are avoided and invalid patterns are caught at startup.
In `@src/session.ts`:
- Around line 57-76: dependencyOrder silently ignores cycles by returning early
in the inner visit when stack.has(task.id); change this to surface an explicit
error instead of dropping edges: in dependencyOrder (function dependencyOrder
and inner visit) detect a back-edge via stack.has(task.id) and throw a
descriptive Error (or a custom Error type) that includes the cycle (e.g.,
reconstruct the cycle path from the stack plus task.id) so callers know the plan
is invalid; keep the rest of the DFS logic and ordering behavior the same so
acyclic inputs still return a stable topological order.
🪄 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: 0a80ef54-526b-4c4c-9e60-634883fc8bd3
📒 Files selected for processing (9)
.kiro/specs/loop-engine/tasks.mdpackage.jsonsrc/run.tssrc/runners/httpRunner.tssrc/runners/runnerFactory.tssrc/session.tstest/httpRunner.test.tstest/roleBindings.test.tstest/session.test.ts
| /** HTTP status codes that indicate an exhausted usage/rate window */ | ||
| statusCodes: z.array(z.number().int()).default([429]), | ||
| /** case-insensitive regex tested against an error response body */ | ||
| pattern: z.string().optional(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification:
# 1) Confirm schema currently accepts unvalidated regex strings.
# 2) Confirm runtime compilation happens in the hot error path.
rg -n 'pattern:\s*z\.string\(\)\.optional\(\)' src/runners/httpRunner.ts
rg -n 'new RegExp\(this\.opts\.quota\.pattern,\s*"i"\)' src/runners/httpRunner.tsRepository: inhaq/loopwright
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print schema and surrounding area
nl -ba src/runners/httpRunner.ts | sed -n '40,95p'
# Print the non-ok/error handling and regex usage area
nl -ba src/runners/httpRunner.ts | sed -n '120,190p'Repository: inhaq/loopwright
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Schema area (lines 40-95) ==="
cat -n src/runners/httpRunner.ts | sed -n '40,95p'
echo "=== Error/quota area (lines 120-190) ==="
cat -n src/runners/httpRunner.ts | sed -n '120,190p'Repository: inhaq/loopwright
Length of output: 5341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/runners/httpRunner.ts | sed -n '190,270p'Repository: inhaq/loopwright
Length of output: 2059
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/runners/httpRunner.ts | sed -n '190,270p'Repository: inhaq/loopwright
Length of output: 2059
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/runners/httpRunner.ts | sed -n '95,140p'Repository: inhaq/loopwright
Length of output: 2243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/runners/httpRunner.ts | sed -n '95,140p'Repository: inhaq/loopwright
Length of output: 2243
Validate and precompile quota.pattern during construction.
HttpRunnerOptionsSchema.parse(...) runs in the constructor, but quota.pattern is only z.string().optional() (Line 63), so invalid regexes pass startup validation. In run(), the runner compiles new RegExp(this.opts.quota.pattern, "i") (Line 152) on every non-res.ok response; if the regex is invalid, that exception is thrown inside the try and is handled by the catch, returning text: "" / quotaExhausted: false instead of the normal non-OK response meta.
Suggested fix
export const HttpRunnerOptionsSchema = z
.object({
@@
- pattern: z.string().optional(),
+ pattern: z
+ .string()
+ .optional()
+ .refine(
+ (p) => {
+ if (p === undefined) return true;
+ try {
+ new RegExp(p, "i");
+ return true;
+ } catch {
+ return false;
+ }
+ },
+ "quota.pattern must be a valid regular expression",
+ ),
@@
export class HttpRunner implements AgentRunner {
@@
+ private readonly quotaPattern?: RegExp;
@@
constructor(profile: RunnerProfile, deps: HttpRunnerDeps = {}) {
@@
+ this.quotaPattern =
+ this.opts.quota.pattern !== undefined
+ ? new RegExp(this.opts.quota.pattern, "i")
+ : undefined;
@@
if (!res.ok) {
const errText = await safeText(res);
+ const quotaProbe = errText.slice(0, 8_000);
const quotaExhausted =
this.opts.quota.statusCodes.includes(res.status) ||
- (this.opts.quota.pattern !== undefined &&
- new RegExp(this.opts.quota.pattern, "i").test(errText));
+ (this.quotaPattern !== undefined && this.quotaPattern.test(quotaProbe));🤖 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/runners/httpRunner.ts` at line 63, HttpRunner's options schema currently
allows any string for quota.pattern so invalid regexes slip past construction
and only throw inside run(); change construction to validate and precompile
quota.pattern: in the constructor (where HttpRunnerOptionsSchema.parse(...) is
called) attempt to new RegExp(this.opts.quota.pattern, "i") if a pattern is
present, store the compiled RegExp on the instance (e.g.,
this.quotaPatternRegex) and surface any syntax error so construction fails fast;
then update HttpRunner.run() (which currently does new
RegExp(this.opts.quota.pattern, "i") on non-ok responses) to use the precompiled
this.quotaPatternRegex (and skip matching if undefined) so runtime exceptions
are avoided and invalid patterns are caught at startup.
Source: Linters/SAST tools
Comment on lines
+57
to
+76
| /** Orders tasks so dependencies precede dependents (stable; assumes acyclic). */ | ||
| function dependencyOrder(tasks: TaskSpec[]): TaskSpec[] { | ||
| const byId = new Map(tasks.map((t) => [t.id, t])); | ||
| const visited = new Set<string>(); | ||
| const ordered: TaskSpec[] = []; | ||
| const visit = (task: TaskSpec, stack: Set<string>): void => { | ||
| if (visited.has(task.id)) return; | ||
| if (stack.has(task.id)) return; // cycle guard; engine validates elsewhere | ||
| stack.add(task.id); | ||
| for (const depId of task.dependencies) { | ||
| const dep = byId.get(depId); | ||
| if (dep) visit(dep, stack); | ||
| } | ||
| stack.delete(task.id); | ||
| visited.add(task.id); | ||
| ordered.push(task); | ||
| }; | ||
| for (const t of tasks) visit(t, new Set()); | ||
| return ordered; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for cycle detection/validation in plan schema and engine
rg -n -C3 'cycle|acyclic|circular' --type=ts -g '!test' -g '!node_modules'Repository: inhaq/loopwright
Length of output: 5440
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find where dependencyOrder is used (to see if callers validate first)
rg -n "dependencyOrder\(" -S src
# 2) Search for any real acyclic/cycle validation logic in plan/schema/engine
rg -n -S "acyclic|circular|cycle guard|topolog|topolog|topological|dependencies.*acyclic|graph.*cycle|detect.*cycle" src
# 3) Search for task dependency normalization/validation
rg -n -S "dependencies.*(validate|coherent|check|schema|normalize|ordering)|TaskSpec" src/schemas src/domain src/engine src/session.ts
# 4) Inspect plan schema files that mention dependencies
ls -1 src/schemas || true
rg -n -S "dependencies|TaskSpec|dependency" src/schemasRepository: inhaq/loopwright
Length of output: 1709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show plan schema for any refinement around dependencies/acyclicity
echo "---- src/schemas/plan.ts (head) ----"
sed -n '1,140p' src/schemas/plan.ts
echo "---- src/session.ts (relevant sections) ----"
sed -n '1,220p' src/session.ts
echo "---- src/engine/loop.ts (search for dependencies/order usage) ----"
rg -n "dependencies|dependencyOrder|topolog|acyclic|cycle" src/engine/loop.tsRepository: inhaq/loopwright
Length of output: 6199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find runPlanReview implementation
rg -n "function runPlanReview|runPlanReview\s*\(" -S src
# Show the portion of src/engine/loop.ts that defines runPlanReview and plan validation
ls -1 src/engine
sed -n '1,260p' src/engine/loop.ts
sed -n '260,520p' src/engine/loop.ts
# Search for any cycle/acyclic validation logic beyond prompt text
rg -n -S "acyclic|circular|cycle|topolog|topological|graph.*cycle|detect.*cycle" srcRepository: inhaq/loopwright
Length of output: 15548
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -S "acyclic|circular|coherent|dependency(s)? .*coher|DAG|topolog" src
# Inspect critic schema/normalization
sed -n '1,220p' src/schemas/critic.ts
rg -n "normalizeReview|blockers as|nits as" src/schemas/critic.ts
sed -n '220,520p' src/schemas/critic.ts
# Inspect plan-review prompt wording that mentions acyclic dependencies
sed -n '200,280p' src/adapters/prompts.tsRepository: inhaq/loopwright
Length of output: 5375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for Zod refinements anywhere
rg -n "refine\(|superRefine\(" src/schemas
# Look for explicit dependency/cycle validation logic
rg -n -S "cycle|acyclic|circular|topolog|topological|graph|DAG|detect.*cycle" src
# Inspect critic JSON contract parsing/validation
rg -n "CRITIC_JSON_CONTRACT|parseCriticResponse|jsonExtract" -S src/engine src/adapters src/schemas
# Find where draftPlan parses/validates plan JSON
rg -n "draftPlan|parse.*plan|PlanSchema|TaskSpecSchema" src/engine src/adapters srcRepository: inhaq/loopwright
Length of output: 42
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Where PlanSchema/TaskSpecSchema are actually used/parsed
rg -n -S "PlanSchema\.|TaskSpecSchema\.|\.safeParse\(|\.parse\(" src
# 2) Find actor.draftPlan implementations (where plan JSON is produced/validated)
rg -n -S "draftPlan\(" src
# 3) Search for any dependency coherence/cycle logic outside prompts/session
rg -n -S "dependencies|dependencyOrder|blockedBy|acyclic|circular|cycle guard|topolog|topological|DAG|detect.*cycle" srcRepository: inhaq/loopwright
Length of output: 3060
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/adapters/runnerRoles.ts
echo "----"
sed -n '220,520p' src/adapters/runnerRoles.ts
echo "---- src/engine/jsonExtract.ts ----"
sed -n '1,220p' src/engine/jsonExtract.ts
echo "---- src/adapters/mocks.ts ----"
sed -n '1,200p' src/adapters/mocks.ts
echo "---- src/adapters/agents.ts (draftPlan interface) ----"
sed -n '1,120p' src/adapters/agents.tsRepository: inhaq/loopwright
Length of output: 18063
Fix dependency-cycle handling in dependencyOrder (don’t silently accept cycles)
src/session.ts’sdependencyOrder()cycle guard (if (stack.has(task.id)) return;) silently drops cycle edges, which can lead to incomplete ordering and cascading task skips.- Upstream plan parsing doesn’t enforce acyclicity:
src/adapters/runnerRoles.tsvalidates viaPlanSchema.parse(...)/TaskSpecSchema(structure only; no dependency graph checks/refinements). - The only “acyclic” enforcement visible is a prompt instruction to the critic, which is not a machine-checked DAG validation—so cyclic plans can reach scheduling without a clear error.
🤖 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/session.ts` around lines 57 - 76, dependencyOrder silently ignores cycles
by returning early in the inner visit when stack.has(task.id); change this to
surface an explicit error instead of dropping edges: in dependencyOrder
(function dependencyOrder and inner visit) detect a back-edge via
stack.has(task.id) and throw a descriptive Error (or a custom Error type) that
includes the cycle (e.g., reconstruct the cycle path from the stack plus
task.id) so callers know the plan is invalid; keep the rest of the DFS logic and
ordering behavior the same so acyclic inputs still return a stable topological
order.
This was referenced Jun 13, 2026
…pendency cycles - HttpRunner: refine quota.pattern to a valid regex at schema parse time and precompile it once in the constructor, so an invalid pattern fails fast at construction instead of throwing inside run()'s try/catch (where it was misclassified as a transport failure). - session.dependencyOrder: throw a descriptive error naming the cycle instead of silently dropping the back-edge (which produced a partial order and cascading skips).
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
1 participant
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.
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
Completes Milestone 2 (Runners) from
.kiro/specs/loop-engine— Tasks 14 and 15.Task 14 —
HttpRunnerVendor-neutral runner for OpenAI-compatible chat endpoints, behind the same
AgentRunnerinterface. Entirely profile-driven:baseUrl+path, model from the profile, secret-by-reference auth (apiKeyEnvnames an env var →Authorization: Bearer),${VAR}-expanded custom headers, optionaltemperature/maxTokens/extraBody.429) and/or an error-body regex → setsquotaExhaustedso the engine's degraded path kicks in.AbortControllertimeout, bounded output capture, strict option validation at construction.fetch) is injectable for tests; transport/HTTP failures resolve to a diagnosticmetaresult rather than throwing. Providerusageis surfaced inmetafor the Milestone 5 cost ledger.runnerFactory(thehttpkind now builds).Task 15 — end-to-end session
session.runGoal(goal, config, opts): headless entrypoint that wiresconfig → createRoles → plan review → per-task loop, runs tasks in dependency order, and skips dependents whose prerequisites didn't reach a usable terminal state. (Bounded concurrency + worktrees arrive in Milestone 4 behind this same entrypoint.)src/run.tsCLI:npm start -- "your goal", fully config-driven, non-zero exit when anything needs human attention.Testing
npm run typecheckclean.npm test: 91 passing (+12:test/httpRunner.test.ts,test/session.test.ts, plus updated factory test). Covers request shaping, secret-by-reference auth, quota/error/timeout handling, and full multi-task sessions (all-verified path + dependent-skip-on-needs-human).Summary by CodeRabbit
New Features
Tests
Chores