mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-30 23:14:37 +00:00
feat(orchestration): actionable Allow/Deny for runtime tool approvals (#4837)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: M3gA-Mind <elvin@tinyhumans.ai>
This commit is contained in:
co-authored by
Claude Opus 4.8
M3gA-Mind
parent
aa910d2422
commit
1cde3bdaf0
@@ -241,6 +241,37 @@ function SessionChatView({ session }: { session: SessionSummary }) {
|
||||
[body, sending, session.agentId, session.sessionId, refresh]
|
||||
);
|
||||
|
||||
// A runtime tool-approval decision → reply "allow"/"deny" to the peer. Rethrows
|
||||
// on failure so SessionTranscript rolls the card back to buttons for a retry.
|
||||
const decide = useCallback(
|
||||
async (decision: 'allow' | 'deny'): Promise<void> => {
|
||||
setSendError(null);
|
||||
debug(
|
||||
'[orchestration:agent-chat] approval decision: send session=%s decision=%s',
|
||||
session.sessionId,
|
||||
decision
|
||||
);
|
||||
try {
|
||||
await orchestrationClient.sendMasterMessage({
|
||||
body: decision,
|
||||
recipient: session.agentId,
|
||||
sessionId: session.sessionId,
|
||||
});
|
||||
if (mountedRef.current) void refresh();
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
debug(
|
||||
'[orchestration:agent-chat] approval decision: failed session=%s %s',
|
||||
session.sessionId,
|
||||
message
|
||||
);
|
||||
if (mountedRef.current) setSendError(message);
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
[session.agentId, session.sessionId, refresh]
|
||||
);
|
||||
|
||||
const runtime = session.harnessType || session.source || null;
|
||||
const directory = session.workspace?.trim() || null;
|
||||
const runningOn = session.agentId?.trim() || null;
|
||||
@@ -323,7 +354,10 @@ function SessionChatView({ session }: { session: SessionSummary }) {
|
||||
{t('tinyplaceOrchestration.noMessages')}
|
||||
</p>
|
||||
) : (
|
||||
<SessionTranscript messages={messages} />
|
||||
<SessionTranscript
|
||||
messages={messages}
|
||||
onDecide={(_message, decision) => decide(decision === 'deny' ? 'deny' : 'allow')}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
</ChatPageScaffold>
|
||||
|
||||
@@ -127,6 +127,36 @@ function SessionView({
|
||||
[body, sending, contactAddr, session.sessionId, refresh]
|
||||
);
|
||||
|
||||
// Runtime tool-approval decision → reply "allow"/"deny" to the runtime peer (the
|
||||
// same send path as a typed reply). Returns the promise (rethrows on failure) so
|
||||
// SessionTranscript rolls the card back to buttons for a retry.
|
||||
const decide = useCallback(
|
||||
(decision: 'allow' | 'deny'): Promise<void> => {
|
||||
setSendError(null);
|
||||
debug(
|
||||
'[orchestration:connections] approval decision: send session=%s decision=%s',
|
||||
session.sessionId,
|
||||
decision
|
||||
);
|
||||
return orchestrationClient
|
||||
.sendMasterMessage({ body: decision, recipient: contactAddr, sessionId: session.sessionId })
|
||||
.then(() => {
|
||||
void refresh();
|
||||
})
|
||||
.catch((error: unknown) => {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
debug(
|
||||
'[orchestration:connections] approval decision: failed session=%s %s',
|
||||
session.sessionId,
|
||||
message
|
||||
);
|
||||
setSendError(message);
|
||||
throw error;
|
||||
});
|
||||
},
|
||||
[contactAddr, session.sessionId, refresh]
|
||||
);
|
||||
|
||||
return (
|
||||
<div className="space-y-3" data-testid="orch-session-view">
|
||||
<button
|
||||
@@ -163,7 +193,10 @@ function SessionView({
|
||||
{t('tinyplaceOrchestration.noMessages')}
|
||||
</p>
|
||||
) : (
|
||||
<SessionTranscript messages={messages} />
|
||||
<SessionTranscript
|
||||
messages={messages}
|
||||
onDecide={(_message, decision) => decide(decision === 'deny' ? 'deny' : 'allow')}
|
||||
/>
|
||||
)}
|
||||
{sendError ? (
|
||||
<p
|
||||
|
||||
@@ -9,7 +9,7 @@
|
||||
* Approvals are actionable only on your OWN agent (master/subconscious) — pass
|
||||
* `onDecide`. In a peer session omit it and the approval renders read-only.
|
||||
*/
|
||||
import type { ReactElement } from 'react';
|
||||
import { type ReactElement, useState } from 'react';
|
||||
|
||||
import { useT } from '../../lib/i18n/I18nContext';
|
||||
import { mergeToolActivity, type ToolActivity } from '../../lib/orchestration/mergeToolActivity';
|
||||
@@ -20,8 +20,17 @@ export type ApprovalDecision = 'approve' | 'deny' | 'always';
|
||||
|
||||
export interface SessionTranscriptProps {
|
||||
messages: ChatMessage[];
|
||||
/** When present, approval rows show actionable buttons wired to this. */
|
||||
onDecide?: (message: ChatMessage, decision: ApprovalDecision) => void;
|
||||
/**
|
||||
* When present, approval rows show actionable Approve/Deny buttons wired to
|
||||
* this. May return a promise; the card rolls back to buttons if it rejects.
|
||||
*/
|
||||
onDecide?: (message: ChatMessage, decision: ApprovalDecision) => void | Promise<unknown>;
|
||||
/**
|
||||
* Show the "Always allow" action. Off by default — only enable it where a
|
||||
* persistent always-allow is actually honored. A runtime that understands only
|
||||
* one-shot allow/deny must NOT offer it (it would silently be a one-time allow).
|
||||
*/
|
||||
alwaysAllow?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -33,6 +42,52 @@ function isOwnerAuthored(from: string): boolean {
|
||||
return from === 'you' || from === 'owner' || from === 'user';
|
||||
}
|
||||
|
||||
/**
|
||||
* An approval decision (Approve/Deny) is sent to the runtime as a one-word reply
|
||||
* ("allow"/"deny"), which mirrors back as an owner bubble. The approval card
|
||||
* already resolves in place to show the outcome, so that echo bubble is pure
|
||||
* noise — suppress a standalone owner-authored decision word.
|
||||
*/
|
||||
const DECISION_ECHO_RE = /^(allow|deny|approve|skip|always(\s+allow)?)$/i;
|
||||
function isDecisionEcho(message: ChatMessage): boolean {
|
||||
return (
|
||||
!message.eventKind &&
|
||||
isOwnerAuthored(message.from) &&
|
||||
DECISION_ECHO_RE.test(message.body.trim())
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Derive each approval's outcome from the persisted message stream so a resolved
|
||||
* card survives a reload/remount (local click state alone is lost). Approvals and
|
||||
* their decision echoes alternate, so pair them FIFO in order of appearance.
|
||||
*
|
||||
* Returns the per-approval outcome AND the ids of the echo messages that were
|
||||
* actually paired to an approval — only THOSE are suppressed from the render. A
|
||||
* one-word "allow"/"deny" with no pending approval is a real chat reply and must
|
||||
* stay visible.
|
||||
*/
|
||||
function deriveDecisions(messages: ChatMessage[]): {
|
||||
decided: Record<string, ApprovalDecision>;
|
||||
suppressed: Set<string>;
|
||||
} {
|
||||
const decided: Record<string, ApprovalDecision> = {};
|
||||
const suppressed = new Set<string>();
|
||||
const pending: string[] = [];
|
||||
for (const m of messages) {
|
||||
if (m.eventKind === 'approval_request') {
|
||||
pending.push(m.id);
|
||||
} else if (isDecisionEcho(m)) {
|
||||
const approvalId = pending.shift();
|
||||
if (approvalId) {
|
||||
decided[approvalId] = /^(deny|skip)/i.test(m.body.trim()) ? 'deny' : 'approve';
|
||||
suppressed.add(m.id);
|
||||
}
|
||||
}
|
||||
}
|
||||
return { decided, suppressed };
|
||||
}
|
||||
|
||||
/** Lightweight `**bold**` rendering without pulling in a markdown lib. */
|
||||
function renderInline(text: string): (string | ReactElement)[] {
|
||||
return text.split(/(\*\*[^*]+\*\*)/g).map((part, i) =>
|
||||
@@ -137,17 +192,32 @@ function ToolBlock({ tool }: { tool: ToolActivity }): ReactElement {
|
||||
function ApprovalRow({
|
||||
message,
|
||||
onDecide,
|
||||
decided,
|
||||
allowAlways,
|
||||
}: {
|
||||
message: ChatMessage;
|
||||
onDecide?: (message: ChatMessage, decision: ApprovalDecision) => void;
|
||||
/** Once decided, the card resolves in place: buttons are replaced by the outcome. */
|
||||
decided?: ApprovalDecision;
|
||||
/** Whether to render the "Always allow" action (only where it's honored). */
|
||||
allowAlways?: boolean;
|
||||
}): ReactElement {
|
||||
const { t } = useT();
|
||||
const denied = decided === 'deny';
|
||||
return (
|
||||
<div className="flex justify-start" data-event-kind="approval_request">
|
||||
<div className="w-full max-w-[85%] rounded-xl border border-amber-300 bg-amber-50 px-3 py-2.5 dark:border-amber-500/40 dark:bg-amber-500/10">
|
||||
<div
|
||||
className={`w-full max-w-[85%] rounded-xl border px-3 py-2.5 ${
|
||||
decided
|
||||
? 'border-line bg-surface/60 dark:bg-black/20'
|
||||
: 'border-amber-300 bg-amber-50 dark:border-amber-500/40 dark:bg-amber-500/10'
|
||||
}`}>
|
||||
<div className="flex items-center gap-2">
|
||||
<span className="text-sm text-amber-500">⚠</span>
|
||||
<span className="text-xs font-semibold text-amber-700 dark:text-amber-300">
|
||||
<span className={`text-sm ${decided ? 'text-content-faint' : 'text-amber-500'}`}>⚠</span>
|
||||
<span
|
||||
className={`text-xs font-semibold ${
|
||||
decided ? 'text-content-faint' : 'text-amber-700 dark:text-amber-300'
|
||||
}`}>
|
||||
{t('chat.approval.title')}
|
||||
</span>
|
||||
{message.toolName ? (
|
||||
@@ -159,7 +229,17 @@ function ApprovalRow({
|
||||
<code className="mt-1.5 block overflow-x-auto whitespace-pre-wrap break-words font-mono text-[11px] text-content-secondary">
|
||||
{message.body}
|
||||
</code>
|
||||
{onDecide ? (
|
||||
{decided ? (
|
||||
// Resolved in place — no more actions, just the outcome.
|
||||
<div
|
||||
className={`mt-2.5 flex items-center gap-1.5 text-xs font-semibold ${
|
||||
denied ? 'text-danger' : 'text-sage-600 dark:text-sage-400'
|
||||
}`}
|
||||
data-testid="approval-resolved">
|
||||
<span>{denied ? '✕' : '✓'}</span>
|
||||
<span>{denied ? t('chat.approval.deny') : t('chat.approval.approve')}</span>
|
||||
</div>
|
||||
) : onDecide ? (
|
||||
<div className="mt-2.5 flex gap-2">
|
||||
<button
|
||||
type="button"
|
||||
@@ -173,12 +253,14 @@ function ApprovalRow({
|
||||
className="rounded-lg border border-line bg-surface px-3 py-1 text-xs font-medium text-content-secondary transition hover:bg-surface-hover">
|
||||
{t('chat.approval.deny')}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => onDecide(message, 'always')}
|
||||
className="rounded-lg px-2 py-1 text-xs font-medium text-content-faint transition hover:text-content-secondary">
|
||||
{t('chat.approval.alwaysAllow')}
|
||||
</button>
|
||||
{allowAlways ? (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => onDecide(message, 'always')}
|
||||
className="rounded-lg px-2 py-1 text-xs font-medium text-content-faint transition hover:text-content-secondary">
|
||||
{t('chat.approval.alwaysAllow')}
|
||||
</button>
|
||||
) : null}
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
@@ -189,8 +271,30 @@ function ApprovalRow({
|
||||
export default function SessionTranscript({
|
||||
messages,
|
||||
onDecide,
|
||||
alwaysAllow = false,
|
||||
}: SessionTranscriptProps): ReactElement {
|
||||
const rows = mergeToolActivity(messages);
|
||||
// Persistent outcomes derived from the message stream (survive reload); only the
|
||||
// decision echoes actually paired to an approval are suppressed from the render.
|
||||
const { decided: derivedDecided, suppressed } = deriveDecisions(messages);
|
||||
const rows = mergeToolActivity(messages.filter(m => !suppressed.has(m.id)));
|
||||
// Local clicks give immediate feedback before the echo message arrives; they
|
||||
// layer on top of the derived (persisted) outcomes. Rolled back if the send
|
||||
// fails, so the buttons return for a retry.
|
||||
const [clicked, setClicked] = useState<Record<string, ApprovalDecision>>({});
|
||||
const rollback = (id: string): void =>
|
||||
setClicked(prev => {
|
||||
const next = { ...prev };
|
||||
delete next[id];
|
||||
return next;
|
||||
});
|
||||
const handleDecide = onDecide
|
||||
? (message: ChatMessage, decision: ApprovalDecision): void => {
|
||||
setClicked(prev => ({ ...prev, [message.id]: decision }));
|
||||
Promise.resolve(onDecide(message, decision)).catch(() => rollback(message.id));
|
||||
}
|
||||
: undefined;
|
||||
const decidedFor = (id: string): ApprovalDecision | undefined =>
|
||||
clicked[id] ?? derivedDecided[id];
|
||||
return (
|
||||
<div className="space-y-3" data-testid="session-transcript">
|
||||
{rows.map((row, i) => {
|
||||
@@ -204,7 +308,15 @@ export default function SessionTranscript({
|
||||
case 'error':
|
||||
return <ErrorRow key={message.id} message={message} />;
|
||||
case 'approval_request':
|
||||
return <ApprovalRow key={message.id} message={message} onDecide={onDecide} />;
|
||||
return (
|
||||
<ApprovalRow
|
||||
key={message.id}
|
||||
message={message}
|
||||
onDecide={handleDecide}
|
||||
decided={decidedFor(message.id)}
|
||||
allowAlways={alwaysAllow}
|
||||
/>
|
||||
);
|
||||
default:
|
||||
// agent_message + legacy v1 rows → bubble by sender. Owner/user-
|
||||
// authored rows (incl. a reply mirrored back with role "owner") sit
|
||||
|
||||
@@ -84,6 +84,7 @@ describe('AgentChatPanel', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
contactSessions.current = [];
|
||||
transcript.current = { state: { status: 'ok' }, messages: [], refresh: vi.fn() };
|
||||
chatsApi.current = {
|
||||
...chatsApi.current,
|
||||
selectedId: 'master',
|
||||
@@ -145,6 +146,35 @@ describe('AgentChatPanel', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('routes a runtime tool-approval decision back as an allow reply', async () => {
|
||||
contactSessions.current = [pinged];
|
||||
transcript.current = {
|
||||
state: { status: 'ok' },
|
||||
messages: [
|
||||
{
|
||||
id: 'ap',
|
||||
from: 'agent',
|
||||
body: 'gh pr status',
|
||||
timestamp: '2026-07-08T00:00:00Z',
|
||||
encrypted: false,
|
||||
eventKind: 'approval_request',
|
||||
toolName: 'shell',
|
||||
},
|
||||
],
|
||||
refresh: vi.fn(),
|
||||
};
|
||||
render(<AgentChatPanel />);
|
||||
fireEvent.click(screen.getByTestId('orch-agent-view-session-s-auth'));
|
||||
fireEvent.click(screen.getByText('chat.approval.approve'));
|
||||
await waitFor(() =>
|
||||
expect(sendMasterMessage).toHaveBeenCalledWith({
|
||||
body: 'allow',
|
||||
recipient: '@peer',
|
||||
sessionId: 's-auth',
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('surfaces a session reply failure', async () => {
|
||||
contactSessions.current = [pinged];
|
||||
sendMasterMessage.mockRejectedValueOnce(new Error('boom'));
|
||||
|
||||
@@ -151,6 +151,38 @@ describe('ConnectionsPanel', () => {
|
||||
expect(screen.getByTestId('orch-connections-panel')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('routes a runtime tool-approval decision back as an allow reply', async () => {
|
||||
const contact = { agentId: ADDR, status: 'accepted', direction: 'outgoing' };
|
||||
sessionsHook.byContact.set(ADDR, [session({})]);
|
||||
pairing.current = { ...pairing.current, state: okState([contact]) as never };
|
||||
transcriptHook.current = {
|
||||
state: { status: 'ok' },
|
||||
messages: [
|
||||
{
|
||||
id: 'ap',
|
||||
from: 'agent',
|
||||
body: 'gh pr status',
|
||||
timestamp: '2026-07-08T00:00:00Z',
|
||||
encrypted: false,
|
||||
eventKind: 'approval_request',
|
||||
toolName: 'shell',
|
||||
},
|
||||
],
|
||||
refresh: vi.fn(),
|
||||
};
|
||||
render(<ConnectionsPanel />);
|
||||
fireEvent.click(screen.getByTestId(`orch-connection-${ADDR}`).querySelector('button')!);
|
||||
fireEvent.click(await screen.findByTestId('orch-session-s1'));
|
||||
fireEvent.click(screen.getByText('chat.approval.approve'));
|
||||
await waitFor(() =>
|
||||
expect(sendMasterMessage).toHaveBeenCalledWith({
|
||||
body: 'allow',
|
||||
recipient: ADDR,
|
||||
sessionId: 's1',
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('surfaces a reply send failure instead of swallowing it', async () => {
|
||||
const contact = { agentId: ADDR, status: 'accepted', direction: 'outgoing' };
|
||||
sessionsHook.byContact.set(ADDR, [session({})]);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { fireEvent, render, screen } from '@testing-library/react';
|
||||
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import type { ChatMessage } from '../../../lib/orchestration/useOrchestrationChats';
|
||||
@@ -73,13 +73,76 @@ describe('SessionTranscript', () => {
|
||||
expect(screen.queryByText('chat.approval.approve')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('wires approval buttons to onDecide', () => {
|
||||
it('wires approval buttons to onDecide and resolves the card in place', () => {
|
||||
const onDecide = vi.fn();
|
||||
const approval = msg({ id: 'ap', eventKind: 'approval_request', body: 'run it' });
|
||||
render(<SessionTranscript messages={[approval]} onDecide={onDecide} />);
|
||||
fireEvent.click(screen.getByText('chat.approval.approve'));
|
||||
fireEvent.click(screen.getByRole('button', { name: 'chat.approval.approve' }));
|
||||
expect(onDecide).toHaveBeenCalledWith(approval, 'approve');
|
||||
fireEvent.click(screen.getByText('chat.approval.deny'));
|
||||
// After deciding, the card resolves in place: buttons are gone, outcome shown.
|
||||
expect(screen.queryByRole('button', { name: 'chat.approval.deny' })).not.toBeInTheDocument();
|
||||
expect(screen.getByTestId('approval-resolved')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('resolves to a denied outcome when denied', () => {
|
||||
const onDecide = vi.fn();
|
||||
const approval = msg({ id: 'ap', eventKind: 'approval_request', body: 'rm -rf' });
|
||||
render(<SessionTranscript messages={[approval]} onDecide={onDecide} />);
|
||||
fireEvent.click(screen.getByRole('button', { name: 'chat.approval.deny' }));
|
||||
expect(onDecide).toHaveBeenCalledWith(approval, 'deny');
|
||||
expect(screen.queryByRole('button', { name: 'chat.approval.approve' })).not.toBeInTheDocument();
|
||||
expect(screen.getByTestId('approval-resolved')).toHaveTextContent('chat.approval.deny');
|
||||
});
|
||||
|
||||
it('resolves from a persisted decision echo (survives reload) and suppresses that echo', () => {
|
||||
// No onDecide (as after remount), but a paired decision echo is present — the
|
||||
// card renders resolved, and the redundant echo bubble is hidden.
|
||||
render(
|
||||
<SessionTranscript
|
||||
messages={[
|
||||
msg({ id: 'ap', eventKind: 'approval_request', toolName: 'shell', body: 'ls' }),
|
||||
msg({ id: 'echo', from: 'owner', body: 'allow' }),
|
||||
]}
|
||||
/>
|
||||
);
|
||||
expect(screen.getByTestId('approval-resolved')).toBeInTheDocument();
|
||||
expect(screen.queryByRole('button', { name: 'chat.approval.approve' })).not.toBeInTheDocument();
|
||||
expect(screen.queryByText('allow')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('preserves an unpaired one-word owner reply (real chat, no approval)', () => {
|
||||
render(
|
||||
<SessionTranscript
|
||||
messages={[
|
||||
msg({ id: 'a', from: 'agent', body: 'the answer' }),
|
||||
msg({ id: 'reply', from: 'owner', body: 'allow' }),
|
||||
]}
|
||||
/>
|
||||
);
|
||||
// no preceding approval → "allow" is a normal reply and must stay visible
|
||||
expect(screen.getByText('allow')).toBeInTheDocument();
|
||||
expect(screen.getByText('the answer')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('hides "Always allow" by default and shows it only when enabled', () => {
|
||||
const approval = msg({ id: 'ap', eventKind: 'approval_request', body: 'run it' });
|
||||
const { rerender } = render(<SessionTranscript messages={[approval]} onDecide={vi.fn()} />);
|
||||
expect(
|
||||
screen.queryByRole('button', { name: 'chat.approval.alwaysAllow' })
|
||||
).not.toBeInTheDocument();
|
||||
rerender(<SessionTranscript messages={[approval]} onDecide={vi.fn()} alwaysAllow />);
|
||||
expect(screen.getByRole('button', { name: 'chat.approval.alwaysAllow' })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('rolls the card back to buttons if the decision send fails', async () => {
|
||||
const onDecide = vi.fn().mockRejectedValue(new Error('relay down'));
|
||||
const approval = msg({ id: 'ap', eventKind: 'approval_request', body: 'run it' });
|
||||
render(<SessionTranscript messages={[approval]} onDecide={onDecide} />);
|
||||
fireEvent.click(screen.getByRole('button', { name: 'chat.approval.approve' }));
|
||||
// optimistic resolve, then rollback on rejection → the buttons return for retry
|
||||
await waitFor(() =>
|
||||
expect(screen.getByRole('button', { name: 'chat.approval.approve' })).toBeInTheDocument()
|
||||
);
|
||||
expect(screen.queryByTestId('approval-resolved')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user