Plan-mode review: approve/reject/revise goals & to-dos before execute (#4105)

This commit is contained in:
Steven Enamakel
2026-06-25 09:24:02 -07:00
committed by GitHub
parent 25e6ae96f3
commit 96437120e5
44 changed files with 1597 additions and 63 deletions
+8
View File
@@ -2989,6 +2989,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'نقل لليمين',
'conversations.taskKanban.title': 'المهام',
'conversations.threadTodo.title': 'الخطة',
'conversations.planReview.title': 'مراجعة الخطة',
'conversations.planReview.subtitle':
'وافق لتشغيلها، أو ارفضها لتجاهلها، أو أرسل ملاحظات لتعديلها.',
'conversations.planReview.approve': 'الموافقة والتشغيل',
'conversations.planReview.reject': 'رفض',
'conversations.planReview.feedbackLabel': 'طلب تغييرات',
'conversations.planReview.feedbackPlaceholder': 'صف ما الذي يجب تغييره…',
'conversations.planReview.sendFeedback': 'إرسال الملاحظات',
'conversations.threadGoal.label': 'الهدف',
'conversations.threadGoal.setCta': 'تحديد هدف',
'conversations.threadGoal.placeholder': 'ما الذي يجب أن تحققه هذه المحادثة؟',
+8
View File
@@ -3055,6 +3055,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'ডানে সরান',
'conversations.taskKanban.title': 'টাস্ক',
'conversations.threadTodo.title': 'পরিকল্পনা',
'conversations.planReview.title': 'পরিকল্পনা পর্যালোচনা করুন',
'conversations.planReview.subtitle':
'চালানোর জন্য অনুমোদন করুন, বাতিল করতে প্রত্যাখ্যান করুন, অথবা সংশোধনের জন্য মতামত পাঠান।',
'conversations.planReview.approve': 'অনুমোদন ও চালান',
'conversations.planReview.reject': 'প্রত্যাখ্যান',
'conversations.planReview.feedbackLabel': 'পরিবর্তন অনুরোধ করুন',
'conversations.planReview.feedbackPlaceholder': 'কী পরিবর্তন করতে হবে বর্ণনা করুন…',
'conversations.planReview.sendFeedback': 'মতামত পাঠান',
'conversations.threadGoal.label': 'লক্ষ্য',
'conversations.threadGoal.setCta': 'লক্ষ্য নির্ধারণ',
'conversations.threadGoal.placeholder': 'এই থ্রেডে কী অর্জন করতে হবে?',
+8
View File
@@ -3126,6 +3126,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'Bewege dich nach rechts',
'conversations.taskKanban.title': 'Aufgaben',
'conversations.threadTodo.title': 'Plan',
'conversations.planReview.title': 'Plan prüfen',
'conversations.planReview.subtitle':
'Genehmigen zum Ausführen, ablehnen zum Verwerfen oder Feedback zum Überarbeiten senden.',
'conversations.planReview.approve': 'Genehmigen & ausführen',
'conversations.planReview.reject': 'Ablehnen',
'conversations.planReview.feedbackLabel': 'Änderungen anfordern',
'conversations.planReview.feedbackPlaceholder': 'Beschreiben Sie, was geändert werden soll…',
'conversations.planReview.sendFeedback': 'Feedback senden',
'conversations.threadGoal.label': 'Ziel',
'conversations.threadGoal.setCta': 'Ziel festlegen',
'conversations.threadGoal.placeholder': 'Was soll dieser Thread erreichen?',
+8
View File
@@ -3590,6 +3590,14 @@ const en: TranslationMap = {
'conversations.taskKanban.moveRight': 'Move right',
'conversations.taskKanban.title': 'Tasks',
'conversations.threadTodo.title': 'Plan',
'conversations.planReview.title': 'Review plan',
'conversations.planReview.subtitle':
'Approve to run it, reject to discard, or send feedback to revise.',
'conversations.planReview.approve': 'Approve & run',
'conversations.planReview.reject': 'Reject',
'conversations.planReview.feedbackLabel': 'Request changes',
'conversations.planReview.feedbackPlaceholder': 'Describe what to change…',
'conversations.planReview.sendFeedback': 'Send feedback',
// Thread-level goal chip (Codex-style per-thread completion contract).
'conversations.threadGoal.label': 'Goal',
'conversations.threadGoal.setCta': 'Set goal',
+8
View File
@@ -3104,6 +3104,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'Mover a la derecha',
'conversations.taskKanban.title': 'Tareas',
'conversations.threadTodo.title': 'Plan',
'conversations.planReview.title': 'Revisar plan',
'conversations.planReview.subtitle':
'Aprueba para ejecutarlo, recházalo para descartarlo o envía comentarios para revisarlo.',
'conversations.planReview.approve': 'Aprobar y ejecutar',
'conversations.planReview.reject': 'Rechazar',
'conversations.planReview.feedbackLabel': 'Solicitar cambios',
'conversations.planReview.feedbackPlaceholder': 'Describe qué cambiar…',
'conversations.planReview.sendFeedback': 'Enviar comentarios',
'conversations.threadGoal.label': 'Objetivo',
'conversations.threadGoal.setCta': 'Definir objetivo',
'conversations.threadGoal.placeholder': '¿Qué debe lograr este hilo?',
+8
View File
@@ -3119,6 +3119,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'Déplacer à droite',
'conversations.taskKanban.title': 'Tâches',
'conversations.threadTodo.title': 'Plan',
'conversations.planReview.title': 'Examiner le plan',
'conversations.planReview.subtitle':
'Approuvez pour lexécuter, rejetez pour labandonner ou envoyez un retour pour le réviser.',
'conversations.planReview.approve': 'Approuver et exécuter',
'conversations.planReview.reject': 'Rejeter',
'conversations.planReview.feedbackLabel': 'Demander des modifications',
'conversations.planReview.feedbackPlaceholder': 'Décrivez ce quil faut changer…',
'conversations.planReview.sendFeedback': 'Envoyer un retour',
'conversations.threadGoal.label': 'Objectif',
'conversations.threadGoal.setCta': 'Définir un objectif',
'conversations.threadGoal.placeholder': 'Que doit accomplir ce fil ?',
+8
View File
@@ -3055,6 +3055,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'दाएं ले जाएं',
'conversations.taskKanban.title': 'टास्क',
'conversations.threadTodo.title': 'योजना',
'conversations.planReview.title': 'योजना की समीक्षा करें',
'conversations.planReview.subtitle':
'इसे चलाने के लिए स्वीकृत करें, हटाने के लिए अस्वीकार करें, या संशोधित करने के लिए प्रतिक्रिया भेजें।',
'conversations.planReview.approve': 'स्वीकृत करें और चलाएँ',
'conversations.planReview.reject': 'अस्वीकार करें',
'conversations.planReview.feedbackLabel': 'बदलाव का अनुरोध करें',
'conversations.planReview.feedbackPlaceholder': 'वर्णन करें कि क्या बदलना है…',
'conversations.planReview.sendFeedback': 'प्रतिक्रिया भेजें',
'conversations.threadGoal.label': 'लक्ष्य',
'conversations.threadGoal.setCta': 'लक्ष्य सेट करें',
'conversations.threadGoal.placeholder': 'इस थ्रेड को क्या हासिल करना चाहिए?',
+8
View File
@@ -3058,6 +3058,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'Pindah ke kanan',
'conversations.taskKanban.title': 'Tugas',
'conversations.threadTodo.title': 'Rencana',
'conversations.planReview.title': 'Tinjau rencana',
'conversations.planReview.subtitle':
'Setujui untuk menjalankannya, tolak untuk membuangnya, atau kirim masukan untuk merevisinya.',
'conversations.planReview.approve': 'Setujui & jalankan',
'conversations.planReview.reject': 'Tolak',
'conversations.planReview.feedbackLabel': 'Minta perubahan',
'conversations.planReview.feedbackPlaceholder': 'Jelaskan apa yang harus diubah…',
'conversations.planReview.sendFeedback': 'Kirim masukan',
'conversations.threadGoal.label': 'Tujuan',
'conversations.threadGoal.setCta': 'Tetapkan tujuan',
'conversations.threadGoal.placeholder': 'Apa yang harus dicapai utas ini?',
+8
View File
@@ -3098,6 +3098,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'Sposta a destra',
'conversations.taskKanban.title': 'Attività',
'conversations.threadTodo.title': 'Piano',
'conversations.planReview.title': 'Rivedi il piano',
'conversations.planReview.subtitle':
'Approva per eseguirlo, rifiuta per scartarlo o invia un feedback per rivederlo.',
'conversations.planReview.approve': 'Approva ed esegui',
'conversations.planReview.reject': 'Rifiuta',
'conversations.planReview.feedbackLabel': 'Richiedi modifiche',
'conversations.planReview.feedbackPlaceholder': 'Descrivi cosa cambiare…',
'conversations.planReview.sendFeedback': 'Invia feedback',
'conversations.threadGoal.label': 'Obiettivo',
'conversations.threadGoal.setCta': 'Imposta obiettivo',
'conversations.threadGoal.placeholder': 'Cosa deve realizzare questa conversazione?',
+8
View File
@@ -3028,6 +3028,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': '오른쪽으로 이동',
'conversations.taskKanban.title': '작업',
'conversations.threadTodo.title': '계획',
'conversations.planReview.title': '계획 검토',
'conversations.planReview.subtitle':
'실행하려면 승인하고, 버리려면 거부하거나, 수정하려면 의견을 보내세요.',
'conversations.planReview.approve': '승인 후 실행',
'conversations.planReview.reject': '거부',
'conversations.planReview.feedbackLabel': '변경 요청',
'conversations.planReview.feedbackPlaceholder': '무엇을 바꿀지 설명하세요…',
'conversations.planReview.sendFeedback': '의견 보내기',
'conversations.threadGoal.label': '목표',
'conversations.threadGoal.setCta': '목표 설정',
'conversations.threadGoal.placeholder': '이 스레드가 무엇을 달성해야 하나요?',
+8
View File
@@ -3085,6 +3085,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'Przesuń w prawo',
'conversations.taskKanban.title': 'Zadania',
'conversations.threadTodo.title': 'Plan',
'conversations.planReview.title': 'Przejrzyj plan',
'conversations.planReview.subtitle':
'Zatwierdź, aby go uruchomić, odrzuć, aby go odrzucić, lub wyślij opinię, aby go poprawić.',
'conversations.planReview.approve': 'Zatwierdź i uruchom',
'conversations.planReview.reject': 'Odrzuć',
'conversations.planReview.feedbackLabel': 'Poproś o zmiany',
'conversations.planReview.feedbackPlaceholder': 'Opisz, co zmienić…',
'conversations.planReview.sendFeedback': 'Wyślij opinię',
'conversations.threadGoal.label': 'Cel',
'conversations.threadGoal.setCta': 'Ustaw cel',
'conversations.threadGoal.placeholder': 'Co ma osiągnąć ten wątek?',
+8
View File
@@ -3103,6 +3103,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'Mover para direita',
'conversations.taskKanban.title': 'Tarefas',
'conversations.threadTodo.title': 'Plano',
'conversations.planReview.title': 'Revisar plano',
'conversations.planReview.subtitle':
'Aprove para executá-lo, rejeite para descartá-lo ou envie comentários para revisá-lo.',
'conversations.planReview.approve': 'Aprovar e executar',
'conversations.planReview.reject': 'Rejeitar',
'conversations.planReview.feedbackLabel': 'Solicitar alterações',
'conversations.planReview.feedbackPlaceholder': 'Descreva o que mudar…',
'conversations.planReview.sendFeedback': 'Enviar comentários',
'conversations.threadGoal.label': 'Objetivo',
'conversations.threadGoal.setCta': 'Definir objetivo',
'conversations.threadGoal.placeholder': 'O que esta conversa deve realizar?',
+8
View File
@@ -3077,6 +3077,14 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': 'Переместить вправо',
'conversations.taskKanban.title': 'Задачи',
'conversations.threadTodo.title': 'План',
'conversations.planReview.title': 'Проверить план',
'conversations.planReview.subtitle':
'Одобрите для запуска, отклоните для отмены или отправьте отзыв для доработки.',
'conversations.planReview.approve': 'Одобрить и запустить',
'conversations.planReview.reject': 'Отклонить',
'conversations.planReview.feedbackLabel': 'Запросить изменения',
'conversations.planReview.feedbackPlaceholder': 'Опишите, что изменить…',
'conversations.planReview.sendFeedback': 'Отправить отзыв',
'conversations.threadGoal.label': 'Цель',
'conversations.threadGoal.setCta': 'Задать цель',
'conversations.threadGoal.placeholder': 'Чего должен достичь этот тред?',
+7
View File
@@ -2907,6 +2907,13 @@ const messages: TranslationMap = {
'conversations.taskKanban.moveRight': '向右移动',
'conversations.taskKanban.title': '任务',
'conversations.threadTodo.title': '计划',
'conversations.planReview.title': '审阅计划',
'conversations.planReview.subtitle': '批准以执行,拒绝以放弃,或发送反馈以修改。',
'conversations.planReview.approve': '批准并执行',
'conversations.planReview.reject': '拒绝',
'conversations.planReview.feedbackLabel': '请求修改',
'conversations.planReview.feedbackPlaceholder': '描述需要修改的内容…',
'conversations.planReview.sendFeedback': '发送反馈',
'conversations.threadGoal.label': '目标',
'conversations.threadGoal.setCta': '设定目标',
'conversations.threadGoal.placeholder': '这个对话需要完成什么?',
+25 -11
View File
@@ -100,6 +100,7 @@ import {
selectBackgroundProcesses,
} from './conversations/components/BackgroundProcessesPanel';
import { CitationChips, type MessageCitation } from './conversations/components/CitationChips';
import { PlanReviewCard } from './conversations/components/PlanReviewCard';
import { SubagentDrawer } from './conversations/components/SubagentDrawer';
import {
ThreadGoalEditorPanel,
@@ -114,7 +115,6 @@ import {
handleComposerSlashCommand,
} from './conversations/composerSendDecision';
import { useMemorySyncActive } from './conversations/hooks/useBackgroundActivity';
import { runDecidePlan } from './conversations/taskPlanActions';
import {
type AgentBubblePosition,
buildAcceptedInlineCompletion,
@@ -321,6 +321,9 @@ const Conversations = ({
const pendingApprovalByThread = useAppSelector(
state => state.chatRuntime.pendingApprovalByThread
);
const pendingPlanReviewByThread = useAppSelector(
state => state.chatRuntime.pendingPlanReviewByThread
);
const streamingAssistantByThread = useAppSelector(
state => state.chatRuntime.streamingAssistantByThread
);
@@ -1503,6 +1506,12 @@ const Conversations = ({
: undefined;
const selectedTaskBoard = selectedThreadId ? (taskBoardByThread[selectedThreadId] ?? null) : null;
const hasTaskBoard = Boolean(selectedTaskBoard?.cards.length);
// A plan the orchestrator parked for interactive review (request_plan_review
// gate). When present, the PlanReviewCard renders above the composer and
// resolves the parked turn; the todo strip stays read-only progress.
const pendingPlanReview = selectedThreadId
? (pendingPlanReviewByThread[selectedThreadId] ?? null)
: null;
const visibleMessages = messages.filter(msg => !msg.extraMetadata?.hidden);
const hasVisibleMessages = visibleMessages.length > 0;
const latestVisibleMessage = visibleMessages[visibleMessages.length - 1] ?? null;
@@ -2672,20 +2681,25 @@ const Conversations = ({
pinned above the composer. Distinct from the Intelligence-tab kanban
(global `user-tasks`). Renders nothing when the thread has no active
cards. */}
{/* Plan-mode review: the orchestrator parked the live turn on a
thread-scoped plan (request_plan_review gate). Surface it for the
user to Approve / Reject / send feedback on before anything executes;
the card resolves the parked turn via plan_review_decide. */}
{selectedThreadId && pendingPlanReview && (
// Key by request id so a re-parked (revised) plan — or a thread switch —
// remounts the card and resets its local decision/feedback state,
// matching the ApprovalRequestCard pattern above.
<PlanReviewCard
key={pendingPlanReview.requestId}
threadId={selectedThreadId}
review={pendingPlanReview}
/>
)}
{selectedThreadId && (
<ThreadTodoStrip
board={selectedTaskBoard}
disabled={!selectedThreadId}
onDecidePlan={(card, approve) => {
void runDecidePlan({
threadId: selectedThreadId,
card,
approve,
dispatch,
notify: setSendAdvisory,
t,
});
}}
onViewSession={card => {
if (!card.sessionThreadId) return;
// Navigation only — do NOT mark the thread active. activeThreadId
@@ -22,6 +22,7 @@ import chatRuntimeReducer, {
appendProcessingProse,
beginInferenceTurn,
setInferenceStatusForThread,
setPendingPlanReviewForThread,
setTaskBoardForThread,
setToolTimelineForThread,
} from '../../store/chatRuntimeSlice';
@@ -157,6 +158,14 @@ vi.mock('../../features/autocomplete/useAutocompleteSkillStatus', () => ({
// openUrl uses Tauri; stub it.
vi.mock('../../utils/openUrl', () => ({ openUrl: vi.fn() }));
// coreRpcClient: the PlanReviewCard resolves a parked plan via callCoreRpc.
// Preserve the real exports (e.g. CoreRpcError) and only stub the call.
const mockCallCoreRpc = vi.fn().mockResolvedValue({});
vi.mock('../../services/coreRpcClient', async orig => {
const actual = await orig<typeof import('../../services/coreRpcClient')>();
return { ...actual, callCoreRpc: (...args: unknown[]) => mockCallCoreRpc(...args) };
});
// coreState/store: getCoreStateSnapshot used by selectSocketStatus.
vi.mock('../../lib/coreState/store', () => ({
getCoreStateSnapshot: vi.fn(() => ({
@@ -2127,7 +2136,8 @@ describe('Conversations — open-session resume (View work)', () => {
expect(screen.getByTestId('route-path')).toHaveTextContent('/human');
});
it('approves a parked plan card from the thread todo strip', async () => {
it('approves a parked plan from the plan-review card', async () => {
mockCallCoreRpc.mockClear().mockResolvedValue({});
const thread = makeThread({ id: 'approve-thread', title: 'Approve thread' });
mockGetThreads.mockResolvedValue({ threads: [thread], count: 1 });
@@ -2135,33 +2145,26 @@ describe('Conversations — open-session resume (View work)', () => {
const selectedId = store.getState().thread.selectedThreadId ?? 'approve-thread';
await act(async () => {
store.dispatch(
setTaskBoardForThread({
setPendingPlanReviewForThread({
threadId: selectedId,
board: {
threadId: selectedId,
updatedAt: '',
cards: [
{
id: 'pc1',
title: 'Needs sign-off',
status: 'awaiting_approval',
order: 0,
updatedAt: '',
},
],
},
review: { requestId: 'pr-1', summary: 'Needs sign-off', steps: ['do the thing'] },
})
);
});
// The strip surfaces Approve/Reject only for parked cards; approving routes
// through onDecidePlan → runDecidePlan → threadApi.decidePlan.
const approveBtn = await screen.findByTitle('Approve');
// A parked plan surfaces the PlanReviewCard above the composer; "Approve &
// run" resolves the parked turn via the plan_review_decide RPC.
const approveBtn = await screen.findByText('Approve & run');
await act(async () => {
fireEvent.click(approveBtn);
});
await waitFor(() => expect(threadApi.decidePlan).toHaveBeenCalledWith(selectedId, 'pc1', true));
await waitFor(() =>
expect(mockCallCoreRpc).toHaveBeenCalledWith({
method: 'openhuman.plan_review_decide',
params: { request_id: 'pr-1', decision: 'approve', feedback: undefined },
})
);
});
});
@@ -0,0 +1,90 @@
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { PendingPlanReview } from '../../../store/chatRuntimeSlice';
import { PlanReviewCard } from './PlanReviewCard';
// Echo i18n keys so we can assert on the stable key string.
vi.mock('../../../lib/i18n/I18nContext', () => ({ useT: () => ({ t: (key: string) => key }) }));
const mockCallCoreRpc = vi.fn();
vi.mock('../../../services/coreRpcClient', () => ({
callCoreRpc: (...args: unknown[]) => mockCallCoreRpc(...args),
}));
const mockDispatch = vi.fn();
vi.mock('../../../store/hooks', () => ({ useAppDispatch: () => mockDispatch }));
function review(partial: Partial<PendingPlanReview> = {}): PendingPlanReview {
return {
requestId: 'r1',
summary: 'Ship the release',
steps: ['step one', 'step two'],
...partial,
};
}
describe('PlanReviewCard', () => {
beforeEach(() => {
mockCallCoreRpc.mockReset().mockResolvedValue({});
mockDispatch.mockReset();
});
it('renders the summary and ordered steps', () => {
render(<PlanReviewCard threadId="t1" review={review()} />);
expect(screen.getByText('Ship the release')).toBeInTheDocument();
expect(screen.getByText('step one')).toBeInTheDocument();
expect(screen.getByText('step two')).toBeInTheDocument();
});
it('approves via plan_review_decide and clears optimistically', async () => {
render(<PlanReviewCard threadId="t1" review={review()} />);
fireEvent.click(screen.getByText('conversations.planReview.approve'));
await waitFor(() =>
expect(mockCallCoreRpc).toHaveBeenCalledWith({
method: 'openhuman.plan_review_decide',
params: { request_id: 'r1', decision: 'approve', feedback: undefined },
})
);
expect(mockDispatch).toHaveBeenCalledTimes(1);
});
it('rejects via plan_review_decide', async () => {
render(<PlanReviewCard threadId="t1" review={review()} />);
fireEvent.click(screen.getByText('conversations.planReview.reject'));
await waitFor(() =>
expect(mockCallCoreRpc).toHaveBeenCalledWith({
method: 'openhuman.plan_review_decide',
params: { request_id: 'r1', decision: 'reject', feedback: undefined },
})
);
});
it('sends trimmed feedback as a revise decision; ignores blank input', async () => {
render(<PlanReviewCard threadId="t1" review={review()} />);
const send = screen.getByText('conversations.planReview.sendFeedback');
const textarea = screen.getByTestId('plan-review-feedback') as HTMLTextAreaElement;
// Blank → disabled, no call.
fireEvent.click(send);
expect(mockCallCoreRpc).not.toHaveBeenCalled();
fireEvent.change(textarea, { target: { value: ' add a verification step ' } });
fireEvent.click(send);
await waitFor(() =>
expect(mockCallCoreRpc).toHaveBeenCalledWith({
method: 'openhuman.plan_review_decide',
params: { request_id: 'r1', decision: 'revise', feedback: 'add a verification step' },
})
);
});
it('surfaces an error and stays mounted when the RPC fails', async () => {
mockCallCoreRpc.mockRejectedValueOnce(new Error('boom'));
render(<PlanReviewCard threadId="t1" review={review()} />);
fireEvent.click(screen.getByText('conversations.planReview.approve'));
await waitFor(() => expect(screen.getByText(/chat\.approval\.error/)).toBeInTheDocument());
// Not cleared on failure.
expect(mockDispatch).not.toHaveBeenCalled();
});
});
@@ -0,0 +1,162 @@
import debug from 'debug';
import React, { useState } from 'react';
import Button from '../../../components/ui/Button';
import { useT } from '../../../lib/i18n/I18nContext';
import { callCoreRpc } from '../../../services/coreRpcClient';
import {
clearPendingPlanReviewForThread,
type PendingPlanReview,
} from '../../../store/chatRuntimeSlice';
import { useAppDispatch } from '../../../store/hooks';
/**
* Plan-mode review surface (Codex/Claude-style). The orchestrator parked the
* live turn on a thread-scoped plan via the `request_plan_review` gate; this
* card surfaces the plan above the composer and resolves the parked turn via
* the `openhuman.plan_review_decide` RPC:
*
* - **Approve & run** → the turn resumes and executes the plan.
* - **Reject** → the turn resumes and stops without executing.
* - **Send feedback** → the turn resumes, re-plans from the free-text request,
* and re-parks for another review.
*
* Mirrors {@link ApprovalRequestCard}: it owns the decision RPC and clears
* itself optimistically; {@link ChatRuntimeProvider}'s turn-end handlers also
* clear the pending review if the turn ends.
*/
const log = debug('openhuman:chat:plan-review-card');
type Decision = 'approve' | 'reject' | 'revise';
interface Props {
threadId: string;
review: PendingPlanReview;
}
export const PlanReviewCard: React.FC<Props> = ({ threadId, review }) => {
const { t } = useT();
const dispatch = useAppDispatch();
const [feedback, setFeedback] = useState('');
const [deciding, setDeciding] = useState<Decision | null>(null);
const [errorMsg, setErrorMsg] = useState<string | null>(null);
const decide = async (decision: Decision, feedbackText?: string) => {
if (deciding) return;
setDeciding(decision);
setErrorMsg(null);
try {
await callCoreRpc({
method: 'openhuman.plan_review_decide',
params: { request_id: review.requestId, decision, feedback: feedbackText },
});
// Resolve optimistically; ChatRuntimeProvider also clears on turn end.
dispatch(clearPendingPlanReviewForThread({ threadId }));
} catch (e) {
log('plan_review_decide failed: %o', e);
setErrorMsg(t('chat.approval.error'));
setDeciding(null);
}
};
const submitFeedback = () => {
const trimmed = feedback.trim();
if (!trimmed) return;
void decide('revise', trimmed);
};
return (
<div
role="alertdialog"
aria-label={t('conversations.planReview.title')}
data-testid="plan-review-card"
className="mb-2 rounded-xl border border-ocean-300 bg-ocean-50 p-3 text-sm shadow-sm dark:border-ocean-700 dark:bg-ocean-950">
<div className="flex items-start gap-2">
<span aria-hidden className="text-base leading-none text-ocean-700 dark:text-ocean-200">
🗺
</span>
<div className="min-w-0 flex-1">
<p className="font-semibold text-ocean-900 dark:text-ocean-100">
{t('conversations.planReview.title')}
</p>
<p className="mt-1 break-words text-ocean-800/90 dark:text-ocean-200/90">
{review.summary?.trim() || t('conversations.planReview.subtitle')}
</p>
{review.steps.length > 0 && (
<ol className="mt-2 max-h-56 list-decimal overflow-y-auto pl-6 text-stone-700 dark:text-neutral-200">
{review.steps.map((step, i) => (
<li key={i} className="break-words">
{step}
</li>
))}
</ol>
)}
{errorMsg && <p className="mt-2 text-xs text-coral"> {errorMsg}</p>}
<div className="mt-3 flex flex-wrap items-center gap-2">
<Button
variant="primary"
size="sm"
data-analytics-id="plan-review-approve"
onClick={() => void decide('approve')}
disabled={deciding !== null}>
{deciding === 'approve'
? t('chat.approval.deciding')
: t('conversations.planReview.approve')}
</Button>
<Button
variant="secondary"
size="sm"
data-analytics-id="plan-review-reject"
onClick={() => void decide('reject')}
disabled={deciding !== null}>
{deciding === 'reject'
? t('chat.approval.deciding')
: t('conversations.planReview.reject')}
</Button>
</div>
<div className="mt-3">
<label
htmlFor="plan-review-feedback"
className="mb-1 block text-xs font-medium text-ocean-800/80 dark:text-ocean-200/80">
{t('conversations.planReview.feedbackLabel')}
</label>
<textarea
id="plan-review-feedback"
data-testid="plan-review-feedback"
value={feedback}
onChange={e => setFeedback(e.target.value)}
onKeyDown={e => {
if ((e.metaKey || e.ctrlKey) && e.key === 'Enter') {
e.preventDefault();
submitFeedback();
}
}}
rows={2}
disabled={deciding !== null}
placeholder={t('conversations.planReview.feedbackPlaceholder')}
className="w-full resize-y rounded-lg border border-ocean-200 bg-white px-2.5 py-1.5 text-sm text-ink shadow-inner outline-none focus:border-ocean-400 disabled:opacity-50 dark:border-ocean-800 dark:bg-neutral-950 dark:text-neutral-100"
/>
<div className="mt-1.5 flex justify-end">
<Button
variant="secondary"
size="sm"
data-analytics-id="plan-review-send-feedback"
onClick={submitFeedback}
disabled={deciding !== null || feedback.trim().length === 0}>
{deciding === 'revise'
? t('chat.approval.deciding')
: t('conversations.planReview.sendFeedback')}
</Button>
</div>
</div>
</div>
</div>
</div>
);
};
export default PlanReviewCard;
@@ -8,8 +8,8 @@ vi.mock('../../services/api/threadApi', () => ({
threadApi: { decidePlan: (...args: unknown[]) => mockDecidePlan(...args) },
}));
function card(): TaskBoardCard {
return { id: 'c1', title: 'T', status: 'awaiting_approval', order: 0, updatedAt: '' };
function card(id = 'c1'): TaskBoardCard {
return { id, title: 'T', status: 'awaiting_approval', order: 0, updatedAt: '' };
}
const t = (key: string) => key;
+21 -2
View File
@@ -9,6 +9,7 @@ import {
type ChatDoneEvent,
type ChatInferenceStartEvent,
type ChatIterationStartEvent,
type ChatPlanReviewRequestEvent,
type ChatSegmentEvent,
type ChatSubagentDoneEvent,
type ChatSubagentTextDeltaEvent,
@@ -27,6 +28,7 @@ import {
clearInferenceStatusForThread,
clearParallelRequest,
clearPendingApprovalForThread,
clearPendingPlanReviewForThread,
clearProcessingForThread,
clearStreamingAssistantForThread,
endInferenceTurn,
@@ -38,6 +40,7 @@ import {
setInferenceStatusForThread,
setParallelStream,
setPendingApprovalForThread,
setPendingPlanReviewForThread,
setStreamingAssistantForThread,
setTaskBoardForThread,
setToolTimelineForThread,
@@ -1031,6 +1034,18 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => {
})
);
},
onPlanReviewRequest: (event: ChatPlanReviewRequestEvent) => {
rtLog('plan_review_request', { thread: event.thread_id, request: event.request_id });
const steps = Array.isArray(event.args?.steps)
? event.args.steps.filter((s): s is string => typeof s === 'string')
: [];
dispatch(
setPendingPlanReviewForThread({
threadId: event.thread_id,
review: { requestId: event.request_id, summary: event.message, steps },
})
);
},
onDone: event => {
const eventKey = `done:${event.thread_id}:${event.request_id ?? 'none'}`;
if (
@@ -1105,6 +1120,7 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => {
dispatch(clearInferenceStatusForThread({ threadId: event.thread_id }));
dispatch(clearStreamingAssistantForThread({ threadId: event.thread_id }));
dispatch(clearPendingApprovalForThread({ threadId: event.thread_id }));
dispatch(clearPendingPlanReviewForThread({ threadId: event.thread_id }));
const existing = store.getState().chatRuntime.toolTimelineByThread[event.thread_id] ?? [];
if (existing.length > 0) {
@@ -1239,6 +1255,7 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => {
dispatch(clearInferenceStatusForThread({ threadId: event.thread_id }));
dispatch(clearStreamingAssistantForThread({ threadId: event.thread_id }));
dispatch(clearPendingApprovalForThread({ threadId: event.thread_id }));
dispatch(clearPendingPlanReviewForThread({ threadId: event.thread_id }));
const existing = store.getState().chatRuntime.toolTimelineByThread[event.thread_id] ?? [];
if (existing.length > 0) {
@@ -1317,9 +1334,11 @@ const ChatRuntimeProvider = ({ children }: { children: React.ReactNode }) => {
});
for (const threadId of threadIds) {
dispatch(clearInferenceStatusForThread({ threadId }));
// Clear any parked approval too: a disconnect before onDone/onError would
// otherwise leave the approval card stuck for a turn that can't complete.
// Clear any parked approval/plan-review too: a disconnect before
// onDone/onError would otherwise leave the card stuck for a turn that
// can't complete.
dispatch(clearPendingApprovalForThread({ threadId }));
dispatch(clearPendingPlanReviewForThread({ threadId }));
dispatch(endInferenceTurn({ threadId }));
}
// A disconnect kills every in-flight turn on the dead session, so clear all
@@ -11,6 +11,7 @@ import {
enqueueFollowup,
registerParallelRequest,
resetSessionTokenUsage,
setPendingPlanReviewForThread,
} from '../../store/chatRuntimeSlice';
import { setStatusForUser } from '../../store/socketSlice';
import { clearAllThreads, loadThreads, setSelectedThread } from '../../store/threadSlice';
@@ -391,6 +392,63 @@ describe('ChatRuntimeProvider — dedupe, proactive resolution, mid-turn invaria
await waitFor(() => expect(mockRefetchSnapshot).toHaveBeenCalledTimes(1));
});
it('stores a parked plan review from the plan_review_request event', () => {
const listeners = renderProvider();
act(() => {
listeners.onPlanReviewRequest?.({
thread_id: 't-plan',
request_id: 'pr-1',
message: 'Ship the release',
args: { steps: ['build', 'verify'] },
});
});
const review = store.getState().chatRuntime.pendingPlanReviewByThread['t-plan'];
expect(review).toEqual({
requestId: 'pr-1',
summary: 'Ship the release',
steps: ['build', 'verify'],
});
});
it('clears a parked plan review when the turn ends or errors', () => {
const listeners = renderProvider();
const park = () =>
store.dispatch(
setPendingPlanReviewForThread({
threadId: 't-plan',
review: { requestId: 'pr-1', summary: 'Plan', steps: ['a'] },
})
);
// chat_done clears the (possibly expired) parked review.
park();
act(() => {
listeners.onDone?.({
thread_id: 't-plan',
request_id: 'r-plan',
full_response: 'done',
rounds_used: 1,
total_input_tokens: 1,
total_output_tokens: 1,
segment_total: 1,
});
});
expect(store.getState().chatRuntime.pendingPlanReviewByThread['t-plan']).toBeUndefined();
// chat_error also clears it (cancelled/errored parked turn).
park();
act(() => {
listeners.onError?.({
thread_id: 't-plan',
request_id: 'r-plan',
message: 'boom',
error_type: 'inference',
round: 1,
});
});
expect(store.getState().chatRuntime.pendingPlanReviewByThread['t-plan']).toBeUndefined();
});
it('flushes queued follow-ups into the transcript when a turn ends', async () => {
const listeners = renderProvider();
store.dispatch(
+28
View File
@@ -148,6 +148,22 @@ export interface ChatApprovalRequestEvent {
args?: Record<string, unknown>;
}
/**
* Interactive plan-review request: the orchestrator parked the live turn on a
* thread-scoped plan the user must review before execution (Codex/Claude plan
* mode). Resolved via the `openhuman.plan_review_decide` RPC. Bridged from the
* Rust `DomainEvent::PlanReviewRequested` by the web channel.
*/
export interface ChatPlanReviewRequestEvent {
thread_id: string;
client_id?: string;
request_id: string;
/** One-line summary of the plan. */
message: string;
/** `{ steps: string[] }` — the ordered plan items shown in the review card. */
args?: { steps?: string[] };
}
/**
* Lowercase variant of the Rust `ArtifactKind` enum surfaced on
* artifact lifecycle socket events. Mirrors the slugs produced by
@@ -441,6 +457,7 @@ export interface ChatEventListeners {
onTaskBoardUpdated?: (event: ChatTaskBoardUpdatedEvent) => void;
onProactiveMessage?: (event: ProactiveMessageEvent) => void;
onApprovalRequest?: (event: ChatApprovalRequestEvent) => void;
onPlanReviewRequest?: (event: ChatPlanReviewRequestEvent) => void;
onArtifactReady?: (event: ArtifactReadyEvent) => void;
onArtifactFailed?: (event: ArtifactFailedEvent) => void;
onDone?: (event: ChatDoneEvent) => void;
@@ -476,6 +493,7 @@ export function subscribeChatEvents(listeners: ChatEventListeners): () => void {
taskBoardUpdated: 'task_board_updated',
proactiveMessage: 'proactive_message',
approvalRequest: 'approval_request',
planReviewRequest: 'plan_review_request',
artifactReady: 'artifact_ready',
artifactFailed: 'artifact_failed',
done: 'chat_done',
@@ -797,6 +815,16 @@ export function subscribeChatEvents(listeners: ChatEventListeners): () => void {
handlers.push([EVENTS.approvalRequest, cb]);
}
if (listeners.onPlanReviewRequest) {
const cb = (payload: unknown) => {
const e = payload as ChatPlanReviewRequestEvent;
chatLog('%s thread_id=%s request_id=%s', EVENTS.planReviewRequest, e.thread_id, e.request_id);
listeners.onPlanReviewRequest?.(e);
};
socket.on(EVENTS.planReviewRequest, cb);
handlers.push([EVENTS.planReviewRequest, cb]);
}
// Artifact lifecycle events (#2779). The Rust subscriber in
// `channels/providers/web::ArtifactSurfaceSubscriber` packs the
// artifact payload into the generic `args` field of the wire
+32
View File
@@ -247,6 +247,20 @@ export interface PendingApproval {
toolkit?: string;
}
/**
* A thread-scoped plan the orchestrator parked for interactive review (Codex/
* Claude plan mode). Surfaced from the `plan_review_request` socket event and
* resolved via the `openhuman.plan_review_decide` RPC. The parked agent turn
* blocks until the user approves / rejects / sends feedback.
*/
export interface PendingPlanReview {
requestId: string;
/** One-line summary of the plan. */
summary: string;
/** Ordered plan steps to display for review. */
steps: string[];
}
/**
* Lifecycle status of a single agent-generated artifact, as projected
* onto the chat runtime per thread.
@@ -327,6 +341,7 @@ interface ChatRuntimeState {
taskBoardByThread: Record<string, TaskBoard>;
inferenceTurnLifecycleByThread: Record<string, InferenceTurnLifecycle>;
pendingApprovalByThread: Record<string, PendingApproval>;
pendingPlanReviewByThread: Record<string, PendingPlanReview>;
/**
* Per-thread artifact ledger. Snapshots are upserted on
* `artifact_ready` / `artifact_failed` socket events keyed on
@@ -382,6 +397,7 @@ const initialState: ChatRuntimeState = {
taskBoardByThread: {},
inferenceTurnLifecycleByThread: {},
pendingApprovalByThread: {},
pendingPlanReviewByThread: {},
artifactsByThread: {},
sessionTokenUsage: {
inputTokens: 0,
@@ -861,6 +877,15 @@ const chatRuntimeSlice = createSlice({
clearPendingApprovalForThread: (state, action: PayloadAction<{ threadId: string }>) => {
delete state.pendingApprovalByThread[action.payload.threadId];
},
setPendingPlanReviewForThread: (
state,
action: PayloadAction<{ threadId: string; review: PendingPlanReview }>
) => {
state.pendingPlanReviewByThread[action.payload.threadId] = action.payload.review;
},
clearPendingPlanReviewForThread: (state, action: PayloadAction<{ threadId: string }>) => {
delete state.pendingPlanReviewByThread[action.payload.threadId];
},
/**
* Mark a producer-tool call as in-flight so the `ArtifactCard` can
* render a spinner before any ready/failed event arrives. Caller
@@ -1040,6 +1065,7 @@ const chatRuntimeSlice = createSlice({
delete state.taskBoardByThread[action.payload.threadId];
delete state.inferenceTurnLifecycleByThread[action.payload.threadId];
delete state.pendingApprovalByThread[action.payload.threadId];
delete state.pendingPlanReviewByThread[action.payload.threadId];
delete state.queueStatusByThread[action.payload.threadId];
delete state.queuedFollowupsByThread[action.payload.threadId];
// Note: artifactsByThread intentionally NOT cleared here. The
@@ -1058,6 +1084,7 @@ const chatRuntimeSlice = createSlice({
state.taskBoardByThread = {};
state.inferenceTurnLifecycleByThread = {};
state.pendingApprovalByThread = {};
state.pendingPlanReviewByThread = {};
state.artifactsByThread = {};
state.queueStatusByThread = {};
state.queuedFollowupsByThread = {};
@@ -1113,6 +1140,9 @@ const chatRuntimeSlice = createSlice({
// Snapshots don't carry pending-approval payloads; drop any stale in-memory
// approval so the card reflects the rehydrated core truth, not pre-drift state.
delete state.pendingApprovalByThread[threadId];
// Likewise drop any stale parked plan review — its gate future cannot
// survive a rehydrate, so the card must not linger.
delete state.pendingPlanReviewByThread[threadId];
if (snapshot.taskBoard) {
state.taskBoardByThread[threadId] = snapshot.taskBoard;
}
@@ -1210,6 +1240,8 @@ export const {
clearTaskBoardForThread,
setPendingApprovalForThread,
clearPendingApprovalForThread,
setPendingPlanReviewForThread,
clearPendingPlanReviewForThread,
upsertArtifactInProgressForThread,
upsertArtifactReadyForThread,
upsertArtifactFailedForThread,
+1
View File
@@ -185,6 +185,7 @@ Canonical mapping of every product feature to its test source(s). Drives gap-fil
| 4.2.4 | Parallel inference (cross-thread + within-thread forked turns) | RU+VU | `src/openhuman/channels/providers/web_tests.rs`, `app/src/store/__tests__/chatRuntimeSlice.test.ts`, `app/src/providers/__tests__/ChatRuntimeProvider.test.tsx` | 🟡 | Concurrent same-/cross-thread dispatch, cooperative `CancellationToken` teardown, and parallel-lane stream routing covered; dedicated WD E2E is a follow-up |
| 4.2.5 | Per-thread todo list (plan strip above composer) | RU+VU+WD | `src/openhuman/agent/tools/todo.rs`, `app/src/pages/conversations/components/ThreadTodoStrip.test.tsx`, `app/test/e2e/specs/chat-thread-todo-strip.spec.ts` | ✅ | Read-only thread-scoped todo strip fed by `task_board_updated`; agent `todo` tool guidance + thread binding; E2E drives a `todo` tool call and asserts the card renders |
| 4.2.6 | Background-activity panel (chat-header Background tasks button) | VU+WD | `app/src/pages/conversations/hooks/useBackgroundActivity.test.ts`, `app/src/pages/conversations/components/__tests__/BackgroundActivityRows.test.tsx`, `app/test/e2e/specs/chat-background-activity-panel.spec.ts` | ✅ | View-only panel surfacing this chat's async sub-agents + global cron jobs, subconscious/heartbeat status, and memory syncing; freshness-only "Syncing now" labeling; E2E opens the panel and asserts its sections render and close |
| 4.2.7 | Plan-mode review (Approve / Reject / Send-feedback before execute) | RU+RI+VU | `src/openhuman/plan_review/gate.rs`, `src/openhuman/plan_review/tool.rs`, `src/openhuman/plan_review/schemas.rs`, `tests/json_rpc_e2e.rs`, `app/src/pages/conversations/components/PlanReviewCard.test.tsx`, `app/src/pages/__tests__/Conversations.render.test.tsx` | ✅ | Interactive turns call `request_plan_review`, which parks the LIVE turn on the in-memory `PlanReviewGate` (oneshot) until the user decides; `plan_review_request` socket event drives `PlanReviewCard`, which resolves via `openhuman.plan_review_decide` (approve resumes-and-executes / reject resumes-and-stops / revise resumes-with-feedback). RU covers gate park/resolve/timeout + tool auto-approve + parking; RI covers the decide RPC; VU covers the card + provider wiring. WD E2E (agent-driven park flow) tracked as follow-up |
### 4.3 Tool Invocation
+3
View File
@@ -149,6 +149,8 @@ fn build_registered_controllers() -> Vec<RegisteredController> {
controllers.extend(crate::openhuman::security::all_security_registered_controllers());
// Interactive approval workflow (#1339 — gate external-effect tool calls)
controllers.extend(crate::openhuman::approval::all_approval_registered_controllers());
// Interactive plan-review gate — parks a live turn on a thread-scoped plan
controllers.extend(crate::openhuman::plan_review::all_plan_review_registered_controllers());
// Agent-generated artifact storage, retrieval, and lifecycle management
controllers.extend(crate::openhuman::artifacts::all_artifacts_registered_controllers());
// Background heartbeat loop controls
@@ -370,6 +372,7 @@ fn build_declared_controller_schemas() -> Vec<ControllerSchema> {
schemas.extend(crate::openhuman::keyring_consent::all_keyring_consent_controller_schemas());
schemas.extend(crate::openhuman::security::all_security_controller_schemas());
schemas.extend(crate::openhuman::approval::all_approval_controller_schemas());
schemas.extend(crate::openhuman::plan_review::all_plan_review_controller_schemas());
schemas.extend(crate::openhuman::artifacts::all_artifacts_controller_schemas());
schemas.extend(crate::openhuman::heartbeat::all_heartbeat_controller_schemas());
schemas.extend(crate::openhuman::http_host::all_http_host_controller_schemas());
+30
View File
@@ -467,6 +467,32 @@ pub enum DomainEvent {
decision: String,
},
// ── Plan review (interactive plan-mode gate) ────────────────────────
/// An interactive turn parked on a thread-scoped plan the user must
/// review before execution. Published by
/// [`crate::openhuman::plan_review::gate::PlanReviewGate::request_review`]
/// and bridged to the web channel as a `plan_review_request` socket event.
PlanReviewRequested {
/// Unique id correlating the decision back to the parked turn.
request_id: String,
/// Chat thread the parked turn belongs to (routing). `None` for
/// non-chat callers (which auto-approve and never park here).
thread_id: Option<String>,
/// Socket.IO client id (room) to surface the review to, when known.
client_id: Option<String>,
/// One-line description of the plan.
summary: String,
/// Ordered plan steps shown in the review card.
steps: Vec<String>,
},
/// User resolved a parked plan review. Published after the gate's parked
/// future wakes. `decision` is `"approve"` / `"reject"` / `"revise"`
/// (revise feedback is user content and is intentionally omitted).
PlanReviewDecided {
request_id: String,
decision: String,
},
// ── Artifacts ───────────────────────────────────────────────────────
/// An artifact transitioned to [`ArtifactStatus::Ready`] — file
/// is on disk and ready to be downloaded. Published by
@@ -1271,6 +1297,8 @@ impl DomainEvent {
| Self::ApprovalGateOverrideIgnored { .. }
| Self::ApprovalGateDisabled { .. } => "approval",
Self::PlanReviewRequested { .. } | Self::PlanReviewDecided { .. } => "plan_review",
Self::ArtifactReady { .. }
| Self::ArtifactFailed { .. }
| Self::ArtifactPending { .. } => "artifact",
@@ -1396,6 +1424,8 @@ impl DomainEvent {
Self::SessionExpired { .. } => "SessionExpired",
Self::ApprovalRequested { .. } => "ApprovalRequested",
Self::ApprovalDecided { .. } => "ApprovalDecided",
Self::PlanReviewRequested { .. } => "PlanReviewRequested",
Self::PlanReviewDecided { .. } => "PlanReviewDecided",
Self::ApprovalGateOverrideIgnored { .. } => "ApprovalGateOverrideIgnored",
Self::ApprovalGateDisabled { .. } => "ApprovalGateDisabled",
Self::ArtifactReady { .. } => "ArtifactReady",
+12 -7
View File
@@ -2549,6 +2549,16 @@ pub async fn bootstrap_core_runtime(host_kind: crate::core::types::HostKind) {
},
);
}
// Bridge interactive web-surface events to the frontend: ApprovalRequested →
// `approval_request` AND PlanReviewRequested → `plan_review_request` (both
// handled by the same subscriber). Registered UNCONDITIONALLY here on the
// always-run serve boot — the plan-review gate is independent of the approval
// gate and parks turns even when `OPENHUMAN_APPROVAL_GATE=0`, while
// `start_channels` is skipped for web-chat-only cores. Without this an
// unguarded standalone/CLI/Docker core would park a plan review that never
// reaches the UI and dies at the gate TTL. Idempotent (Once-guarded).
crate::openhuman::channels::providers::web::register_approval_surface_subscriber();
if decision.install_gate {
// Per-launch correlation token for the approval gate. This is
// a fresh UUID every boot — it is NOT derived from the
@@ -2565,13 +2575,8 @@ pub async fn bootstrap_core_runtime(host_kind: crate::core::types::HostKind) {
"[runtime] approval gate installed (on by default; set OPENHUMAN_APPROVAL_GATE=0 to disable, session_id={session_id}) — \
Prompt-class external-effect tool calls park for approval in interactive chat turns"
);
// Bridge ApprovalRequested → `approval_request` web socket event. This MUST
// be registered here on the always-run serve boot, not only inside
// `start_channels` — that path is skipped when no messaging integrations
// (Telegram/Discord/…) are configured, which is the common web-chat-only
// case. Without this, the gate parks and publishes but nothing reaches the
// frontend → every prompt dies at the TTL. Idempotent (Once-guarded).
crate::openhuman::channels::providers::web::register_approval_surface_subscriber();
// (The approval/plan-review surface bridge is registered unconditionally
// above — it must run even when this gate-install branch is skipped.)
crate::openhuman::channels::providers::web::register_artifact_surface_subscriber();
} else {
log::error!(
+10
View File
@@ -221,6 +221,16 @@ pub(super) const CAPABILITIES: &[Capability] = &[
status: CapabilityStatus::Beta,
privacy: None,
},
Capability {
id: "conversation.plan_review",
name: "Plan Review",
domain: "conversation",
category: CapabilityCategory::Conversation,
description: "Pause an interactive turn for review whenever the assistant proposes a thread-scoped plan (a multi-step to-do list with its objective). Review the whole plan once above the composer, then Approve to run it, Reject to discard it, or send feedback to have the assistant revise and re-propose — nothing executes until you approve. Background and scheduled runs are never gated.",
how_to: "Conversations > review the plan card above the composer when the assistant lays out a multi-step plan",
status: CapabilityStatus::Beta,
privacy: None,
},
Capability {
id: "conversation.background_monitors",
name: "Background Monitors",
+16 -7
View File
@@ -193,17 +193,26 @@ pub(super) fn pick_next_todo(
/// Whether a card must be parked at `awaiting_approval` before it can run.
///
/// The global `require_task_plan_approval` setting applies *unless* the card is
/// explicitly marked `approval_mode = NotRequired` — a per-card opt-out for
/// tasks that have already cleared human review (e.g. approved out of the
/// `task-sources` inbox onto `user-tasks`). Per-card opt-out wins over the
/// global default; without this, an already-approved card would be re-parked
/// and stranded.
/// Per-card `approval_mode` is authoritative when set; the global
/// `require_task_plan_approval` setting is only the fallback for cards with no
/// explicit preference:
/// - `Required` → always park, **even when the global default is off**. The
/// interactive plan-review gate (WebChat turns, see
/// [`crate::openhuman::agent::tools::todo`]) stamps `Required`, and that
/// review must hold regardless of the global switch — otherwise an
/// interactive plan would execute before the user ever sees the review card.
/// - `NotRequired` → never park (already cleared human review, e.g. approved
/// out of the `task-sources` inbox onto `user-tasks`).
/// - unset → fall back to the global default.
pub(super) fn requires_plan_approval(
global_required: bool,
approval_mode: Option<&TaskApprovalMode>,
) -> bool {
global_required && approval_mode != Some(&TaskApprovalMode::NotRequired)
match approval_mode {
Some(TaskApprovalMode::Required) => true,
Some(TaskApprovalMode::NotRequired) => false,
None => global_required,
}
}
pub(super) fn card_urgency(card: &TaskBoardCard) -> f64 {
+10 -4
View File
@@ -221,22 +221,28 @@ fn poller_agent_only_returns_none_when_all_unassigned() {
#[test]
fn approval_gate_respects_global_and_per_card_optout() {
// Global off → never park.
// Unset → follow the global default.
assert!(!requires_plan_approval(false, None));
assert!(!requires_plan_approval(
assert!(requires_plan_approval(true, None));
// Explicit `Required` always parks — even when the global default is off
// (the interactive plan-review gate stamps `Required`).
assert!(requires_plan_approval(
false,
Some(&TaskApprovalMode::Required)
));
// Global on → park, unless the card opts out via NotRequired.
assert!(requires_plan_approval(true, None));
assert!(requires_plan_approval(
true,
Some(&TaskApprovalMode::Required)
));
// Explicit `NotRequired` never parks, regardless of the global default.
assert!(!requires_plan_approval(
true,
Some(&TaskApprovalMode::NotRequired)
));
assert!(!requires_plan_approval(
false,
Some(&TaskApprovalMode::NotRequired)
));
}
#[test]
+6
View File
@@ -182,6 +182,12 @@ impl Tool for TodoTool {
}
async fn default_task_approval_mode() -> Option<TaskApprovalMode> {
// Interactive plan review is handled by the `request_plan_review` gate
// (it parks the live turn), NOT by stamping conversation-thread cards: the
// background dispatcher never sweeps conversation boards, so a card status
// can't gate a chat turn. This default therefore just carries the
// config-driven behaviour for the dispatched boards (`user-tasks` /
// `task-sources`).
match crate::openhuman::config::ops::load_config_with_timeout().await {
Ok(config) => Some(if config.autonomy.require_task_plan_approval {
TaskApprovalMode::Required
@@ -214,15 +214,24 @@ named = [
# arithmetic to the leaf (which once computed "24h ago" ~10 months off and
# missed the latest data). Never hand-compute Unix seconds.
"resolve_time",
# Coding-harness coordination primitives from #1208. `todowrite`
# gives the orchestrator a shared todo store to track multi-step
# work across delegations; `plan_exit` is the stable marker that
# the (forthcoming) plan→build mode runner consumes when a planner
# subagent hands a plan back up. Editing/navigation primitives
# (grep/glob/edit/apply_patch/lsp) intentionally stay with
# downstream agents — the orchestrator coordinates, it doesn't
# touch files itself.
"todowrite",
# Coding-harness coordination primitives from #1208. `todo` is the
# registered unified thread task-board tool (`TodoTool::name() == "todo"`);
# it gives the orchestrator a shared todo store to track multi-step work
# across delegations and is what the interactive plan-review gate hooks
# (WebChat turns stamp new cards `approval_mode = Required` → parked for
# user review). The named scope matches tool names exactly, so the
# orchestrator must list `todo` (not the legacy `todowrite` alias, which
# resolves to no registered tool) to be able to call it. `plan_exit` is the
# stable marker that the (forthcoming) plan→build mode runner consumes when
# a planner subagent hands a plan back up. Editing/navigation primitives
# (grep/glob/edit/apply_patch/lsp) intentionally stay with downstream
# agents — the orchestrator coordinates, it doesn't touch files itself.
"todo",
# Interactive plan-review gate (Codex/Claude plan mode). After laying out a
# multi-step plan on an interactive turn, call `request_plan_review` to PAUSE
# the turn until the user approves / rejects / sends feedback — nothing
# executes until they approve. Auto-approves on non-interactive turns.
"request_plan_review",
# Thread-level goal (Codex-style per-thread completion contract). `goal_set`
# records the durable objective for THIS thread when a non-trivial request
# lands (the orchestrator is authoritative — it always creates or replaces);
@@ -66,6 +66,7 @@ You can open and operate native apps on this machine, but you do it by **delegat
- **Structured handoffs** — Prefer delegation fields like `objective`, `evidence`, `constraints`, `must_not_assume`, `expected_output`, and `citation_requirement`. Put only observed facts, file paths, URLs, ids, or tool outputs in `evidence`.
- **Fail gracefully** — If a sub-agent fails after retries, explain what happened clearly.
- **Escalate when appropriate** — If orchestration is the wrong mode or a specialist cannot make progress, hand control back to OpenHuman Core with a concise explanation and let Core handle general interactions.
- **Plan before you execute (interactive plan review).** For any interactive request that needs a thread-scoped plan — a multi-step task (3+ steps) or a durable objective for this conversation — call **`request_plan_review`** with a one-line `summary` and the ordered `steps` **before doing any of the work and before creating any `todo` cards**. The review card shows the user the `steps` you pass, so you do **not** need a `todo` plan to exist yet. That call PAUSES your turn until the user decides, and its result tells you what to do: `approved`**now** lay the plan out with the `todo` tool (one card per step) and execute it; `rejected` → do **not** execute and do **not** create cards, briefly ask what they want instead; `revise` → the result carries their feedback, so call `request_plan_review` again with the revised `steps` (still no cards yet). Creating `todo` cards only **after** approval keeps a rejected/revised plan from lingering pinned on the board. Never start executing until `request_plan_review` returns `approved`. Trivial single-step requests need no plan and no review — answer directly. (On non-interactive turns `request_plan_review` auto-approves, so this same flow is safe in cron / subconscious / CLI runs.)
**Scheduling rule of thumb.** Route reminders, one-shot jobs, recurring jobs, and job list/remove to `schedule_task`; the scheduler specialist owns the schedule shapes, cron expressions, and worked examples. Two rules still bind you directly:
@@ -29,7 +29,7 @@ pub fn register_approval_surface_subscriber() {
Some(handle) => {
let _ = APPROVAL_SURFACE_HANDLE.set(handle);
log::info!(
"[web-channel] approval-surface subscriber registered (domain=approval) — will bridge ApprovalRequested → approval_request socket event"
"[web-channel] approval-surface subscriber registered (domains=approval,plan_review) — bridges ApprovalRequested → approval_request and PlanReviewRequested → plan_review_request socket events"
);
}
None => {
@@ -218,7 +218,7 @@ impl EventHandler for ApprovalSurfaceSubscriber {
}
fn domains(&self) -> Option<&[&str]> {
Some(&["approval"])
Some(&["approval", "plan_review"])
}
async fn handle(&self, event: &DomainEvent) {
@@ -257,6 +257,39 @@ impl EventHandler for ApprovalSurfaceSubscriber {
);
}
}
} else if let DomainEvent::PlanReviewRequested {
request_id,
thread_id,
client_id,
summary,
steps,
} = event
{
match (thread_id, client_id) {
(Some(thread_id), Some(client_id)) => {
log::info!(
"[web-channel] plan-review-surface emitting plan_review_request request_id={request_id} thread_id={thread_id} client_id={client_id} steps={}",
steps.len()
);
publish_web_channel_event(WebChannelEvent {
event: "plan_review_request".to_string(),
client_id: client_id.clone(),
thread_id: thread_id.clone(),
request_id: request_id.clone(),
tool_name: Some("request_plan_review".to_string()),
message: Some(summary.clone()),
args: Some(serde_json::json!({ "steps": steps })),
..Default::default()
});
}
_ => {
log::warn!(
"[web-channel] plan-review-surface received PlanReviewRequested request_id={request_id} but thread_id/client_id absent (thread={}, client={}) — NOT surfacing",
thread_id.is_some(),
client_id.is_some()
);
}
}
}
}
}
+1
View File
@@ -87,6 +87,7 @@ pub mod monitor;
pub mod notifications;
pub mod overlay;
pub mod people;
pub mod plan_review;
pub mod profiles;
pub mod prompt_injection;
pub mod provider_surfaces;
+261
View File
@@ -0,0 +1,261 @@
//! `PlanReviewGate` — parks a live interactive turn on a plan the user must
//! review before execution.
//!
//! Flow (mirrors [`crate::openhuman::approval::ApprovalGate`], in-memory):
//! 1. The orchestrator calls the `request_plan_review` tool after laying out a
//! thread-scoped plan. The tool calls [`PlanReviewGate::request_review`].
//! 2. The gate registers a `oneshot::Sender` keyed by `request_id`, publishes
//! [`DomainEvent::PlanReviewRequested`] (bridged to the `plan_review_request`
//! socket event), and parks the turn on the receiver.
//! 3. The UI's `PlanReviewCard` calls `plan_review_decide` (RPC) →
//! [`PlanReviewGate::decide`] → sends the resolution on the oneshot.
//! 4. The parked turn wakes with [`PlanReviewResolution`] and the tool returns
//! a result that tells the agent to proceed / stop / revise.
//!
//! On TTL or a dropped sender the gate resolves to `Reject` — fail-closed, so a
//! plan never executes without an explicit approval.
use std::collections::HashMap;
use std::sync::OnceLock;
use std::time::Duration;
use parking_lot::Mutex;
use tokio::sync::oneshot;
use uuid::Uuid;
use crate::core::event_bus::{publish_global, DomainEvent};
use super::types::PlanReviewResolution;
/// How long the gate parks a turn before timing out → `Reject`. Matches the
/// default approval TTL (10 min) — long enough for a human to read the plan.
const DEFAULT_PLAN_REVIEW_TTL: Duration = Duration::from_secs(60 * 10);
/// In-memory registry of parked plan reviews. Process-global singleton (see
/// [`global`]); no persistence — a parked interactive turn cannot resume
/// across a restart, so an orphaned review has nothing to recover.
pub struct PlanReviewGate {
ttl: Duration,
waiters: Mutex<HashMap<String, oneshot::Sender<PlanReviewResolution>>>,
/// Newest parked `request_id` per thread, so a typed reply or a UI action
/// that only knows the thread can resolve the latest review.
thread_to_request: Mutex<HashMap<String, String>>,
}
impl PlanReviewGate {
fn new(ttl: Duration) -> Self {
Self {
ttl,
waiters: Mutex::new(HashMap::new()),
thread_to_request: Mutex::new(HashMap::new()),
}
}
/// Park the current turn on a plan review and block until the user decides
/// or the TTL elapses. `summary` is a one-line description; `steps` are the
/// ordered plan items shown in the review card. `thread_id` / `client_id`
/// route the surface to the originating chat (absent → no routable surface,
/// so the park TTL-rejects).
pub async fn request_review(
&self,
thread_id: Option<String>,
client_id: Option<String>,
summary: String,
steps: Vec<String>,
) -> PlanReviewResolution {
let request_id = format!("plan-{}", Uuid::new_v4());
let (tx, rx) = oneshot::channel();
self.waiters.lock().insert(request_id.clone(), tx);
if let Some(tid) = thread_id.clone() {
self.thread_to_request
.lock()
.insert(tid, request_id.clone());
}
// RAII cleanup: remove the waiter + thread mapping on ANY exit path,
// including when the parked future is cancelled/dropped before
// `rx.await` completes (turn cancel, supervisor shutdown). Without this,
// a cancelled review would leak a `waiters` / `thread_to_request` entry
// that could be re-decided against a dead turn.
let _guard = ParkGuard {
gate: self,
request_id: request_id.clone(),
thread_id: thread_id.clone(),
};
tracing::info!(
request_id = %request_id,
thread_id = ?thread_id,
steps = steps.len(),
"[plan_review::gate] parking turn for plan review"
);
publish_global(DomainEvent::PlanReviewRequested {
request_id: request_id.clone(),
thread_id: thread_id.clone(),
client_id,
summary,
steps,
});
let resolution = match tokio::time::timeout(self.ttl, rx).await {
Ok(Ok(resolution)) => resolution,
// Sender dropped (decided elsewhere / shutdown) or TTL elapsed →
// fail closed: never execute a plan the user didn't approve.
Ok(Err(_)) | Err(_) => {
tracing::warn!(
request_id = %request_id,
"[plan_review::gate] review unresolved (timeout/dropped) → reject"
);
PlanReviewResolution::Reject
}
};
// `_guard` drops here on the normal path too (cleanup is idempotent with
// `decide`, which already removed the waiter).
publish_global(DomainEvent::PlanReviewDecided {
request_id: request_id.clone(),
decision: resolution.as_str().to_string(),
});
tracing::info!(
request_id = %request_id,
decision = resolution.as_str(),
"[plan_review::gate] review resolved"
);
resolution
}
/// Resolve a parked review by `request_id`. Returns `true` when a waiter was
/// woken; `false` when the id is unknown (already decided / expired).
pub fn decide(&self, request_id: &str, resolution: PlanReviewResolution) -> bool {
let sender = self.waiters.lock().remove(request_id);
match sender {
Some(tx) => tx.send(resolution).is_ok(),
None => {
tracing::debug!(
request_id = %request_id,
"[plan_review::gate] decide for unknown/expired request"
);
false
}
}
}
/// Resolve the newest parked review on `thread_id` (typed-reply / thread-
/// scoped path). Returns `true` when a waiter was woken.
pub fn decide_by_thread(&self, thread_id: &str, resolution: PlanReviewResolution) -> bool {
let request_id = self.thread_to_request.lock().get(thread_id).cloned();
match request_id {
Some(id) => self.decide(&id, resolution),
None => false,
}
}
}
/// Removes a parked review's registry entries on drop — covers both the normal
/// return and cancellation of the parked future.
struct ParkGuard<'a> {
gate: &'a PlanReviewGate,
request_id: String,
thread_id: Option<String>,
}
impl Drop for ParkGuard<'_> {
fn drop(&mut self) {
self.gate.waiters.lock().remove(&self.request_id);
if let Some(tid) = &self.thread_id {
let mut map = self.gate.thread_to_request.lock();
if map.get(tid) == Some(&self.request_id) {
map.remove(tid);
}
}
}
}
/// Process-global plan-review gate.
pub fn global() -> &'static PlanReviewGate {
static GATE: OnceLock<PlanReviewGate> = OnceLock::new();
GATE.get_or_init(|| PlanReviewGate::new(DEFAULT_PLAN_REVIEW_TTL))
}
#[cfg(test)]
mod tests {
use super::*;
#[tokio::test]
async fn approve_resolves_parked_turn() {
let gate = PlanReviewGate::new(Duration::from_secs(5));
let gate = std::sync::Arc::new(gate);
let g2 = gate.clone();
let parked = tokio::spawn(async move {
g2.request_review(
Some("t1".into()),
Some("c1".into()),
"Ship it".into(),
vec!["step one".into()],
)
.await
});
// Let the park register the waiter.
tokio::time::sleep(Duration::from_millis(20)).await;
assert!(gate.decide_by_thread("t1", PlanReviewResolution::Approve));
assert_eq!(parked.await.unwrap(), PlanReviewResolution::Approve);
}
#[tokio::test]
async fn revise_carries_feedback_back() {
let gate = std::sync::Arc::new(PlanReviewGate::new(Duration::from_secs(5)));
let g2 = gate.clone();
let parked = tokio::spawn(async move {
g2.request_review(Some("t2".into()), None, "Plan".into(), vec![])
.await
});
tokio::time::sleep(Duration::from_millis(20)).await;
assert!(gate.decide_by_thread(
"t2",
PlanReviewResolution::Revise {
feedback: "add a test step".into(),
},
));
assert_eq!(
parked.await.unwrap(),
PlanReviewResolution::Revise {
feedback: "add a test step".into(),
}
);
}
#[tokio::test]
async fn timeout_fails_closed_to_reject() {
let gate = PlanReviewGate::new(Duration::from_millis(40));
let resolution = gate
.request_review(Some("t3".into()), None, "Plan".into(), vec![])
.await;
assert_eq!(resolution, PlanReviewResolution::Reject);
// The waiter is cleaned up after timeout.
assert!(!gate.decide_by_thread("t3", PlanReviewResolution::Approve));
}
#[tokio::test]
async fn decide_unknown_request_is_false() {
let gate = PlanReviewGate::new(Duration::from_secs(5));
assert!(!gate.decide("nope", PlanReviewResolution::Approve));
}
#[tokio::test]
async fn cancelled_park_cleans_up_waiter() {
// A parked review whose future is dropped (turn cancel) before it
// resolves must not leak its waiter / thread mapping.
let gate = std::sync::Arc::new(PlanReviewGate::new(Duration::from_secs(30)));
let g2 = gate.clone();
let handle = tokio::spawn(async move {
g2.request_review(Some("t-drop".into()), None, "Plan".into(), vec![])
.await
});
tokio::time::sleep(Duration::from_millis(20)).await;
handle.abort();
let _ = handle.await;
// The drop guard removed the entry, so there is nothing left to decide.
assert!(!gate.decide_by_thread("t-drop", PlanReviewResolution::Approve));
}
}
+24
View File
@@ -0,0 +1,24 @@
//! Interactive plan-review gate (Codex/Claude plan-mode contract).
//!
//! When an interactive (`WebChat`) turn proposes a thread-scoped plan, the
//! orchestrator calls the [`tool::RequestPlanReviewTool`] (`request_plan_review`)
//! which parks the live turn on [`gate::PlanReviewGate`] until the user decides.
//! The UI's plan-review card resolves it via the `openhuman.plan_review_decide`
//! RPC (see [`schemas`]). Approve resumes-and-executes, Reject resumes-and-stops,
//! Revise resumes-with-feedback so the agent re-plans and re-parks.
//!
//! This is the live-turn counterpart to the task-board approval lifecycle (which
//! the background dispatcher runs on the `user-tasks` / `task-sources` boards):
//! the dispatcher never sweeps conversation thread boards, so a chat plan must be
//! gated on the turn itself, not via a board status. Modelled on
//! [`crate::openhuman::approval`] but in-memory only.
pub mod gate;
pub mod schemas;
pub mod tool;
pub mod types;
pub use schemas::all_controller_schemas as all_plan_review_controller_schemas;
pub use schemas::all_registered_controllers as all_plan_review_registered_controllers;
pub use tool::RequestPlanReviewTool;
pub use types::PlanReviewResolution;
+133
View File
@@ -0,0 +1,133 @@
//! JSON-RPC surface for the plan-review gate: `openhuman.plan_review_decide`
//! resolves a parked interactive turn (approve / reject / revise-with-feedback).
use serde::de::DeserializeOwned;
use serde::Deserialize;
use serde_json::{Map, Value};
use crate::core::all::{ControllerFuture, RegisteredController};
use crate::core::{ControllerSchema, FieldSchema, TypeSchema};
use super::gate;
use super::types::PlanReviewResolution;
pub fn all_controller_schemas() -> Vec<ControllerSchema> {
vec![schemas("decide")]
}
pub fn all_registered_controllers() -> Vec<RegisteredController> {
vec![RegisteredController {
schema: schemas("decide"),
handler: handle_decide,
}]
}
pub fn schemas(function: &str) -> ControllerSchema {
match function {
"decide" => ControllerSchema {
namespace: "plan_review",
function: "decide",
description: "Resolve a parked plan review: approve (the turn resumes and executes), \
reject (the turn resumes and stops), or revise (the turn resumes, \
re-plans from `feedback`, and re-parks).",
inputs: vec![
FieldSchema {
name: "request_id",
ty: TypeSchema::String,
comment: "Plan-review request id from the `plan_review_request` event.",
required: true,
},
FieldSchema {
name: "decision",
ty: TypeSchema::String,
comment: "One of `approve` | `reject` | `revise`.",
required: true,
},
FieldSchema {
name: "feedback",
ty: TypeSchema::Option(Box::new(TypeSchema::String)),
comment: "Free-text revision request — required when `decision` is `revise`.",
required: false,
},
],
outputs: vec![FieldSchema {
name: "resolved",
ty: TypeSchema::Bool,
comment:
"true if a parked turn was woken; false if the request was unknown/expired.",
required: true,
}],
},
_ => ControllerSchema {
namespace: "plan_review",
function: "unknown",
description: "Unknown plan_review controller function.",
inputs: vec![],
outputs: vec![FieldSchema {
name: "error",
ty: TypeSchema::String,
comment: "Lookup error details.",
required: true,
}],
},
}
}
#[derive(Debug, Deserialize)]
struct DecideParams {
request_id: String,
decision: String,
#[serde(default)]
feedback: Option<String>,
}
fn handle_decide(params: Map<String, Value>) -> ControllerFuture {
Box::pin(async move {
let p = parse::<DecideParams>(params)?;
let resolution = match p.decision.trim().to_ascii_lowercase().as_str() {
"approve" => PlanReviewResolution::Approve,
"reject" => PlanReviewResolution::Reject,
"revise" => {
let feedback = p.feedback.unwrap_or_default();
if feedback.trim().is_empty() {
return Err("revise requires non-empty `feedback`".to_string());
}
PlanReviewResolution::Revise { feedback }
}
other => {
return Err(format!(
"invalid decision '{other}' (expected approve|reject|revise)"
))
}
};
tracing::debug!(
request_id = %p.request_id,
decision = resolution.as_str(),
"[rpc][plan_review] decide entry"
);
let resolved = gate::global().decide(&p.request_id, resolution);
Ok(serde_json::json!({ "resolved": resolved }))
})
}
fn parse<T: DeserializeOwned>(params: Map<String, Value>) -> Result<T, String> {
serde_json::from_value(Value::Object(params)).map_err(|e| format!("invalid params: {e}"))
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn controller_lists_match_lengths() {
assert_eq!(
all_controller_schemas().len(),
all_registered_controllers().len()
);
}
#[test]
fn schema_uses_plan_review_namespace() {
assert_eq!(schemas("decide").namespace, "plan_review");
}
}
+198
View File
@@ -0,0 +1,198 @@
//! `request_plan_review` — the agent tool that parks the current interactive
//! turn on a plan the user must review before execution.
//!
//! The orchestrator calls this AFTER laying out a thread-scoped plan and BEFORE
//! executing it. On an interactive (`WebChat`) turn the call blocks on
//! [`PlanReviewGate`] until the user decides; the tool result then tells the
//! agent to proceed / stop / revise. On any non-interactive origin (cron,
//! subconscious, CLI, channels) there is no human to ask, so the tool
//! auto-approves immediately — background automation is never blocked.
use async_trait::async_trait;
use serde_json::json;
use crate::openhuman::agent::turn_origin::{self, AgentTurnOrigin};
use crate::openhuman::approval::APPROVAL_CHAT_CONTEXT;
use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult, ToolTimeout};
use super::gate;
use super::types::PlanReviewResolution;
pub struct RequestPlanReviewTool;
impl RequestPlanReviewTool {
pub fn new() -> Self {
Self
}
}
impl Default for RequestPlanReviewTool {
fn default() -> Self {
Self::new()
}
}
#[async_trait]
impl Tool for RequestPlanReviewTool {
fn name(&self) -> &str {
"request_plan_review"
}
fn description(&self) -> &str {
"Pause an interactive turn so the user can review a thread-scoped plan \
BEFORE you execute it (the Codex/Claude plan-mode contract). Call this \
once you have laid out a multi-step plan (e.g. via the `todo` tool) and \
BEFORE doing any of the work. The call BLOCKS until the user decides; \
the result tells you what to do next: \
`approved` proceed and execute the plan now; \
`rejected` do NOT execute, ask the user what they want instead; \
`revise` the result carries the user's feedback revise the plan \
and call `request_plan_review` again before executing. \
On non-interactive turns (cron / subconscious / CLI) this returns \
`approved` immediately so automation is never blocked. \
Pass `summary` (one line) and `steps` (the ordered plan items)."
}
fn parameters_schema(&self) -> serde_json::Value {
json!({
"type": "object",
"properties": {
"summary": {
"type": "string",
"description": "One-line description of the plan being reviewed."
},
"steps": {
"type": "array",
"description": "Ordered plan steps shown to the user for review.",
"items": { "type": "string" }
}
},
"required": ["summary"]
})
}
fn permission_level(&self) -> PermissionLevel {
// The gate IS the user consent surface — don't double-gate it through
// the ApprovalGate as well.
PermissionLevel::None
}
fn external_effect(&self) -> bool {
false
}
fn timeout_policy(&self, _args: &serde_json::Value) -> ToolTimeout {
// This tool BLOCKS while the user reviews the plan — the global tool
// timeout (default ~120s) would otherwise drop the parked future before
// the gate's own 10-minute TTL, so approving the visible card could not
// resume the turn. The gate is the real deadline (fail-closed reject on
// TTL), so the harness must not impose its own.
ToolTimeout::Unbounded
}
async fn execute(&self, args: serde_json::Value) -> anyhow::Result<ToolResult> {
let summary = args
.get("summary")
.and_then(|v| v.as_str())
.unwrap_or("")
.trim()
.to_string();
let steps: Vec<String> = args
.get("steps")
.and_then(|v| v.as_array())
.map(|arr| {
arr.iter()
.filter_map(|s| s.as_str())
.map(|s| s.trim().to_string())
.filter(|s| !s.is_empty())
.collect()
})
.unwrap_or_default();
// Only interactive (WebChat) turns have a human to review the plan.
// Anything else auto-approves so background automation isn't wedged.
let origin = turn_origin::current().unwrap_or(AgentTurnOrigin::Unknown);
if !matches!(origin, AgentTurnOrigin::WebChat { .. }) {
tracing::debug!(
origin = ?origin,
"[tool][request_plan_review] non-interactive turn — auto-approving"
);
return Ok(ToolResult::success(
"approved: non-interactive turn (no review surface) — proceed with the plan."
.to_string(),
));
}
// Route the surface back to the originating chat thread/client (set by
// the web channel around the turn, same task-local the ApprovalGate uses).
let chat_ctx = APPROVAL_CHAT_CONTEXT.try_with(|c| c.clone()).ok();
let thread_id = chat_ctx.as_ref().map(|c| c.thread_id.clone());
let client_id = chat_ctx.as_ref().map(|c| c.client_id.clone());
tracing::info!(
thread_id = ?thread_id,
steps = steps.len(),
"[tool][request_plan_review] parking interactive turn for plan review"
);
let resolution = gate::global()
.request_review(thread_id, client_id, summary, steps)
.await;
let result = match resolution {
PlanReviewResolution::Approve => ToolResult::success(
"approved: the user approved the plan — proceed and execute it now.".to_string(),
),
PlanReviewResolution::Reject => ToolResult::success(
"rejected: the user rejected the plan — do NOT execute it. Briefly ask what \
they would like to do instead."
.to_string(),
),
PlanReviewResolution::Revise { feedback } => ToolResult::success(format!(
"revise: the user requested changes before executing. Their feedback:\n{feedback}\n\
Revise the plan accordingly, then call `request_plan_review` again before \
executing."
)),
};
Ok(result)
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::openhuman::agent::turn_origin::with_origin;
#[tokio::test]
async fn non_interactive_origin_auto_approves() {
let tool = RequestPlanReviewTool::new();
let out = with_origin(
AgentTurnOrigin::Cli,
tool.execute(json!({ "summary": "do x", "steps": ["a", "b"] })),
)
.await
.unwrap();
assert!(!out.is_error);
assert!(out.output().starts_with("approved"));
}
#[tokio::test]
async fn interactive_turn_parks_until_resolved() {
let tool = RequestPlanReviewTool::new();
let fut = with_origin(
AgentTurnOrigin::WebChat {
thread_id: "t-int".into(),
client_id: "c-int".into(),
},
tool.execute(json!({ "summary": "plan", "steps": ["one"] })),
);
// An interactive turn must BLOCK on the gate rather than return
// immediately — a short timeout elapses with no result (the parked
// future is then dropped, and the gate cleans up).
let res = tokio::time::timeout(std::time::Duration::from_millis(60), fut).await;
assert!(
res.is_err(),
"interactive turn should park, not resolve immediately"
);
}
}
+37
View File
@@ -0,0 +1,37 @@
//! Types for the interactive plan-review gate.
//!
//! A plan review parks a live (interactive) agent turn after the orchestrator
//! has laid out a thread-scoped plan, surfaces the plan to the user, and
//! resumes the SAME turn with the user's decision. Unlike the task-board
//! approval lifecycle (which the background dispatcher runs on the
//! `user-tasks` / `task-sources` boards), this gate is a parked-future on the
//! live turn — modelled on [`crate::openhuman::approval::ApprovalGate`] but
//! in-memory only: an interactive turn that can't resume across a restart has
//! nothing to persist.
use serde::{Deserialize, Serialize};
/// The user's decision on a parked plan, sent back to the parked turn.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(tag = "decision", rename_all = "snake_case")]
pub enum PlanReviewResolution {
/// Run the plan as proposed — the parked turn resumes and executes.
Approve,
/// Drop the plan — the parked turn resumes and stops without executing.
Reject,
/// Revise the plan per `feedback` — the parked turn resumes, re-plans, and
/// re-parks for another review.
Revise { feedback: String },
}
impl PlanReviewResolution {
/// Stable label for events/logs (the `Revise` payload is omitted here —
/// feedback is user content and must not land in redacted event fields).
pub fn as_str(&self) -> &'static str {
match self {
Self::Approve => "approve",
Self::Reject => "reject",
Self::Revise { .. } => "revise",
}
}
}
+72
View File
@@ -434,6 +434,35 @@ pub fn decide_plan(
update_status(location, id, new_status)
}
/// Clear a parked plan for re-planning. Transitions **every**
/// `AwaitingApproval` card on the board to `Rejected` so none stays runnable,
/// then returns the fresh snapshot. The caller (the plan-review surface) sends
/// the user's `feedback` back into the thread as a normal message so the
/// orchestrator re-plans and re-parks a new plan. Lenient when nothing is
/// awaiting — a benign no-op (returns the snapshot unchanged) rather than an
/// error, so a racing decision can't strand the feedback message.
pub fn revise_plan(location: &BoardLocation, feedback: &str) -> Result<TodosSnapshot, String> {
let _scratch_guard = maybe_scratch_lock(location);
let mut cards = load_cards(location)?;
let mut revised = 0usize;
for card in cards.iter_mut() {
if card.status == TaskCardStatus::AwaitingApproval {
card.status = TaskCardStatus::Rejected;
card.updated_at = Utc::now().to_rfc3339();
revised += 1;
}
}
tracing::info!(
thread_id = ?location.thread_id(),
revised,
feedback_len = feedback.len(),
"[todos][ops] revise_plan rejected awaiting cards for re-plan"
);
let cards = save_cards(location, cards)?;
emit_progress(location, &cards);
Ok(into_snapshot(location, cards))
}
/// Remove a card by id. Errors if `id` is unknown.
pub fn remove(location: &BoardLocation, id: &str) -> Result<TodosSnapshot, String> {
tracing::debug!(
@@ -788,6 +817,49 @@ mod tests {
assert_eq!(rejected.cards[0].status, TaskCardStatus::Rejected);
}
#[test]
fn revise_plan_rejects_only_awaiting_cards() {
let dir = tempdir().unwrap();
let loc = thread_loc(dir.path(), "t1");
// Each `add` returns the whole board; the new card is the last one.
let a = add(&loc, "A", CardPatch::default()).unwrap();
let b = add(&loc, "B", CardPatch::default()).unwrap();
let c = add(&loc, "C", CardPatch::default()).unwrap();
let a_id = a.cards.last().unwrap().id.clone();
let b_id = b.cards.last().unwrap().id.clone();
let c_id = c.cards.last().unwrap().id.clone();
// Two cards parked for review, one left as a plain todo.
update_status(&loc, &a_id, TaskCardStatus::AwaitingApproval).unwrap();
update_status(&loc, &b_id, TaskCardStatus::AwaitingApproval).unwrap();
let snap = revise_plan(&loc, "please add a verification step").unwrap();
let by_id = |id: &str| {
snap.cards
.iter()
.find(|c| c.id == id)
.unwrap()
.status
.clone()
};
assert_eq!(by_id(&a_id), TaskCardStatus::Rejected);
assert_eq!(by_id(&b_id), TaskCardStatus::Rejected);
// The non-awaiting card is untouched.
assert_eq!(by_id(&c_id), TaskCardStatus::Todo);
}
#[test]
fn revise_plan_is_noop_when_nothing_awaiting() {
let dir = tempdir().unwrap();
let loc = thread_loc(dir.path(), "t1");
let a = add(&loc, "A", CardPatch::default()).unwrap();
let a_id = a.cards[0].id.clone();
let snap = revise_plan(&loc, "tweak it").unwrap();
assert_eq!(snap.cards.len(), 1);
assert_eq!(snap.cards[0].id, a_id);
assert_eq!(snap.cards[0].status, TaskCardStatus::Todo);
}
#[test]
fn edit_can_clear_approval_mode() {
let dir = tempdir().unwrap();
+41
View File
@@ -22,6 +22,7 @@ pub fn all_controller_schemas() -> Vec<ControllerSchema> {
schemas("update_status"),
schemas("set_session_thread"),
schemas("decide_plan"),
schemas("revise_plan"),
schemas("remove"),
schemas("replace"),
schemas("clear"),
@@ -57,6 +58,10 @@ pub fn all_registered_controllers() -> Vec<RegisteredController> {
schema: schemas("decide_plan"),
handler: handle_decide_plan,
},
RegisteredController {
schema: schemas("revise_plan"),
handler: handle_revise_plan,
},
RegisteredController {
schema: schemas("remove"),
handler: handle_remove,
@@ -209,6 +214,22 @@ pub fn schemas(function: &str) -> ControllerSchema {
],
outputs: vec![snapshot_output()],
},
"revise_plan" => ControllerSchema {
namespace: "todos",
function: "revise_plan",
description: "Reject every card awaiting plan approval so the orchestrator can \
re-plan from the user's feedback (the feedback is sent back into the \
thread as a message by the caller). Idempotent no-op when nothing is \
awaiting.",
inputs: vec![
thread_id_input(),
optional_string(
"feedback",
"User's free-text revision request (recorded for logging/audit).",
),
],
outputs: vec![snapshot_output()],
},
"remove" => ControllerSchema {
namespace: "todos",
function: "remove",
@@ -409,6 +430,13 @@ struct DecidePlanParams {
approve: bool,
}
#[derive(Debug, Deserialize)]
struct RevisePlanParams {
thread_id: String,
#[serde(default)]
feedback: String,
}
#[derive(Debug, Deserialize)]
struct ReplaceParams {
thread_id: String,
@@ -516,6 +544,19 @@ fn handle_decide_plan(params: Map<String, Value>) -> ControllerFuture {
})
}
fn handle_revise_plan(params: Map<String, Value>) -> ControllerFuture {
Box::pin(async move {
let p = parse::<RevisePlanParams>(params)?;
let loc = thread_location(&p.thread_id).await?;
tracing::debug!(
thread_id = %p.thread_id,
feedback_len = p.feedback.len(),
"[rpc][todos] revise_plan entry"
);
snapshot_to_json(ops::revise_plan(&loc, &p.feedback)?)
})
}
fn parse_approval_mode(raw: Option<String>) -> Result<Option<TaskApprovalMode>, String> {
let Some(raw) = raw else {
return Ok(None);
+3
View File
@@ -187,6 +187,9 @@ pub fn all_tools_with_runtime(
// build-mode pass. The plan→build mode switch itself is a
// follow-up; the tool emits a stable marker today.
Box::new(TodoTool::new()),
// Interactive plan-review gate: parks the live turn on a thread-scoped
// plan the user must approve before execution (Codex/Claude plan mode).
Box::new(crate::openhuman::plan_review::RequestPlanReviewTool::new()),
// Move/update a specific task card by id on a target board (defaults to
// the proactive `task-sources` board) — lets the agent advance the task
// it's working (in_progress / done+evidence / blocked+reason) from any
+134
View File
@@ -2519,6 +2519,140 @@ async fn json_rpc_todos_crud_on_personal_board() {
rpc_join.abort();
}
#[tokio::test]
async fn json_rpc_todos_revise_plan_rejects_awaiting() {
// Plan-mode "Send feedback" path: a parked plan (card awaiting approval) is
// cleared by `todos_revise_plan` so the orchestrator can re-plan from the
// user's feedback. Awaiting cards become `rejected`; non-awaiting cards are
// left untouched.
let _env_lock = json_rpc_e2e_env_lock();
let tmp = tempdir().expect("tempdir");
let home = tmp.path();
let openhuman_home = home.join(".openhuman");
let _home_guard = EnvVarGuard::set_to_path("HOME", home);
let _workspace_guard = EnvVarGuard::unset("OPENHUMAN_WORKSPACE");
let _backend_url_guard = EnvVarGuard::unset("BACKEND_URL");
let _vite_backend_url_guard = EnvVarGuard::unset("VITE_BACKEND_URL");
let _api_url_guard = EnvVarGuard::unset("OPENHUMAN_API_URL");
let (api_addr, api_join) = serve_on_ephemeral(mock_upstream_router()).await;
let api_origin = format!("http://{api_addr}");
write_min_config(openhuman_home.as_path(), &api_origin);
let (rpc_addr, rpc_join) = serve_on_ephemeral(build_core_http_router(false)).await;
let rpc_base = format!("http://{rpc_addr}");
let board = "plan-review-thread";
// 1. Add a card and park it for review (awaiting_approval).
let add = post_json_rpc(
&rpc_base,
9201,
"openhuman.todos_add",
json!({ "thread_id": board, "content": "Refactor schema", "status": "awaiting_approval" }),
)
.await;
let add_result = assert_no_jsonrpc_error(&add, "todos_add awaiting");
let card_id = add_result
.get("cards")
.and_then(Value::as_array)
.and_then(|c| c.first())
.and_then(|c| c.get("id"))
.and_then(Value::as_str)
.expect("parked card id")
.to_string();
// 2. Send feedback → revise_plan rejects the parked card.
let revise = post_json_rpc(
&rpc_base,
9202,
"openhuman.todos_revise_plan",
json!({ "thread_id": board, "feedback": "split into smaller steps" }),
)
.await;
let revise_result = assert_no_jsonrpc_error(&revise, "todos_revise_plan");
let revised_cards = revise_result
.get("cards")
.and_then(Value::as_array)
.expect("cards in revise response");
assert_eq!(revised_cards.len(), 1, "card retained, status changed");
assert_eq!(
revised_cards[0].get("id").and_then(Value::as_str),
Some(card_id.as_str()),
"same card id"
);
assert_eq!(
revised_cards[0].get("status").and_then(Value::as_str),
Some("rejected"),
"parked card is rejected after revise_plan"
);
api_join.abort();
rpc_join.abort();
}
#[tokio::test]
async fn json_rpc_plan_review_decide_unknown_and_invalid() {
// The plan-review gate is in-memory and parks a live turn; over RPC we can
// still exercise the decision surface: deciding an unknown/expired request
// resolves nothing (`resolved: false`), and an invalid decision errors.
let _env_lock = json_rpc_e2e_env_lock();
let tmp = tempdir().expect("tempdir");
let home = tmp.path();
let openhuman_home = home.join(".openhuman");
let _home_guard = EnvVarGuard::set_to_path("HOME", home);
let _workspace_guard = EnvVarGuard::unset("OPENHUMAN_WORKSPACE");
let _backend_url_guard = EnvVarGuard::unset("BACKEND_URL");
let _vite_backend_url_guard = EnvVarGuard::unset("VITE_BACKEND_URL");
let _api_url_guard = EnvVarGuard::unset("OPENHUMAN_API_URL");
let (api_addr, api_join) = serve_on_ephemeral(mock_upstream_router()).await;
let api_origin = format!("http://{api_addr}");
write_min_config(openhuman_home.as_path(), &api_origin);
let (rpc_addr, rpc_join) = serve_on_ephemeral(build_core_http_router(false)).await;
let rpc_base = format!("http://{rpc_addr}");
// Unknown request id → resolved:false (no parked turn to wake).
let unknown = post_json_rpc(
&rpc_base,
9301,
"openhuman.plan_review_decide",
json!({ "request_id": "plan-does-not-exist", "decision": "approve" }),
)
.await;
let unknown_result = assert_no_jsonrpc_error(&unknown, "plan_review_decide unknown");
assert_eq!(
unknown_result.get("resolved").and_then(Value::as_bool),
Some(false),
"deciding an unknown request resolves nothing"
);
// revise without feedback → error.
let bad_revise = post_json_rpc(
&rpc_base,
9302,
"openhuman.plan_review_decide",
json!({ "request_id": "plan-x", "decision": "revise" }),
)
.await;
assert_jsonrpc_error(&bad_revise, "plan_review_decide revise w/o feedback");
// Unknown decision verb → error.
let bad_decision = post_json_rpc(
&rpc_base,
9303,
"openhuman.plan_review_decide",
json!({ "request_id": "plan-x", "decision": "maybe" }),
)
.await;
assert_jsonrpc_error(&bad_decision, "plan_review_decide invalid decision");
api_join.abort();
rpc_join.abort();
}
#[tokio::test]
async fn json_rpc_thread_goal_lifecycle() {
let _env_lock = json_rpc_e2e_env_lock();