Repository navigation
fix(repl): forward the run's policy instead of re-listing the options it knows - #75
Merged
Merged
Conversation
… it knows The REPL built its `runAgent` call field by field, and the list was incomplete: `steer` and `stopGuard` were never passed. So `--plan-interval` planned nothing interactively and `--mode tasks` could end a turn with its list unfinished — both worked in a one-shot run, and both did nothing in the REPL, silently. That is the same failure shape as a mode whose tools are wired but whose runner is not (#69): the capability exists, the host drops it on the way to the loop. The fix is structural rather than a third field on the list. `ReplOptions` now extends the loop's `RunAgentOptions` (minus `messages`/`onEvent`, which the REPL owns), and the turn passes its options through wholesale: const result = await runAgent({ ...options, messages: next, onEvent: render }); so a new run seam is forwarded by default and omitting one has to be written down. The `ReplOptions` doc comment says why, because the next person to add an option to the loop should not have to rediscover this. Two unit tests pin the seams that were missing — a `steer` whose text must reach the model, and a `stopGuard` that must earn a second model call — and both fail against the old hand-picked list. One e2e test drives the real REPL over stdin (answer-driven, because `readline.question` drops anything typed while no prompt is pending) and asserts a `--plan-interval 1` turn plans, records the plans, and reports the cost in `/steps`. That last assertion also needed the `/steps` footer to read `planning: plans=…` rather than `planning:plans=…`; the footer could not appear before this fix, so the spacing had no chance to be seen (it landed in #74). Docs: `docs/architecture.md` states the forwarding rule where the opt-in seams are described, `README.md` says run policy applies interactively, and `AGENTS.md` gains the REPL paragraph next to the loop seams. Verified: `pnpm build`, `pnpm typecheck`, `pnpm lint`, `pnpm test:coverage` all green (coverage 90.3 / 84.1 / 89.3 / 90.3). `src/repl.ts`, `src/repl.test.ts` and the touched docs are prettier-clean; the pre-existing drift in `src/cli.e2e.test.ts` is left alone (#49).
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
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.
The bug
The REPL built its
runAgentcall field by field, and the list was incomplete:steerandstopGuardwere never in it. The consequence is invisible and wrong:--plan-interval <n>planned nothing in the REPL.--mode taskscould end a turn with the shared list still unfinished.Both work in a one-shot run, so the flags look healthy; only the interactive path was broken. It is the same shape as a mode whose tools are wired but whose runner is not (#69): the capability exists and the host drops it on the way to the loop.
I found it while adding the planning-cost report (#74): the
/stepsfooter asserted in the e2e never appeared, because no planning call ever happened in the REPL.The fix
Structural, not a third field on the list:
A new seam added to the loop now flows to the REPL automatically; dropping one has to be written down. The
ReplOptionsdoc comment records why, anddocs/architecture.mdstates the rule where the opt-in seams are described.Verified
steerwhose text must reach the model's next prompt, and astopGuardthat must earn a second model call. Both fail against the old hand-picked list (checked by temporarily restoring it).--plan-interval 1turn plans, records the plans, and reportsplanning: plans=… plan_tokens=… plan_ms=…in/steps. The driver is answer-driven —readline.questiondrops anything typed while no prompt is pending, so a single stdin dump only ever runs the first line.pnpm build,pnpm typecheck,pnpm lint,pnpm test:coverage— green (331 passed / 5 skipped; coverage 90.3 / 84.1 / 89.3 / 90.3).Note
The
/stepsfooter spacing (planning: plans=…instead ofplanning:plans=…) is fixed here rather than in #74: the footer could not appear before this change, so the cosmetic problem had no way to be seen.