feat: /goal命令能力支持，参考codex实现#1261
Open
Moyhub wants to merge 5 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a persistent per-session goal system: in-memory state machine, JSONL persistence and resume, LLM-facing GoalTool, /goal command and UI, REPL auto-continuation, token tracking, steering prompts, and unit/integration tests. ChangesPersistent Goal Tracking for Thread Sessions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/sessionStorage.ts (1)
3199-3210:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPre-boundary metadata scan omits
goal/goal-clearedmarkers.
loadTranscriptFile()now parses goal metadata in the pre-boundary pass (Line 3694+), butMETADATA_TYPE_MARKERSnever includes those entry types. When pre-compact skip truncates earlier bytes, goal state can be silently lost on resume.Suggested fix
const METADATA_TYPE_MARKERS = [ '"type":"summary"', '"type":"custom-title"', '"type":"tag"', '"type":"agent-name"', '"type":"agent-color"', '"type":"agent-setting"', '"type":"mode"', '"type":"worktree-state"', + '"type":"goal"', + '"type":"goal-cleared"', '"type":"pr-link"', ]🤖 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/utils/sessionStorage.ts` around lines 3199 - 3210, METADATA_TYPE_MARKERS used by the pre-boundary scan omits goal metadata, causing loadTranscriptFile to skip '"type":"goal"' and '"type":"goal-cleared"' entries when pre-compact truncation occurs; update the METADATA_TYPE_MARKERS array (and thereby METADATA_MARKER_BUFS) to include '"type":"goal"' and '"type":"goal-cleared"' so the pre-boundary pass will detect and preserve goal state, then run tests that exercise loadTranscriptFile to verify goals survive resume.
🧹 Nitpick comments (3)
src/services/goal/goalAudit.ts (1)
5-6: ⚡ Quick winSwitch these imports to
src/*aliases.Use alias-based imports here for consistency with the source-tree import policy.
As per coding guidelines, "Import paths must use the
src/*alias (e.g.,import { ... } from 'src/utils/...')."Suggested diff
-import { BLOCKED_CONSECUTIVE_THRESHOLD, MAX_GOAL_TURNS } from './goalState.js' -import type { GoalStatus } from '../../types/logs.js' +import { BLOCKED_CONSECUTIVE_THRESHOLD, MAX_GOAL_TURNS } from 'src/services/goal/goalState.js' +import type { GoalStatus } from 'src/types/logs.js'🤖 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/services/goal/goalAudit.ts` around lines 5 - 6, Replace the relative imports in goalAudit.ts with src/* aliases: change the import of BLOCKED_CONSECUTIVE_THRESHOLD and MAX_GOAL_TURNS from './goalState.js' to the aliased path (e.g., 'src/services/goal/goalState') and change the import of the GoalStatus type from '../../types/logs.js' to the aliased path (e.g., 'src/types/logs'); ensure the imported symbols BLOCKED_CONSECUTIVE_THRESHOLD, MAX_GOAL_TURNS, and GoalStatus are used unchanged.Source: Coding guidelines
src/services/goal/goalState.ts (1)
8-10: ⚡ Quick winUse
src/*alias imports insrc/files.These relative imports should use the
src/...alias to match the repo import-path contract.As per coding guidelines, "Import paths must use the
src/*alias (e.g.,import { ... } from 'src/utils/...')."Suggested diff
-import type { GoalState, GoalStatus } from '../../types/logs.js' -import { getSessionId } from '../../bootstrap/state.js' -import { logForDebugging } from '../../utils/debug.js' +import type { GoalState, GoalStatus } from 'src/types/logs.js' +import { getSessionId } from 'src/bootstrap/state.js' +import { logForDebugging } from 'src/utils/debug.js'🤖 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/services/goal/goalState.ts` around lines 8 - 10, Replace the relative imports in src/services/goal/goalState.ts with the repository alias form: change the import of types GoalState and GoalStatus, getSessionId, and logForDebugging to use 'src/...' paths (e.g., import { GoalState, GoalStatus } from 'src/types/logs' and import { getSessionId } from 'src/bootstrap/state' and import { logForDebugging } from 'src/utils/debug') so the file follows the repo's import-path contract.Source: Coding guidelines
src/services/goal/prompts.ts (1)
44-49: ⚡ Quick winEscape objective text before embedding into XML-wrapped prompts.
Raw
goal.objective/newObjective/previousObjectivecan break wrapper tags (or inject competing tags), which makes steering prompts brittle.Suggested fix
+function escapeXmlText(value: string): string { + return value + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + .replaceAll('"', '"') + .replaceAll("'", ''') +} export function buildContinuationPrompt(goal: GoalState): string { + const safeObjective = escapeXmlText(goal.objective) const elapsed = formatGoalElapsed(goal) const tokenInfo = formatTokenUsage(goal) const turnInfo = `Continuation turns executed: ${goal.turnsExecuted}` @@ -${goal.objective} +${safeObjective} @@ export function buildBudgetLimitPrompt(goal: GoalState): string { + const safeObjective = escapeXmlText(goal.objective) return `<goal-steering type="budget_limit"> @@ -- Goal: ${goal.objective} +- Goal: ${safeObjective} @@ export function buildObjectiveUpdatedPrompt( newObjective: string, previousObjective?: string, ): string { + const safeNewObjective = escapeXmlText(newObjective) const previousSection = previousObjective - ? `\nPrevious objective: ${previousObjective}\n` + ? `\nPrevious objective: ${escapeXmlText(previousObjective)}\n` : '' @@ -New objective: ${newObjective} +New objective: ${safeNewObjective} @@ export function buildGoalContextBlock(goal: GoalState): string { + const safeObjective = escapeXmlText(goal.objective) @@ - goal.objective, + safeObjective,Also applies to: 93-95, 120-127, 146-150
🤖 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/services/goal/prompts.ts` around lines 44 - 49, The XML-wrapped prompt templates in src/services/goal/prompts.ts embed raw text (e.g., goal.objective, newObjective, previousObjective) which can break or inject tags; fix by HTML/XML-escaping special characters (&, <, >, ", ') before interpolation. Add or reuse a small helper like escapeXml(str) and call it wherever those template strings are constructed (the places that return `<goal-steering ...>` and the other template blocks referenced around lines 93-95, 120-127, 146-150), replacing direct uses of goal.objective/newObjective/previousObjective with escapeXml(value).
🤖 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 `@packages/builtin-tools/src/tools/GoalTool/prompt.ts`:
- Around line 34-37: The prompt's contract is incorrect: it claims the `update`
operation is a no-op when no goal exists, but the implementation (GoalTool.ts
`update` method) throws/returns an error; update the prompt in
GoalTool/prompt.ts to state that `update` returns an error if no active goal
exists (or alternately change the implementation to be a no-op), and ensure the
wording matches the `update` method behavior and error semantics used in
GoalTool.ts so planners use and handle the tool consistently.
In `@src/commands/goal/goal.tsx`:
- Around line 18-31: The file uses relative imports; update all imports to use
the src/* path alias instead (e.g. change imports that reference
'../../commands.js', '../../types/command.js',
'../../services/goal/goalState.js', '../../services/goal/goalStorage.js', and
'./GoalReplaceConfirmDialog.js' to their corresponding src/... paths).
Specifically replace the imports that provide LocalJSXCommandContext,
LocalJSXCommandOnDone, the functions clearGoal, completeGoal, formatGoalElapsed,
formatGoalStatusLabel, getGoal, pauseGoal, resumeGoal, setGoal, the
persistCurrentGoal/persistGoalClear functions, and the GoalReplaceConfirmDialog
component to import from src/commands.js, src/types/command.js,
src/services/goal/goalState.js, src/services/goal/goalStorage.js, and
src/commands/goal/GoalReplaceConfirmDialog.js (or the correct src-relative
locations) so the module uses the src alias consistently.
In `@src/commands/goal/GoalReplaceConfirmDialog.tsx`:
- Around line 9-13: The imports at the top of GoalReplaceConfirmDialog.tsx use
relative paths; change them to the project alias form (src/...) to comply with
the src/* import rule. Replace the relative import for GoalState, Select
(CustomSelect), PermissionDialog, and the goal state helpers (formatGoalElapsed,
formatGoalStatusLabel) with their equivalent src/ alias module paths so lines
importing GoalState, Select, PermissionDialog, formatGoalElapsed, and
formatGoalStatusLabel use src/... instead of ../../ or ../../components/... etc.
In `@src/commands/goal/index.ts`:
- Line 1: The import currently pulls the Command type using a relative path for
the symbol "Command" in the module's top import; replace that relative import
with the repo alias style (use the src/* alias) so the type import uses the
aliased module path instead of '../../commands.js' (e.g., import type { Command
} from 'src/commands'); update any extension as needed to match TS build config.
- Around line 9-10: The command is marked bridgeSafe: true but its interactive
path can open a local JSX dialog (GoalReplaceConfirmDialog), which makes
bridge/mobile invocations unsafe; remove or set bridgeSafe to false on the
command export (the object containing bridgeSafe and load in
src/commands/goal/index.ts) or guard the interactive branch so it never triggers
under bridge-safe invocation (e.g., check bridge-safe context before showing
GoalReplaceConfirmDialog and fall back to a non-interactive flow); update the
exported command object to reflect the safe setting and ensure
GoalReplaceConfirmDialog is not invoked when bridgeSafe is true.
In `@src/components/StatusLine.tsx`:
- Line 48: Replace the relative import in StatusLine.tsx that pulls in
formatTokens from '../utils/format.js' with the project alias path
'src/utils/format.js' so it conforms to the src/* import rule; update the import
statement referencing the formatTokens symbol to use 'src/utils/format.js' and
ensure any IDE/type imports still resolve after the change.
In `@src/hooks/useGoalContinuation.ts`:
- Around line 54-55: The budgetLimitFiredRef (declared in useGoalContinuation)
is never reset, so once a budget-limit wrap-up is injected it blocks future
goals; update useGoalContinuation to reset budgetLimitFiredRef.current = false
whenever a new goal starts (e.g., inside an effect that watches the current goal
id or goal index) and apply the same reset logic to the similar refs/handlers
referenced in the 85-101 region so each new goal can emit its own budget-limit
prompt; locate budgetLimitFiredRef and the other refs in useGoalContinuation and
add a useEffect or reset point at goal transition to clear them.
- Line 97: In useGoalContinuation, the enqueue calls are forcing origin to a
string which breaks the expected MessageOrigin shape; remove the "as unknown as
string" casts and pass the origin as an object with the correct shape (e.g., {
kind: 'goal-budget-limit' } and { kind: 'goal-continuation' }) so enqueue (and
the QueuedCommand origin) retains type safety and downstream code can read
origin.kind; update any local typings only if necessary to accept MessageOrigin
in the enqueue call within the useGoalContinuation hook.
In `@src/screens/REPL.tsx`:
- Around line 2545-2559: The shared cancel handler in REPL.tsx currently always
pauses active goals and sets wasAborted; change it so only
keyboard/interrupt-caused cancels pause goals: add an explicit abort-source flag
(e.g., userInterrupt or abortCause) to the onCancel path and update callers
(restore/edit flows) to pass false; then wrap the existing feature('GOAL') block
(the calls to getGoal, pauseGoal, persistCurrentGoal) and the
setWasAborted(true) so they run only when that flag indicates a real user
interrupt (not a restore/dialog dismissal). Ensure references to getGoal,
pauseGoal, persistCurrentGoal and setWasAborted are guarded by the new
condition.
- Around line 2233-2240: REPL currently only hydrates goal state inside
REPL.resume() via hydrateGoalFromTranscript(log.goal), but resumes using
initialMessages (ResumeConversation -> <REPL
initialMessages={resumeData.messages} />) don't call resume(), so the in-memory
goalState map isn't populated; update the initialMessages mount effect (the
useEffect that restores read-file state/remote tasks) to also detect a resumed
goal (e.g., resumeData.currentSessionGoal or resumeData.goal) and call
hydrateGoalFromTranscript with a Map keyed by sessionId (same shape used in
REPL.resume), or alternatively call the same helper used in REPL.resume to
populate goalState; ensure you reference hydrateGoalFromTranscript, REPL.resume,
ResumeConversation, initialMessages, restoreSessionMetadata/currentSessionGoal
and populate the goalState map during the mount flow so goal continuation works
for initialMessages-based resumes.
In `@src/services/goal/goalState.ts`:
- Around line 94-100: The resumeGoal logic currently permits resuming goals in
'blocked', 'budget_limited', and 'usage_limited' states; update the check in
resumeGoal so only goals with status 'paused' may be resumed (do not allow
resume for terminal states), or alternatively call and respect
isGoalTerminal(goal) to prevent resuming any terminal/limit states (including
'blocked', 'budget_limited', 'usage_limited'); ensure the function returns null
and the UX message remains "No paused goal to resume" when the goal is not
resumable.
In `@src/services/goal/prompts.ts`:
- Around line 141-147: The opening tag string in buildGoalContextBlock is
constructing invalid XML/HTML by injecting " | budget: ..." into the tag body;
change this to produce a proper attribute instead—compute a budgetAttr (e.g.,
budgetAttr = goal.tokenBudget !== null ? `
budget="${goal.tokensUsed}/${goal.tokenBudget}"` : '') and interpolate that into
the tag `<active-goal ...${budgetAttr} turns="...">`, ensuring spacing and no
pipe character so the tag remains well-formed; update the code that builds the
`budget` variable (or replace it) and adjust the return line that currently
references `budget` to use the new attribute string.
---
Outside diff comments:
In `@src/utils/sessionStorage.ts`:
- Around line 3199-3210: METADATA_TYPE_MARKERS used by the pre-boundary scan
omits goal metadata, causing loadTranscriptFile to skip '"type":"goal"' and
'"type":"goal-cleared"' entries when pre-compact truncation occurs; update the
METADATA_TYPE_MARKERS array (and thereby METADATA_MARKER_BUFS) to include
'"type":"goal"' and '"type":"goal-cleared"' so the pre-boundary pass will detect
and preserve goal state, then run tests that exercise loadTranscriptFile to
verify goals survive resume.
---
Nitpick comments:
In `@src/services/goal/goalAudit.ts`:
- Around line 5-6: Replace the relative imports in goalAudit.ts with src/*
aliases: change the import of BLOCKED_CONSECUTIVE_THRESHOLD and MAX_GOAL_TURNS
from './goalState.js' to the aliased path (e.g., 'src/services/goal/goalState')
and change the import of the GoalStatus type from '../../types/logs.js' to the
aliased path (e.g., 'src/types/logs'); ensure the imported symbols
BLOCKED_CONSECUTIVE_THRESHOLD, MAX_GOAL_TURNS, and GoalStatus are used
unchanged.
In `@src/services/goal/goalState.ts`:
- Around line 8-10: Replace the relative imports in
src/services/goal/goalState.ts with the repository alias form: change the import
of types GoalState and GoalStatus, getSessionId, and logForDebugging to use
'src/...' paths (e.g., import { GoalState, GoalStatus } from 'src/types/logs'
and import { getSessionId } from 'src/bootstrap/state' and import {
logForDebugging } from 'src/utils/debug') so the file follows the repo's
import-path contract.
In `@src/services/goal/prompts.ts`:
- Around line 44-49: The XML-wrapped prompt templates in
src/services/goal/prompts.ts embed raw text (e.g., goal.objective, newObjective,
previousObjective) which can break or inject tags; fix by HTML/XML-escaping
special characters (&, <, >, ", ') before interpolation. Add or reuse a small
helper like escapeXml(str) and call it wherever those template strings are
constructed (the places that return `<goal-steering ...>` and the other template
blocks referenced around lines 93-95, 120-127, 146-150), replacing direct uses
of goal.objective/newObjective/previousObjective with escapeXml(value).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d824707-13ff-45aa-bb4d-b3cf9f599ff0
📒 Files selected for processing (22)
packages/builtin-tools/src/index.tspackages/builtin-tools/src/tools/GoalTool/GoalTool.tspackages/builtin-tools/src/tools/GoalTool/constants.tspackages/builtin-tools/src/tools/GoalTool/prompt.tsscripts/defines.tssrc/commands.tssrc/commands/goal/GoalReplaceConfirmDialog.tsxsrc/commands/goal/goal.tsxsrc/commands/goal/index.tssrc/components/StatusLine.tsxsrc/cost-tracker.tssrc/hooks/useGoalContinuation.tssrc/screens/REPL.tsxsrc/services/goal/__tests__/goalState.test.tssrc/services/goal/goalAudit.tssrc/services/goal/goalState.tssrc/services/goal/goalStorage.tssrc/services/goal/prompts.tssrc/tools.tssrc/types/logs.tssrc/utils/sessionStorage.tstests/integration/goal-lifecycle.test.ts
added 2 commits
June 8, 2026 17:47
- prompt 中 update 行为描述与运行时不一致（no-op → error） - src/commands/goal/ 使用相对路径导入，改为 src/* 别名 - /goal 命令标记 bridgeSafe 但含交互式对话框，改为 false - useGoalContinuation 中 origin 使用 as unknown as string 强转，改为直接传字符串 - ResumeConversation 路径缺少 goal hydration，补齐恢复逻辑 - onCancel 在非查询状态下误暂停 goal，加 queryGuard 守卫 - resumeGoal 允许从终态恢复，收紧为仅允许 paused 状态 - buildGoalContextBlock 生成畸形 XML 属性，改为合法 budget 属性
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/services/goal/goalState.ts (2)
67-72: ⚡ Quick winAvoid redundant
resolveSessionIdcalls.
resolveSessionId(sessionId)is called twice in this function. Cache the result to avoid repeated computation.♻️ Suggested fix
export function clearGoal(sessionId?: string): boolean { - const had = goals.has(resolveSessionId(sessionId)) - const result = goals.delete(resolveSessionId(sessionId)) + const id = resolveSessionId(sessionId) + const had = goals.has(id) + const result = goals.delete(id) if (had) goalLog('CLEAR', 'goal removed') return result }🤖 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/services/goal/goalState.ts` around lines 67 - 72, The function clearGoal calls resolveSessionId(sessionId) twice causing redundant work; cache the resolved id in a local variable (e.g., const id = resolveSessionId(sessionId)) and use that variable for goals.has(id), goals.delete(id), and the goalLog('CLEAR', ...) call so resolveSessionId is invoked only once and behavior of clearGoal, goals.has, goals.delete, and goalLog remains unchanged.
8-8: 💤 Low valueUse
src/*import alias instead of relative path.The import uses a relative path
../../types/logs.js. As per coding guidelines, import paths should use thesrc/*alias.♻️ Suggested fix
-import type { GoalState, GoalStatus } from '../../types/logs.js' +import type { GoalState, GoalStatus } from 'src/types/logs.js'🤖 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/services/goal/goalState.ts` at line 8, Replace the relative import for GoalState and GoalStatus with the project alias: change the import in goalState.ts that currently references '../../types/logs.js' to use the 'src/*' alias (importing GoalState and GoalStatus from 'src/types/logs' or 'src/types/logs.js' depending on resolver) so the symbols GoalState and GoalStatus are imported via the alias rather than a relative path.Source: Coding guidelines
src/screens/REPL.tsx (1)
2547-2550: ⚡ Quick winKeep
feature()as the direct branch condition.Line 2550 uses
feature('GOAL')inside an&&chain. Split this into nested conditionals so the Bun feature flag stays in the directifposition required by the repo rule.As per coding guidelines, feature flags "can only be used directly in
ifstatements or ternary expressions, not assigned to variables or used in&&chains."🤖 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/screens/REPL.tsx` around lines 2547 - 2550, The branch currently uses feature('GOAL') in an && chain; change it so feature('GOAL') is the direct if condition by nesting: first check if (feature('GOAL')) and then inside that block check queryGuard.getSnapshot() (the existing queryGuard.getSnapshot() condition and its body). Ensure the symbol feature('GOAL') remains the direct branch and that queryGuard.getSnapshot() is evaluated only within the nested block.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@src/screens/REPL.tsx`:
- Around line 2547-2550: The branch currently uses feature('GOAL') in an &&
chain; change it so feature('GOAL') is the direct if condition by nesting: first
check if (feature('GOAL')) and then inside that block check
queryGuard.getSnapshot() (the existing queryGuard.getSnapshot() condition and
its body). Ensure the symbol feature('GOAL') remains the direct branch and that
queryGuard.getSnapshot() is evaluated only within the nested block.
In `@src/services/goal/goalState.ts`:
- Around line 67-72: The function clearGoal calls resolveSessionId(sessionId)
twice causing redundant work; cache the resolved id in a local variable (e.g.,
const id = resolveSessionId(sessionId)) and use that variable for goals.has(id),
goals.delete(id), and the goalLog('CLEAR', ...) call so resolveSessionId is
invoked only once and behavior of clearGoal, goals.has, goals.delete, and
goalLog remains unchanged.
- Line 8: Replace the relative import for GoalState and GoalStatus with the
project alias: change the import in goalState.ts that currently references
'../../types/logs.js' to use the 'src/*' alias (importing GoalState and
GoalStatus from 'src/types/logs' or 'src/types/logs.js' depending on resolver)
so the symbols GoalState and GoalStatus are imported via the alias rather than a
relative path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73c7157c-353c-48f6-8aa0-7c6aa6297905
📒 Files selected for processing (8)
src/commands/goal/GoalReplaceConfirmDialog.tsxsrc/commands/goal/goal.tsxsrc/commands/goal/index.tssrc/hooks/useGoalContinuation.tssrc/screens/REPL.tsxsrc/screens/ResumeConversation.tsxsrc/services/goal/goalState.tssrc/services/goal/prompts.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/commands/goal/index.ts
- src/commands/goal/goal.tsx
- src/commands/goal/GoalReplaceConfirmDialog.tsx
- src/services/goal/prompts.ts
- src/hooks/useGoalContinuation.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks/useGoalContinuation.ts (1)
50-51: ⚡ Quick win
optsRefis assigned but never read.
optsRef.currentis set but never accessed in the effect body. The effect readsoptsdirectly (lines 57, 62, 69, 73, 77), making this ref unused.♻️ Remove unused ref
export function useGoalContinuation(opts: UseGoalContinuationOpts): void { - const optsRef = useRef(opts) - optsRef.current = opts - const enqueuedRef = useRef(false) const budgetLimitFiredRef = useRef(false)🤖 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/hooks/useGoalContinuation.ts` around lines 50 - 51, The optsRef variable in useGoalContinuation is assigned (optsRef.current = opts) but never read; remove the unused ref to simplify the hook: delete the optsRef declaration and the assignment, and update the effect to keep using the existing opts references (used in the effect body where opts is read on lines referencing opts) so behavior is unchanged; ensure no other code in useGoalContinuation (the effect callbacks) tries to access optsRef after removal.
🤖 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.
Nitpick comments:
In `@src/hooks/useGoalContinuation.ts`:
- Around line 50-51: The optsRef variable in useGoalContinuation is assigned
(optsRef.current = opts) but never read; remove the unused ref to simplify the
hook: delete the optsRef declaration and the assignment, and update the effect
to keep using the existing opts references (used in the effect body where opts
is read on lines referencing opts) so behavior is unchanged; ensure no other
code in useGoalContinuation (the effect callbacks) tries to access optsRef after
removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ac63775-567a-4254-8fd2-0a5233c52daf
📒 Files selected for processing (2)
src/components/StatusLine.tsxsrc/hooks/useGoalContinuation.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/StatusLine.tsx
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Author
|
本地测试流程： /goal 为以下每个模块创建完整的单元测试文件，每个文件至少20个测试用例，覆盖所有导出函数的正常路径、边界条件和错误处理。每个测试文件写完后立即运行验证。模块列表：1) src/services/goal/goalState.ts 2) src/services/goal/goalStorage.ts 3) src/services/goal/goalAudit.ts 4) src/services/goal/prompts.ts 5) src/commands/goal/goal.tsx 6) src/hooks/useGoalContinuation.ts 7) packages/builtin-tools/src/tools/GoalTool/GoalTool.ts。全部完成后运行bun test确认零失败。 debug日志： 
@claude-code-best 可以看看是否有BUG |
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.
从 CODEX 项目（Rust 多 crate 架构）移植 /goal 功能到 Claude Code（TypeScript 单进程），适配 React/Ink UI 框架和 JSONL 持久化方案。
从 Codex 项目移植 /goal 命令到 Claude Code，实现：
完整流程图：
Summary by CodeRabbit
New Features
Tests