feat(cli): /goal and /loop commands for long-horizon tasks - #257
Conversation
/goal sets a verifiable completion condition; after each turn a small evaluator call decides whether the condition holds and either marks the goal achieved or injects the evaluator's reason as the next-turn directive. Wires into client.ts at the same continuation site as the existing Stop hook and CompletionChecker paths, so feedback drives a new turn via the same sendMessageStream recursion the rest of the codebase uses. /loop schedules a recurring prompt via the existing CronScheduler so it inherits jitter, expiry, and the idle-drain submit path already wired in useGeminiStream. Supports both leading-token (5m check deploy) and trailing "every N unit" parsing, a 10m default, list/clear/stop, and single-job cancellation by 8-char id. Replaces the bundled loop skill that used to model-route /loop input through cron_create / cron_list / cron_delete tools; the tools stay for natural-language scheduling. New modules in core: goal/ (manager, evaluator) and loop/ (intervalParser, intervalToCron). Docs added at docs/guides/goal.md and the existing docs/guides/scheduled-tasks.md picks up the new management syntax. 815 tests pass across the directories touched. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning Review limit reached
Your plan currently allows 2 reviews/hour. Refill in 19 minutes and 41 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughThis PR adds two new long-horizon task automation features to the proto CLI: the ChangesGoal and Loop Task Automation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@docs/guides/goal.md`:
- Line 3: Update the sentence describing /goal to remove the incorrect claim of
a “small fast model” and state that the evaluator uses the configured primary
model (Config.getModel()) instead; locate the phrase "/goal" and replace "a
small fast model checks the transcript" with wording like "the configured
primary model (Config.getModel()) checks the transcript" and note that the
behavior otherwise (starting another turn until the condition is met and
automatic clearing) remains the same.
- Around line 16-18: The fenced code blocks in docs/guides/goal.md are missing
language identifiers; update the three shown snippets (the blocks containing
"/goal all tests in test/auth pass and the lint step is clean", the single
"/goal" block, and the "/goal clear" block) to use a language tag such as text
(i.e., change ``` to ```text) and apply the same fix to the other fenced blocks
noted at lines 43-45 and 53-55 so they all include a language identifier to
satisfy MD040.
In `@packages/core/src/core/client.ts`:
- Around line 1028-1038: Wrap the evaluateGoal call in a best-effort try/catch
so evaluator failures do not abort a completed turn: call evaluateGoal (using
this.getContentGeneratorOrFail(), this.config.getModel(), and the payload
containing active.condition, toolCallSummary, lastGoalAssistantMessage, signal)
inside try, and only call goalManager.recordEvaluation(evalResult) if the call
succeeded; in the catch block log the error (including context like
active.id/condition and which model) and do not rethrow, leaving the goal active
so the request continues normally.
- Around line 995-1053: The current logic treats any SendMessageType.Hook as
exempt from goal re-evaluation, but the unmet-goal continuation uses
SendMessageType.Hook (continueRequest via sendMessageStream), so goal evaluation
runs only once; change the continuation to use a distinct type or flag so
goal-driven continuations still get re-evaluated: update the
continueRequest/sendMessageStream call in the unmet-goal branch to pass either a
new enum member (e.g., SendMessageType.GoalContinuation) or an options flag
(e.g., { goalContinuation: true } or { skipUserHooks: true }) and then adjust
the top-level gate that checks messageType !== SendMessageType.Hook to instead
skip only true user hooks (e.g., messageType === SendMessageType.Hook &&
!options?.goalContinuation) or check the flag, ensuring evaluateGoal/goalManager
logic (evaluateGoal, goalManager.recordEvaluation/markAchieved) will run for
these injected continuations.
In `@packages/core/src/goal/goalEvaluator.ts`:
- Around line 147-158: The function extractFirstJsonObject currently counts
every '{' and '}' including those inside JSON strings; update it to track
whether the parser is inside a double-quoted string and ignore braces while
inString is true (toggle inString only on unescaped double quotes, handle escape
sequences by checking preceding backslashes), so depth is
incremented/decremented only when not inString; ensure escaped quotes (\"),
escaped backslashes (\\) and typical JSON string escapes are handled so strings
containing braces like "see {artifact}" don't break brace matching.
In `@packages/core/src/goal/GoalManager.ts`:
- Around line 52-53: The manager currently returns internal state objects (e.g.,
returning this.active) directly which allows external mutation; update all
accessor/returning spots (the return this.active; line and the other returns
around lines 61-67 and 84-85) to return defensive copies instead of the
originals—use an appropriate cloning strategy (shallow copy with object/array
spread or structuredClone/deep copy for nested objects) so callers receive a
copy while internal state is only mutated via recordTurn/recordEvaluation;
ensure you apply the same change to the other methods that expose internal
arrays/objects referenced in GoalManager (e.g., any getters that return goals,
turns, or evaluations).
In `@packages/core/src/loop/intervalToCron.ts`:
- Around line 70-75: The code computes days = Math.round(hours / 24) but sets
rounded by comparing days !== hours / 24 which can be false for near-24h inputs
that did get rounded; change the rounded check to detect that the rounded day
value changed the hour count (e.g., use days * 24 !== hours) so the returned
object (cron/description/rounded) correctly reports rounding; update the
returned object's rounded property (in the same block that uses days and builds
cron/description) to use this new comparison.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b8734a9-69da-4b1c-992b-8a6375f6eda2
📒 Files selected for processing (24)
docs/guides/_meta.tsdocs/guides/goal.mddocs/guides/scheduled-tasks.mdpackages/cli/src/services/BuiltinCommandLoader.tspackages/cli/src/ui/commands/goalCommand.test.tspackages/cli/src/ui/commands/goalCommand.tspackages/cli/src/ui/commands/loopCommand.test.tspackages/cli/src/ui/commands/loopCommand.tspackages/core/src/config/config.tspackages/core/src/core/client.tspackages/core/src/goal/GoalManager.test.tspackages/core/src/goal/GoalManager.tspackages/core/src/goal/goalEvaluator.test.tspackages/core/src/goal/goalEvaluator.tspackages/core/src/goal/index.tspackages/core/src/goal/types.tspackages/core/src/index.tspackages/core/src/loop/index.tspackages/core/src/loop/intervalParser.test.tspackages/core/src/loop/intervalParser.tspackages/core/src/loop/intervalToCron.test.tspackages/core/src/loop/intervalToCron.tspackages/core/src/loop/types.tspackages/core/src/skills/bundled/loop/SKILL.md
💤 Files with no reviewable changes (1)
- packages/core/src/skills/bundled/loop/SKILL.md
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
- /goal evaluator now re-runs after goal-driven continuations. Previously
the unmet-goal continuation used SendMessageType.Hook, which the next
pass's gate excluded -- so /goal would inject feedback once and then
stop. Added SendMessageType.GoalContinuation, drop the Hook-only gate
on the goal block, and extend the CompletionChecker gate to also skip
goal-driven continuations so they don't interleave.
- Evaluator failures no longer abort a completed turn. evaluateGoal is a
second model call after the user's turn already succeeded; if it
throws we now log and skip recording, leaving the goal active and the
user's turn output intact.
- extractFirstJsonObject tracks in-string state and JSON escape
sequences, so payloads like {"reason": "see {artifact}"} no longer
break brace matching.
- stripCodeFence replaces the lazy-quantifier regex (CodeQL polynomial
regex finding) with a simple indexOf/lastIndexOf scan. Regression
test verifies pathological whitespace input returns in under 100ms.
- GoalManager.setGoal and clearGoal now return defensive copies so
callers can't mutate internal state through the return value.
getActiveGoal/getLastAchievedGoal already did this.
- intervalToCron rounded check for near-24h inputs (e.g. 1439m) now
compares back in minutes (days * 24 * 60 !== minutes) instead of
hours / 24, so the rounding warning fires correctly.
- docs/guides/goal.md corrects the "small fast model" wording to
reference Config.getModel() with a note about the future fast-model
switch, and adds language tags to fenced code blocks for MD040.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
/goal <condition>— sets a verifiable completion condition; after every turn a small evaluator call decides whether the condition holds and either marks the goal achieved or injects the evaluator's reason as the next-turn directive. Mirrors Claude Code's/goal./loop [interval] <prompt>— schedules a recurring prompt on the existingCronSchedulerso it inherits jitter, expiry, and the idle-drain submit path already wired inuseGeminiStream. Supports/loop list,/loop stop(+ aliases), and per-job cancel by 8-char id. Default interval is 10m, matching the prior skill.loopskill (model-routed/loop→ cron tools) with the slash command. Thecron_create/cron_list/cron_deletetools stay for natural-language scheduling like "remind me at 3pm".How it works
/goalintegration site (packages/core/src/core/client.ts): added a Goal-evaluation block right afterCompletionChecker, using the samesendMessageStreamcontinuation pattern. When the evaluator returnsmet: false, its reason is injected as a new user message (SendMessageType.Hook,boundedTurns - 1) so the loop is bounded the same way Stop hooks are./loopdesign (packages/cli/src/ui/commands/loopCommand.ts): translates duration tokens (5m,2h,1d, plus "every N units") to cron expressions via the newintervalToCronhelper, then callsCronScheduler.create()directly. The first iteration fires immediately viasubmit_prompt; subsequent iterations come through the cron tick path already wired inuseGeminiStream.ts:2175.What's deferred
--resumedoesn't restore goal state. CronScheduler already persists jobs, so/loopsurvives session restart;/goaldoesn't yet. Small follow-up.Config.getModel(). When protoCLI grows agetFastModel()accessor, the one-line swap is inclient.ts./goalis implemented parallel to the hooks system (GoalManagersits next toCompletionChecker) rather than as a session-scoped prompt-based Stop hook. Reason: prompt hooks aren't wired to call the model yet (hookRunner.ts:370is a TODO). Refactoring to ride on hooks is a future improvement, not a blocker.Test plan
npx vitest run packages/core/src/goal packages/core/src/loop packages/core/src/hooks packages/core/src/config packages/core/src/core/client.test.ts packages/cli/src/ui/commands packages/cli/src/services— 815 tests passnpx tsc -p packages/core --noEmitcleannpx tsc -p packages/cli --noEmitclean/goal all tests pass, observe one round of evaluator feedback, then/goal clearexperimental.cron: true, run/loop 1m echo hello, observe at least one tick fire, then/loop stop/goaldoesn't fire when the test mock-config lacksgetGoalManager(regression coverage already added — optional-chaining guard atclient.ts:994)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
/goalcommand to set auto-checked completion conditions that evaluate after each turn/loopcommand to schedule recurring prompts at configurable intervalsDocumentation
/loopcommand documentation with supported subcommands and usage details