-
Notifications
You must be signed in to change notification settings - Fork 9.1k
feat(goal): add session-scoped /goal continuation #1293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
98105f4
feat(goal): add persisted session goal state
chioarub 1672d5a
feat(goal): add slash command controls
chioarub 6cc17b2
feat(goal): continue goals through stop hooks
chioarub 3b242df
test(goal): cover commands continuation and resume
chioarub c5477b1
fix(goal): fail closed on evaluator failures
chioarub 4c59f49
fix(goal): clear stale goal on resume
chioarub 6cedc93
Persist goal continuations before auto-resume
chioarub e55050e
test: isolate CI-sensitive state
chioarub 6d7cd02
test: isolate attribution provider state
chioarub a5c1b41
test: tighten CI state isolation
chioarub a392a3b
fix(goal): clear cached metadata on resume
chioarub b2c4449
test(goal): harden resume metadata coverage
chioarub 758a1c5
test: clean up teammate model fixture merge
chioarub 88bbf5b
fix(goal): align persistence session id type
chioarub 00b6143
fix(goal): address status command review
chioarub 4b3f46b
fix(goal): address follow-up review comments
chioarub 389c886
test(goal): isolate review regression coverage
chioarub 8c74971
test(goal): make queryengine fixture ci-safe
chioarub 361979e
fix(goal): clarify resume message
chioarub 1241272
test: restore api preconnect provider mock
chioarub d2966fc
fix(goal): address review feedback
chioarub File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { describe, expect, test } from 'bun:test' | ||
|
|
||
| import { | ||
| getSessionId, | ||
| getSessionProjectDir, | ||
| switchSession, | ||
| } from '../../bootstrap/state.js' | ||
| import { createGoalState } from '../../services/goal/state.js' | ||
| import { getDefaultAppState, type AppState } from '../../state/AppStateStore.js' | ||
| import { clearConversation } from './conversation.js' | ||
|
|
||
| describe('/clear goal lifecycle', () => { | ||
| test('/clear clears active goal state', async () => { | ||
| const previousBareMode = process.env.CLAUDE_CODE_SIMPLE | ||
| const previousSessionId = getSessionId() | ||
| const previousSessionProjectDir = getSessionProjectDir() | ||
| process.env.CLAUDE_CODE_SIMPLE = '1' | ||
| let state: AppState = { | ||
| ...getDefaultAppState(), | ||
| goal: createGoalState('finish implementation'), | ||
| } | ||
| let messages: any[] = [{ type: 'user', uuid: 'user-1' }] | ||
| let sawEmptyMessages = false | ||
|
|
||
| try { | ||
| await clearConversation({ | ||
| setMessages: updater => { | ||
| messages = updater(messages) | ||
| if (messages.length === 0) sawEmptyMessages = true | ||
| }, | ||
| readFileState: new Map() as any, | ||
| getAppState: () => state, | ||
| setAppState: updater => { | ||
| state = updater(state) | ||
| }, | ||
| }) | ||
| } finally { | ||
| if (previousBareMode === undefined) { | ||
| delete process.env.CLAUDE_CODE_SIMPLE | ||
| } else { | ||
| process.env.CLAUDE_CODE_SIMPLE = previousBareMode | ||
| } | ||
| switchSession(previousSessionId, previousSessionProjectDir) | ||
| } | ||
|
|
||
| expect(state.goal).toBeNull() | ||
| expect(sawEmptyMessages).toBe(true) | ||
| }) | ||
| }) | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,213 @@ | ||
| import { describe, expect, test } from 'bun:test' | ||
|
|
||
| import { | ||
| achieveGoal, | ||
| createGoalState, | ||
| pauseGoal, | ||
| } from '../../services/goal/state.js' | ||
| import { getDefaultAppState, type AppState } from '../../state/AppStateStore.js' | ||
| import type { LocalCommandResult } from '../../types/command.js' | ||
| import { call, createGoalCall } from './goal.js' | ||
|
|
||
| type TextCommandResult = Extract<LocalCommandResult, { type: 'text' }> | ||
|
|
||
| function expectTextResult(result: LocalCommandResult): TextCommandResult { | ||
| expect(result.type).toBe('text') | ||
| return result as TextCommandResult | ||
| } | ||
|
|
||
| function makeContext(initialGoal: AppState['goal'] = null) { | ||
| let state: AppState = { | ||
| ...getDefaultAppState(), | ||
| goal: initialGoal, | ||
| } | ||
|
|
||
| return { | ||
| context: { | ||
| getAppState: () => state, | ||
| setAppState: (updater: (prev: AppState) => AppState) => { | ||
| state = updater(state) | ||
| }, | ||
| } as any, | ||
| getState: () => state, | ||
| } | ||
| } | ||
|
|
||
| describe('/goal command', () => { | ||
| test('/goal shows no goal status', async () => { | ||
| const { context } = makeContext() | ||
|
|
||
| const result = expectTextResult(await call('', context)) | ||
|
|
||
| expect(result.value).toContain('No goal set') | ||
| }) | ||
|
|
||
| test('/goal status returns current status without mutating goal', async () => { | ||
| const { context, getState } = makeContext( | ||
| createGoalState('finish implementation'), | ||
| ) | ||
| const originalGoal = getState().goal | ||
|
|
||
| const result = expectTextResult(await call('status', context)) | ||
|
|
||
| expect(result.value).toContain('Status: active') | ||
| expect(result.value).toContain('Condition: finish implementation') | ||
| expect(getState().goal).toBe(originalGoal) | ||
| expect(result.shouldQuery).toBeUndefined() | ||
| expect(result.metaMessages).toBeUndefined() | ||
| }) | ||
|
|
||
| test('/goal shows active, paused, and achieved status details', async () => { | ||
| const active = createGoalState('finish implementation') | ||
| const { context, getState } = makeContext(active) | ||
|
|
||
| const activeResult = expectTextResult(await call('', context)) | ||
| expect(activeResult.value).toContain('Status: active') | ||
| expect(activeResult.value).toContain('Condition: finish implementation') | ||
| expect(activeResult.value).toContain('Turns: 0/50') | ||
| expect(activeResult.value).toContain('Evaluator failures: 0') | ||
|
|
||
| context.setAppState(prev => ({ ...prev, goal: pauseGoal(getState().goal!) })) | ||
| const pausedResult = expectTextResult(await call('', context)) | ||
| expect(pausedResult.value).toContain('Status: paused') | ||
|
|
||
| context.setAppState(prev => ({ | ||
| ...prev, | ||
| goal: achieveGoal(createGoalState('finish implementation'), { | ||
| evaluatedMessageUuid: 'assistant-1', | ||
| reason: 'done', | ||
| }), | ||
| })) | ||
| const achievedResult = expectTextResult(await call('', context)) | ||
| expect(achievedResult.value).toContain('Status: achieved') | ||
| expect(achievedResult.value).toContain('Turns: 1/50') | ||
| expect(achievedResult.value).toContain('Last evaluator reason: done') | ||
| }) | ||
|
|
||
| test('/goal <condition> sets an active goal and starts a turn', async () => { | ||
| const { context, getState } = makeContext() | ||
|
|
||
| const result = expectTextResult( | ||
| await call('finish the implementation', context), | ||
| ) | ||
|
|
||
| expect(getState().goal?.status).toBe('active') | ||
| expect(getState().goal?.condition).toBe('finish the implementation') | ||
| expect(result.value).toContain('Goal set') | ||
| expect(result.shouldQuery).toBe(true) | ||
| expect(result.metaMessages?.[0]).toContain('finish the implementation') | ||
| }) | ||
|
|
||
| test('/goal validates empty conditions', async () => { | ||
| const { context, getState } = makeContext() | ||
|
|
||
| const result = expectTextResult(await call('""', context)) | ||
|
|
||
| expect(getState().goal).toBeNull() | ||
| expect(result.value).toContain('Goal condition cannot be empty') | ||
| expect(result.shouldQuery).toBeUndefined() | ||
| }) | ||
|
|
||
| test('/goal validates conditions over 4,000 characters', async () => { | ||
| const { context, getState } = makeContext() | ||
|
|
||
| const result = expectTextResult(await call('x'.repeat(4001), context)) | ||
|
|
||
| expect(getState().goal).toBeNull() | ||
| expect(result.value).toContain('4,000 characters') | ||
| expect(result.shouldQuery).toBeUndefined() | ||
| }) | ||
|
|
||
| test('/goal replaces an active goal cleanly', async () => { | ||
| const { context, getState } = makeContext() | ||
|
|
||
| await call('first goal', context) | ||
| const firstId = getState().goal?.id | ||
| await call('second goal', context) | ||
|
|
||
| expect(getState().goal?.id).not.toBe(firstId) | ||
| expect(getState().goal?.condition).toBe('second goal') | ||
| expect(getState().goal?.status).toBe('active') | ||
| }) | ||
|
|
||
| test('/goal clear and aliases clear active goal', async () => { | ||
| const aliases = ['clear', 'stop', 'off', 'reset', 'none', 'cancel'] | ||
|
|
||
| for (const alias of aliases) { | ||
| const { context, getState } = makeContext( | ||
| createGoalState('finish implementation'), | ||
| ) | ||
| const result = expectTextResult(await call(alias, context)) | ||
|
|
||
| expect(getState().goal).toBeNull() | ||
| expect(result.value).toContain('Goal cleared') | ||
| } | ||
| }) | ||
|
|
||
| test('/goal pause pauses auto-continuation', async () => { | ||
| const { context, getState } = makeContext( | ||
| createGoalState('finish implementation'), | ||
| ) | ||
|
|
||
| const result = expectTextResult(await call('pause', context)) | ||
|
|
||
| expect(getState().goal?.status).toBe('paused') | ||
| expect(result.value).toContain('Goal paused') | ||
| }) | ||
|
|
||
| test('/goal resume resumes a paused goal and starts a turn', async () => { | ||
| const paused = pauseGoal(createGoalState('finish implementation')) | ||
| const { context, getState } = makeContext(paused) | ||
|
|
||
| const result = expectTextResult(await call('resume', context)) | ||
|
|
||
| expect(getState().goal?.status).toBe('active') | ||
| expect(result.value).toContain('Goal resumed') | ||
| expect(result.shouldQuery).toBe(true) | ||
| expect(result.metaMessages?.[0]).toContain('finish implementation') | ||
| }) | ||
|
|
||
| test('/goal resume reports when there is no goal to resume', async () => { | ||
| const { context, getState } = makeContext() | ||
|
|
||
| const result = expectTextResult(await call('resume', context)) | ||
|
|
||
| expect(getState().goal).toBeNull() | ||
| expect(result.value).toBe('No goal to resume.') | ||
| expect(result.shouldQuery).toBeUndefined() | ||
| expect(result.metaMessages).toBeUndefined() | ||
| }) | ||
|
|
||
| test('/goal does not mutate in-memory state when persistence fails', async () => { | ||
| const callWithFailingPersistence = createGoalCall(async () => { | ||
| throw new Error('persist failed') | ||
| }) | ||
| const cases = [ | ||
| { | ||
| action: 'new persisted goal', | ||
| initialGoal: createGoalState('existing goal'), | ||
| }, | ||
| { | ||
| action: 'clear', | ||
| initialGoal: createGoalState('goal to clear'), | ||
| }, | ||
| { | ||
| action: 'pause', | ||
| initialGoal: createGoalState('goal to pause'), | ||
| }, | ||
| { | ||
| action: 'resume', | ||
| initialGoal: pauseGoal(createGoalState('goal to resume')), | ||
| }, | ||
| ] | ||
|
|
||
| for (const { action, initialGoal } of cases) { | ||
| const { context, getState } = makeContext(initialGoal) | ||
|
|
||
| await expect(callWithFailingPersistence(action, context)).rejects.toThrow( | ||
| 'persist failed', | ||
| ) | ||
| expect(getState().goal).toBe(initialGoal) | ||
| } | ||
| }) | ||
| }) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.