feat: Human-in-the-Loop (HITL) approval UI - #104
Merged
Merged
Conversation
- New hitl.ts API module (types + functions for both surfaces) - AWAITING_HUMAN state on conversations, CANCELLED state on groups - HITL pause bookmark fields on conversation/group models - hitlConfig on Agent and AgentGroupConfiguration - SSE events: awaiting_approval, hitl_resume, cancelled - use-hitl.ts hooks (queries with polling + mutations) - ApprovalBanner component (timeout countdown, per-task approvals) - Approvals queue page (/manage/approvals) with search + actions - Sidebar + router integration - conversation-detail: approval banner for AWAITING_HUMAN - conversations: AWAITING_HUMAN filter and state rendering - discussion-transcript: approval banner for AWAITING_APPROVAL - group-detail: CANCELLED state + cancel button - task-board: AWAITING_APPROVAL column - MSW handlers for all HITL endpoints - i18n: 27 hitl.* keys across all 11 locales
…ckend contract Makes the HITL UI functional end-to-end and configurable, and adapts it to the current EDDI feat/hitl-framework contract. Group approval flow - Wire the group ApprovalBanner to a real approve/reject via the approve/stream SSE endpoint (approveAndStream) so the discussion resumes live; fix the cancel path (gcId), and drive success/error toasts off the actual resume outcome rather than optimistically. - Handle the error/group_error and member_pause_skipped SSE events; on pause, switch to the persisted conversation (with a live fallback to avoid a skeleton flash) so the banner shows the countdown, timeout policy and awaiting tasks. HITL config editors - Agent editor: HitlConfig section (timeout policy + ISO-8601 approval timeout with client validation and a seeded default for finite policies, plus the new pauseReason field). - Group wizard: per-phase approval points (materialised from the exact preset phases), timeout policy/duration, and TASK granularity + on-rejection for TASK_FORCE; read-only HITL summary in the group config panel. Backend contract adaptation - Use the new cross-group GET /groups/pending-approvals inbox (single request) instead of a per-group fan-out; mirror agent-level pauseReason. Correctness & polish - Central, backend-accurate ISO-8601 duration parsing (fraction on seconds only); correct react-query invalidations; task-board awaiting-approval column; a11y (aria-pressed, labelled note); i18n across all 11 locales. - Fix the vacuous typecheck script (tsc --noEmit -> tsc -b). Tests: hitl-config, hitl-labels, approval-banner, use-hitl, config section, and group-stream HITL SSE transitions; MSW handlers for all HITL endpoints.
📝 WalkthroughWalkthroughChangesDS-sync build/validate/capture pipeline
Estimated code review effort: 5 (Critical) | ~150 minutes Human-in-the-loop approval workflow
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ApprovalBanner
participant useGroupDiscussionStream
participant GroupsAPI as "groups.ts (streamGroupApproval)"
participant Backend
User->>ApprovalBanner: click Approve/Reject
ApprovalBanner->>useGroupDiscussionStream: onDecide(verdict, note, taskApprovals/toolDecisions)
useGroupDiscussionStream->>GroupsAPI: approveAndStream(groupId, gcId, request)
GroupsAPI->>Backend: POST /approve/stream
Backend-->>GroupsAPI: SSE hitl_resume, ... , group_complete
GroupsAPI-->>useGroupDiscussionStream: yield GroupSSEEvent
useGroupDiscussionStream-->>ApprovalBanner: streamState (hitlResume, COMPLETED)
ApprovalBanner-->>User: toast success / updated transcript
sequenceDiagram
participant Chat as ChatPanel
participant Hook as useSendMessage
participant Backend
Chat->>Hook: send message
Hook->>Backend: POST /agents/:conversationId
Backend-->>Hook: 409 (awaiting human approval)
Hook->>Hook: remove optimistic message, setPaused(true, reason)
Hook-->>Chat: isPaused=true, pauseReason
Chat-->>Chat: render pause banner, disable input
Possibly related PRs
Suggested labels: enhancement, feature, needs-review Suggested reviewers: (based on repository ownership of 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (12)
src/pages/approvals.tsx (1)
151-157: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winIcon-only refresh button lacks an accessible name.
Screen readers announce nothing meaningful for this control.
♿ Proposed fix
<button onClick={handleRefresh} className="inline-flex items-center gap-1.5 rounded-lg border border-input bg-background px-3 py-2 text-sm font-medium text-foreground hover:bg-muted transition-colors" data-testid="refresh-approvals" + aria-label={t("common.refresh", "Refresh")} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/approvals.tsx` around lines 151 - 157, The refresh button rendered in approvals.tsx via the handleRefresh button currently has only an icon, so it needs an accessible name for screen readers. Add an appropriate text alternative to the button itself in the approvals page component (for example on the button using RefreshCw), and make sure the control is still visually icon-only while exposing a clear label to assistive tech.src/lib/api/groups.ts (2)
214-222: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider typing
hitlTimeoutPolicy/hitlApprovalTimeoutmore strictly.These are declared as plain
string, butgroup-wizard.tsxtreats timeout policy as a constrained set of values (requiresApprovalTimeout(state.hitl.timeoutPolicy)) and validatesapprovalTimeoutas an ISO-8601 duration. Reusing the union type fromhitl.tshere (rather thanstring) would preserve type safety for consumers readingGroupConversation.hitlTimeoutPolicy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/api/groups.ts` around lines 214 - 222, The HITL pause fields in GroupConversation are too loosely typed, especially hitlTimeoutPolicy and hitlApprovalTimeout. Update the GroupConversation shape in groups.ts to reuse the stricter HITL types from hitl.ts so consumers like group-wizard.tsx can rely on the same constrained timeout policy values and ISO-8601 duration typing. Keep the existing field names, but replace the plain string annotations with the shared union/type definitions used by the HITL state model.
504-551: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate fetch/header scaffolding between
streamGroupDiscussionandstreamGroupApproval.Both functions build an identical
Content-Type/auth-header fetch call before delegating toreadGroupSSE. Consider extracting a smallpostSSE(url, body, signal)helper to avoid drift between the two call sites.♻️ Sketch of a shared helper
+async function postSSE(path: string, body: unknown, signal?: AbortSignal): Promise<Response> { + return fetch(`${api.getBaseUrl()}${path}`, { + method: "POST", + headers: { "Content-Type": "application/json", ...api.getAuthHeader() }, + body: JSON.stringify(body), + signal, + }); +} + export async function* streamGroupDiscussion(...): AsyncGenerator<GroupSSEEvent> { - const response = await fetch(`${api.getBaseUrl()}/groups/${groupId}/conversations/stream`, {...}); + const response = await postSSE(`/groups/${groupId}/conversations/stream`, { question, userId: userId || "manager-user" }, signal); yield* readGroupSSE(response); } export async function* streamGroupApproval(...): AsyncGenerator<GroupSSEEvent> { - const response = await fetch(`${api.getBaseUrl()}/groups/${groupId}/conversations/${gcId}/approve/stream`, {...}); + const response = await postSSE(`/groups/${groupId}/conversations/${gcId}/approve/stream`, request, signal); yield* readGroupSSE(response); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/api/groups.ts` around lines 504 - 551, The fetch-and-SSE setup is duplicated in streamGroupDiscussion and streamGroupApproval, which can drift over time. Extract the shared POST logic into a small helper such as postSSE that takes the URL, request body, and optional signal, sets the Content-Type and auth headers via api.getAuthHeader(), performs fetch, and returns the response for readGroupSSE. Update both streamGroupDiscussion and streamGroupApproval to call the helper so the request scaffolding lives in one place.src/hooks/use-group-discussion-stream.ts (1)
41-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMinor type duplication and looser typing for HITL fields.
hitlPause's shape and theawaiting_approvalpayload cast at Line 479-484 duplicate the same literal type; likewisehitlResume.verdictis typedstringrather than theHitlVerdictunion already used elsewhere (e.g.DiscussionTranscriptProps.onApprove). Extracting/reusing shared types would tighten consistency across the HITL surface.Also applies to: 477-524
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/use-group-discussion-stream.ts` around lines 41 - 65, The HITL state types in use-group-discussion-stream are duplicated and too loose, so tighten them by extracting shared types for the pause payload and using the existing HitlVerdict union for hitlResume instead of string. Update the relevant types around hitlPause, hitlResume, and the awaiting_approval event handling to reuse these shared symbols consistently, and align any related casts or prop contracts such as DiscussionTranscriptProps.onApprove so the HITL surface stays type-safe and source-of-truth driven..ds-sync/resync.mjs (1)
67-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate of
remote-diff.mjs's argument-parsing block.Same "unrecognized argument" loop as
lib/remote-diff.mjslines 51-59. Worth consolidating into a shared helper (see companion comment on that file) rather than keeping two copies in sync by hand.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.ds-sync/resync.mjs around lines 67 - 76, The argument-parsing block in resync.mjs duplicates the same unrecognized-argument loop used in remote-diff.mjs, so consolidate this logic into the shared helper used by both scripts instead of keeping a second copy in sync. Refactor the processing around argv, flag, and VALUE_FLAGS in this file to call that shared parser/validator, and keep the --no-render-check and value-flag handling behavior unchanged..ds-sync/lib/remote-diff.mjs (1)
51-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winArgument-parsing boilerplate duplicated with
resync.mjs.This "unrecognized argument" flag-parsing block is near-identical to the one in
resync.mjs(lines 67-76). Consider extracting a shared helper intolib/common.mjs(which already centralizes cross-script utilities likevalidateConfig) so future flag additions/fixes only need to happen once.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.ds-sync/lib/remote-diff.mjs around lines 51 - 59, The argument-parsing block in remote-diff.mjs duplicates the same unrecognized-argument logic used in resync.mjs, so factor it into a shared helper in lib/common.mjs alongside utilities like validateConfig. Update remote-diff.mjs to call that shared helper for parsing/validation, and have resync.mjs use the same helper so future flag changes only need one implementation.src/components/hitl/approval-banner.tsx (1)
42-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCountdown/duration units are hardcoded, not localized.
formatMs/formatDurationemit raw"d","h","m","s"suffixes directly, unlike every other user-facing string in this component which goes throught(...). This is the one piece of text that will render in English regardless of locale (Arabic, Japanese, etc.), even though the rest of the banner (including the adjacent "Paused" timestamp viaIntl.DateTimeFormat) is properly localized.♻️ Suggested approach
-const parts: [number, string][] = [[d, "d"], [h, "h"], [m, "m"], [s, "s"]]; +const parts: [number, string][] = [ + [d, t("hitl.unit.day", "d")], + [h, t("hitl.unit.hour", "h")], + [m, t("hitl.unit.minute", "m")], + [s, t("hitl.unit.second", "s")], +];Note
formatMs/formatDurationcurrently don't receivet; they'd need to accept it as a parameter or be called from within the component with access touseTranslation().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/hitl/approval-banner.tsx` around lines 42 - 60, The duration formatting in formatMs and formatDuration uses hardcoded unit suffixes, so update these helpers to use the component’s translation function instead of raw "d"/"h"/"m"/"s" text. Pass t from ApprovalBanner into formatDuration (and formatMs if needed), then build the displayed string from localized unit labels so all countdown text follows the active locale like the rest of the banner.src/components/hitl/__tests__/approval-banner.test.tsx (1)
1-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo coverage for the countdown/overdue rendering path.
The suite covers decision submission and per-task logic well, but
getTimeRemaining/formatMs/the "Overdue" vs "Remaining" chip branch (component lines 106-199) has no test at all despite being new, timer-driven logic central to this PR's "timeout and countdown handling" objective. Consider adding a test withvi.useFakeTimers()asserting both the "Remaining: …" and "Overdue" states.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/hitl/__tests__/approval-banner.test.tsx` around lines 1 - 89, Add test coverage for the new countdown/overdue render path in ApprovalBanner, since the suite currently only validates decisions and task granularity. In src/components/hitl/__tests__/approval-banner.test.tsx, use vi.useFakeTimers() around renderWithProviders(<ApprovalBanner ... />) to exercise the timer-driven logic in ApprovalBanner and assert the chip switches from a “Remaining: …” state (via getTimeRemaining/formatMs) to “Overdue” after advancing time. Include checks that target the same rendered UI branch used by the component’s countdown display so the new timeout handling is covered.src/pages/group-detail.tsx (1)
311-335: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew cancel
<button>is nested inside the row's outer<button>.The row at line 264 is itself a
<button>; the delete button (pre-existing) and now this new cancel button are both rendered as children of it. Nested interactive elements are invalid per the HTML content model and can confuse assistive-tech role/announcement (a screen reader may announce ambiguous or duplicated button semantics), even thoughstopPropagation()keeps click behavior working for mouse users.♻️ Suggested approach — make the row a non-button container with an explicit click handler, so action buttons are true siblings
- <button + <div + role="button" + tabIndex={0} key={conv.id} onClick={() => handleSelectConversation(conv.id)} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") handleSelectConversation(conv.id); + }} className={cn(...)} ... > ... - </button> + </div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/group-detail.tsx` around lines 311 - 335, The row item in group detail is using a button as the outer container while also rendering the delete and cancel action buttons inside it, creating invalid nested interactive elements. Update the row wrapper in group-detail.tsx so the clickable row is a non-button container with an explicit click handler, and keep the action controls in the same row as sibling buttons. Preserve the existing behavior in handleDeleteConversation and handleCancelDiscussion, but ensure the new cancel button and the existing trash button are no longer descendants of the row’s main clickable control.src/components/groups/__tests__/discussion-transcript.test.tsx (1)
183-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo new test exercises the AWAITING_APPROVAL / ApprovalBanner path.
Both changed spots only add
hitlPause/hitlResume/cancelInfo: nullto existing mock objects for unrelated (IN_PROGRESS/FAILED) scenarios. Given this PR wires a whole newApprovalBannerrender branch,onApprove/onCancelDiscussioncallbacks, andtasksAwaitingApprovalderivation into this component, a test renderingDiscussionTranscriptwithstreamState.state === "AWAITING_APPROVAL"(and/or a persistedconversationin that state) asserting the banner appears andonApprove/onCancelDiscussionfire correctly would meaningfully cover a new critical path.Also applies to: 220-226
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/groups/__tests__/discussion-transcript.test.tsx` around lines 183 - 189, Add a test for the new AWAITING_APPROVAL flow in DiscussionTranscript, since the current mock updates only cover unrelated states. Extend src/components/groups/__tests__/discussion-transcript.test.tsx with a case where streamState.state is AWAITING_APPROVAL (or the conversation is persisted in that state), then assert ApprovalBanner renders and that the onApprove and onCancelDiscussion handlers are invoked correctly. Use the DiscussionTranscript component and the new tasksAwaitingApproval/ApprovalBanner path as the main symbols to target.src/pages/group-wizard.tsx (1)
852-864: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider auto-seeding a valid approval timeout on policy switch, like the agent-level config.
agent-config-sections.tsx'sHitlConfigSectionseedsapprovalTimeout: "PT15M"when switching to a finite policy so the config never lands in an obviously-invalid state. Here, switching to a finitetimeoutPolicyleavesapprovalTimeoutuntouched (confirmed by the wizard test expectingNextto become disabled), forcing the user to manually type a duration before proceeding. Mirroring the agent-level seeding behavior would reduce friction and keep both HITL config UIs consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/group-wizard.tsx` around lines 852 - 864, The HITL timeout policy selector in group-wizard should seed a valid approval timeout when switching from WAIT_INDEFINITELY to a finite policy, matching HitlConfigSection in agent-config-sections.tsx. Update the onChange handling around patchHitl so that choosing AUTO_APPROVE, AUTO_REJECT, or ABORT also sets approvalTimeout to a sensible default (for example PT15M) when it is currently unset or indefinite, keeping the wizard state valid and consistent with the agent-level config.src/components/groups/group-config-panel.tsx (1)
207-215: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a shared formatter for
approvalTimeout.
approvalTimeoutis rendered as raw ISO-8601 text here, while the approval banner shows a formatted duration. Move that formatting into a shared HITL helper and reuse it in both places.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/groups/group-config-panel.tsx` around lines 207 - 215, The approval timeout is being rendered inconsistently in group-config-panel.tsx: the `approvalTimeout` value is shown as raw ISO-8601 text here while the approval banner uses a formatted duration. Extract the timeout formatting into a shared HITL helper (for example alongside `timeoutPolicyLabel` or the existing approval banner formatting logic) and update both this `InfoRow` in `group-config-panel` and the banner to call that helper so they display the same formatted duration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.ds-sync/lib/bundle.mjs:
- Around line 133-144: The alias resolution in tsconfig-paths currently returns
as soon as existsSync finds a match, which can incorrectly accept directories
and block the /index.* fallback. Update the onResolve logic in bundle.mjs to
verify the candidate at stem + ext is a file before returning it, using
statSync(...).isFile() in the tsconfig-paths resolver so directory aliases like
`@/components` continue on to the index candidates.
- Around line 269-270: The evidence pass in bundle.mjs is incorrectly assuming
the first metafile output is the entry JS bundle, which can pick a CSS output
and leave exports empty. Update the logic around the metafile outputs lookup to
select the output associated with the entry point instead of using
Object.values(...)[0], then keep the __dsMainNs filter and export collection in
the same flow.
In @.ds-sync/package-validate.mjs:
- Around line 374-396: The .d.ts validation walk in the package-validate logic
is too brittle: it calls walkDts on join(OUT, 'components') without checking
that directory exists, so tokens-only syncs can throw and get mislabeled by the
catch. Update the walkDts invocation to follow the same existsSync guard pattern
used elsewhere, and in the TypeScript parse loop replace the internal
sf.parseDiagnostics access with a supported syntactic-diagnostics API on the ts
SourceFile/createSourceFile flow so the check remains stable across TypeScript
versions.
- Around line 413-434: The render-check setup in package-validate.mjs is failing
early when OUT/components does not exist, because collect() calls readdirSync on
that path before the guarded render-check flow starts. Add an existsSync check
for the components directory or move the whole setup into the existing try block
so tokens-only syncs skip render-check cleanly. Keep the fix centered around
collect(), serveDir(OUT), and mkdirSync(shotDir, ...) so missing components do
not abort validation.
In @.ds-sync/storybook/compare.mjs:
- Around line 437-444: The catch block in compare.mjs assumes the rejection
reason has a message property, which can crash if goto throws a null/undefined
value or a non-Error. Update the catch handling around dsPage.goto so the
non-timeout branch safely derives the error text from the caught value itself,
guarding access before using split; keep the existing timeout check and preserve
pageErr assignment logic in compare.mjs.
In `@package.json`:
- Line 11: The CONTRIBUTING.md CI-checks table is stale for the Type Check step
and still says tsc --noEmit, but the package.json typecheck script now uses tsc
-b. Update the documentation entry for the Type Check/CI-checks row to match the
current npm run typecheck behavior so contributors can debug failures against
the correct command.
In `@src/components/editors/agent-config-sections.tsx`:
- Around line 1098-1104: The timeout input draft in the HITL editor is only
synced from hitl.approvalTimeout, so it can stay stale when the finite
timeoutPolicy is toggled off and back on. Update the timeoutDraft state in
agent-config-sections.tsx so it also resyncs when
requiresApprovalTimeout(hitl.timeoutPolicy) changes, not just when
hitl.approvalTimeout changes, and make sure the policy-switch handling around
the HITL timeout field uses the same source of truth. Keep the draft reset
behavior aligned with the existing useEffect and the timeout field rendering
logic so the value shown after re-enabling a finite policy matches the persisted
approvalTimeout.
In `@src/hooks/use-group-discussion-stream.ts`:
- Around line 265-269: The task attribution in use-group-discussion-stream
should not rely on payload.displayName alone because duplicate display names can
map progress to the wrong speaker. Update the agentTask lookup in the group
discussion stream logic to match on a unique speaker identifier from the payload
and task plan, or add a disambiguating field alongside displayName, so the
matcher in the task attribution flow is unique and stable.
In `@src/pages/approvals.tsx`:
- Around line 32-33: The approvals page is only using `usePendingApprovals()`
for loading/error handling, so failures from `useAllGroupPendingApprovals()` are
ignored. Update `approvals.tsx` to destructure and combine the
`isLoading`/`isError` state from both hooks (`usePendingApprovals` and
`useAllGroupPendingApprovals`) and drive the page-level branches from the merged
state, while still rendering both `regular` and `groupPendings` data when
available. Ensure the main render logic in the approvals page reflects missing
group approvals as an error state instead of silently showing only regular
approvals.
---
Nitpick comments:
In @.ds-sync/lib/remote-diff.mjs:
- Around line 51-59: The argument-parsing block in remote-diff.mjs duplicates
the same unrecognized-argument logic used in resync.mjs, so factor it into a
shared helper in lib/common.mjs alongside utilities like validateConfig. Update
remote-diff.mjs to call that shared helper for parsing/validation, and have
resync.mjs use the same helper so future flag changes only need one
implementation.
In @.ds-sync/resync.mjs:
- Around line 67-76: The argument-parsing block in resync.mjs duplicates the
same unrecognized-argument loop used in remote-diff.mjs, so consolidate this
logic into the shared helper used by both scripts instead of keeping a second
copy in sync. Refactor the processing around argv, flag, and VALUE_FLAGS in this
file to call that shared parser/validator, and keep the --no-render-check and
value-flag handling behavior unchanged.
In `@src/components/groups/__tests__/discussion-transcript.test.tsx`:
- Around line 183-189: Add a test for the new AWAITING_APPROVAL flow in
DiscussionTranscript, since the current mock updates only cover unrelated
states. Extend src/components/groups/__tests__/discussion-transcript.test.tsx
with a case where streamState.state is AWAITING_APPROVAL (or the conversation is
persisted in that state), then assert ApprovalBanner renders and that the
onApprove and onCancelDiscussion handlers are invoked correctly. Use the
DiscussionTranscript component and the new tasksAwaitingApproval/ApprovalBanner
path as the main symbols to target.
In `@src/components/groups/group-config-panel.tsx`:
- Around line 207-215: The approval timeout is being rendered inconsistently in
group-config-panel.tsx: the `approvalTimeout` value is shown as raw ISO-8601
text here while the approval banner uses a formatted duration. Extract the
timeout formatting into a shared HITL helper (for example alongside
`timeoutPolicyLabel` or the existing approval banner formatting logic) and
update both this `InfoRow` in `group-config-panel` and the banner to call that
helper so they display the same formatted duration.
In `@src/components/hitl/__tests__/approval-banner.test.tsx`:
- Around line 1-89: Add test coverage for the new countdown/overdue render path
in ApprovalBanner, since the suite currently only validates decisions and task
granularity. In src/components/hitl/__tests__/approval-banner.test.tsx, use
vi.useFakeTimers() around renderWithProviders(<ApprovalBanner ... />) to
exercise the timer-driven logic in ApprovalBanner and assert the chip switches
from a “Remaining: …” state (via getTimeRemaining/formatMs) to “Overdue” after
advancing time. Include checks that target the same rendered UI branch used by
the component’s countdown display so the new timeout handling is covered.
In `@src/components/hitl/approval-banner.tsx`:
- Around line 42-60: The duration formatting in formatMs and formatDuration uses
hardcoded unit suffixes, so update these helpers to use the component’s
translation function instead of raw "d"/"h"/"m"/"s" text. Pass t from
ApprovalBanner into formatDuration (and formatMs if needed), then build the
displayed string from localized unit labels so all countdown text follows the
active locale like the rest of the banner.
In `@src/hooks/use-group-discussion-stream.ts`:
- Around line 41-65: The HITL state types in use-group-discussion-stream are
duplicated and too loose, so tighten them by extracting shared types for the
pause payload and using the existing HitlVerdict union for hitlResume instead of
string. Update the relevant types around hitlPause, hitlResume, and the
awaiting_approval event handling to reuse these shared symbols consistently, and
align any related casts or prop contracts such as
DiscussionTranscriptProps.onApprove so the HITL surface stays type-safe and
source-of-truth driven.
In `@src/lib/api/groups.ts`:
- Around line 214-222: The HITL pause fields in GroupConversation are too
loosely typed, especially hitlTimeoutPolicy and hitlApprovalTimeout. Update the
GroupConversation shape in groups.ts to reuse the stricter HITL types from
hitl.ts so consumers like group-wizard.tsx can rely on the same constrained
timeout policy values and ISO-8601 duration typing. Keep the existing field
names, but replace the plain string annotations with the shared union/type
definitions used by the HITL state model.
- Around line 504-551: The fetch-and-SSE setup is duplicated in
streamGroupDiscussion and streamGroupApproval, which can drift over time.
Extract the shared POST logic into a small helper such as postSSE that takes the
URL, request body, and optional signal, sets the Content-Type and auth headers
via api.getAuthHeader(), performs fetch, and returns the response for
readGroupSSE. Update both streamGroupDiscussion and streamGroupApproval to call
the helper so the request scaffolding lives in one place.
In `@src/pages/approvals.tsx`:
- Around line 151-157: The refresh button rendered in approvals.tsx via the
handleRefresh button currently has only an icon, so it needs an accessible name
for screen readers. Add an appropriate text alternative to the button itself in
the approvals page component (for example on the button using RefreshCw), and
make sure the control is still visually icon-only while exposing a clear label
to assistive tech.
In `@src/pages/group-detail.tsx`:
- Around line 311-335: The row item in group detail is using a button as the
outer container while also rendering the delete and cancel action buttons inside
it, creating invalid nested interactive elements. Update the row wrapper in
group-detail.tsx so the clickable row is a non-button container with an explicit
click handler, and keep the action controls in the same row as sibling buttons.
Preserve the existing behavior in handleDeleteConversation and
handleCancelDiscussion, but ensure the new cancel button and the existing trash
button are no longer descendants of the row’s main clickable control.
In `@src/pages/group-wizard.tsx`:
- Around line 852-864: The HITL timeout policy selector in group-wizard should
seed a valid approval timeout when switching from WAIT_INDEFINITELY to a finite
policy, matching HitlConfigSection in agent-config-sections.tsx. Update the
onChange handling around patchHitl so that choosing AUTO_APPROVE, AUTO_REJECT,
or ABORT also sets approvalTimeout to a sensible default (for example PT15M)
when it is currently unset or indefinite, keeping the wizard state valid and
consistent with the agent-level config.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29352b15-3f82-4c9c-b8a6-42f290635365
⛔ Files ignored due to path filters (1)
.ds-sync/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (68)
.ds-sync/lib/bundle.mjs.ds-sync/lib/common.mjs.ds-sync/lib/css-fallback.mjs.ds-sync/lib/css.mjs.ds-sync/lib/detect.mjs.ds-sync/lib/docs.mjs.ds-sync/lib/dts.mjs.ds-sync/lib/emit.mjs.ds-sync/lib/preview-gen-storybook.mjs.ds-sync/lib/preview-rebuild.mjs.ds-sync/lib/previews.mjs.ds-sync/lib/remote-diff.mjs.ds-sync/lib/source-kit.mjs.ds-sync/lib/source-storybook.mjs.ds-sync/lib/story-imports.mjs.ds-sync/lib/sync-hashes.mjs.ds-sync/package-build.mjs.ds-sync/package-capture.mjs.ds-sync/package-validate.mjs.ds-sync/package.json.ds-sync/resync.mjs.ds-sync/storybook/SKILL.md.ds-sync/storybook/compare.mjs.ds-sync/storybook/http-serve.mjs.ds-sync/storybook/probe.mjs.gitignorepackage.jsonsrc/app.tsxsrc/components/editors/__tests__/hitl-config-section.test.tsxsrc/components/editors/agent-config-sections.tsxsrc/components/groups/__tests__/discussion-transcript.test.tsxsrc/components/groups/__tests__/task-board.test.tsxsrc/components/groups/discussion-transcript.tsxsrc/components/groups/group-config-panel.tsxsrc/components/groups/task-board.tsxsrc/components/hitl/__tests__/approval-banner.test.tsxsrc/components/hitl/approval-banner.tsxsrc/components/layout/sidebar.tsxsrc/hooks/__tests__/use-group-discussion-stream.test.tssrc/hooks/__tests__/use-hitl.test.tsxsrc/hooks/use-group-discussion-stream.tssrc/hooks/use-hitl.tssrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/lib/__tests__/hitl-config.test.tssrc/lib/api/agents.tssrc/lib/api/conversations.tssrc/lib/api/groups.tssrc/lib/api/hitl.tssrc/lib/hitl-config.tssrc/lib/hitl-labels.tssrc/pages/__tests__/group-wizard.test.tsxsrc/pages/agent-detail.tsxsrc/pages/approvals.tsxsrc/pages/conversation-detail.tsxsrc/pages/conversations.tsxsrc/pages/group-detail.tsxsrc/pages/group-wizard.tsxsrc/test/mocks/handlers.ts
There was a problem hiding this comment.
Pull request overview
Adds a complete Human-in-the-Loop (HITL) approval experience to EDDI-Manager, covering 1:1 conversations, group discussions (including approve/resume via SSE), and a unified pending-approvals inbox page, plus agent/group configuration UI and supporting API/hooks/mocks.
Changes:
- Introduces HITL runtime UI: approval banners, new conversation state/filter, group pause/resume/cancel handling, and a cross-surface approvals queue (
/manage/approvals). - Adds HITL configuration: agent-level timeout + pauseReason, and group-level approval points / policies / granularity behavior.
- Expands supporting infrastructure: API bindings + hooks, MSW handlers, tests, i18n updates, and includes
.ds-synctooling plus atypecheckscript fix.
Reviewed changes
Copilot reviewed 67 out of 69 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/mocks/handlers.ts | Adds MSW coverage for HITL pending approvals, approval status, resume/cancel, and group approve/stream SSE. |
| src/pages/group-detail.tsx | Wires group HITL approve/resume + cancel into the group detail streaming + transcript experience. |
| src/pages/conversations.tsx | Adds AWAITING_HUMAN UI state icon and filter option in the conversations list. |
| src/pages/conversation-detail.tsx | Renders HITL ApprovalBanner for 1:1 conversations and hooks up resume/cancel mutations. |
| src/pages/approvals.tsx | New unified “Pending Approvals” inbox page (1:1 + cross-group). |
| src/pages/agent-detail.tsx | Adds the HITL configuration section into the agent editor page. |
| src/pages/tests/group-wizard.test.tsx | Adds group wizard tests for HITL gating, timeout validation, and granularity controls. |
| src/lib/hitl-labels.ts | Centralizes localized labels for HITL enums (timeout policy, granularity, rejection). |
| src/lib/hitl-config.ts | Adds preset phase materialization and ISO-8601 duration validation utilities for HITL config. |
| src/lib/api/hitl.ts | Introduces HITL API DTOs and endpoint bindings (pending approvals, resume/cancel, group cancel, etc.). |
| src/lib/api/groups.ts | Extends group conversation state/types and adds streamGroupApproval (approve/stream) support. |
| src/lib/api/conversations.ts | Extends 1:1 conversation state/type to include AWAITING_HUMAN and HITL bookmark fields. |
| src/lib/api/agents.ts | Adds agent-level hitlConfig typing to agent model. |
| src/lib/tests/hitl-config.test.ts | Unit tests for phase materialization and ISO duration validation helpers. |
| src/i18n/locales/en.json | Adds HITL/approvals strings and common strings used by the new UI. |
| src/i18n/locales/de.json | Propagates HITL/approvals strings to German locale. |
| src/i18n/locales/fr.json | Propagates HITL/approvals strings to French locale. |
| src/i18n/locales/es.json | Propagates HITL/approvals strings to Spanish locale. |
| src/i18n/locales/ar.json | Propagates HITL/approvals strings to Arabic locale. |
| src/i18n/locales/zh.json | Propagates HITL/approvals strings to Chinese locale. |
| src/i18n/locales/th.json | Propagates HITL/approvals strings to Thai locale. |
| src/i18n/locales/ja.json | Propagates HITL/approvals strings to Japanese locale. |
| src/i18n/locales/ko.json | Propagates HITL/approvals strings to Korean locale. |
| src/i18n/locales/pt.json | Propagates HITL/approvals strings to Portuguese locale. |
| src/i18n/locales/hi.json | Propagates HITL/approvals strings to Hindi locale. |
| src/hooks/use-hitl.ts | Adds React Query hooks for pending approvals + resume/cancel (regular + group cancel). |
| src/hooks/use-group-discussion-stream.ts | Adds HITL pause/resume/cancel SSE state handling and approve/stream support. |
| src/hooks/tests/use-hitl.test.tsx | Adds tests for HITL hooks (pending approvals, resume, validation error surfacing). |
| src/hooks/tests/use-group-discussion-stream.test.ts | Adds tests for HITL SSE transitions (awaiting_approval, hitl_resume, cancelled, error). |
| src/components/layout/sidebar.tsx | Adds navigation entry for the new approvals page. |
| src/components/hitl/tests/approval-banner.test.tsx | Adds unit tests for ApprovalBanner decisions, notes, and per-task approvals. |
| src/components/groups/task-board.tsx | Adds an “Awaiting Approval” column and status bucketing for HITL TASK pauses. |
| src/components/groups/group-config-panel.tsx | Adds read-only HITL summary block in group config panel. |
| src/components/groups/discussion-transcript.tsx | Renders group HITL ApprovalBanner and supports per-task approvals + cancel actions. |
| src/components/groups/tests/task-board.test.tsx | Adds coverage for awaiting-approval status outranking “completed”. |
| src/components/groups/tests/discussion-transcript.test.tsx | Updates fixtures to include new HITL fields in stream state. |
| src/components/editors/agent-config-sections.tsx | Implements the agent HITL config editor section (policy + timeout + pauseReason). |
| src/components/editors/tests/hitl-config-section.test.tsx | Adds unit tests for the agent HITL config section behavior. |
| src/app.tsx | Registers the /manage/approvals route. |
| package.json | Fixes typecheck script to run project build checks (tsc -b). |
| .gitignore | Ignores generated ds-bundle/ output from .ds-sync tooling. |
| .ds-sync/storybook/probe.mjs | Adds storybook provider-chain probing helper for design-system sync tooling. |
| .ds-sync/storybook/http-serve.mjs | Adds a minimal static file server for .ds-sync storybook utilities. |
| .ds-sync/package.json | Adds .ds-sync tool dependencies (esbuild, ts-morph, etc.). |
| .ds-sync/lib/sync-hashes.mjs | Adds hashing/recipe logic used by design-system sync pipeline. |
| .ds-sync/lib/source-kit.mjs | Adds non-storybook package adapter/resolution logic for design-system sync. |
| .ds-sync/lib/previews.mjs | Adds preview generation and compilation logic for design-system sync. |
| .ds-sync/lib/preview-gen-storybook.mjs | Adds storybook preview wrapper generator for design-system sync. |
| .ds-sync/lib/detect.mjs | Adds storybook/package shape detection for design-system sync. |
| .ds-sync/lib/css.mjs | Adds CSS/token/font extraction and styles.css writer for design-system sync. |
| .ds-sync/lib/css-fallback.mjs | Adds storybook-based CSS and font fallbacks for design-system sync. |
| .ds-sync/lib/common.mjs | Adds shared helpers/config validation/error remedies for design-system sync. |
Files not reviewed (1)
- .ds-sync/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add the missing taskBoard.awaitingApproval i18n key (was falling back to English in every locale) across all 11 locales. - Surface a non-blocking warning when the cross-group approvals inbox fails to load instead of silently showing only 1:1 approvals; give the refresh button an accessible name. - Resync the agent approval-timeout draft when the field is re-shown after a policy toggle; seed a default timeout on finite-policy switch in the group wizard too (consistent with the agent editor). - Allow null on nullable PendingApprovalSummary fields; tighten GroupConversation HITL field types to the shared unions. - Centralise ISO-8601 duration formatting (formatIsoDuration/formatDurationMs) and reuse it in the banner and group config panel; DRY the two SSE POST calls. - Update CONTRIBUTING.md (typecheck is now tsc -b). Tests: countdown/overdue timer, the AWAITING_APPROVAL banner + decision callbacks, and the updated wizard finite-policy seeding.
The backend emits approve/stream resume rejections (409 concurrent decision, 400 invalid taskApprovals/note) as "group_error", never a bare "error" — a raw "error" name collides with the browser EventSource transport-error event (EDDI #36). - Remove the dead "error" SSE union member and switch case, and correct the now-false comments to reflect that group_error carries these rejections. - Retarget the rejection test to emit the real "group_error" event. - Classify config-drift aborts via errorKind so the UI can guide the user to fix the config and re-approve instead of treating the pause as terminal.
The previous commit made errorKind a required field on GroupStreamState, but the DiscussionTranscript fixtures that construct that type were in a separate uncommitted change — so the committed tree failed `tsc -b` in CI (TS2741) even though the local working tree passed. Add errorKind to both fixtures to restore the typecheck.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/group-wizard.tsx (1)
186-196: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReselecting a template leaves stale
approvalPhasesreferencing a different style's phase names.
selectTemplateupdatesstyle/maxRoundsbut doesn't resetapprovalPhases(unlike the style-card handler at Line 676 and the max-rounds handler at Lines 715-724, which both prune/reset phase selections on style/rounds changes). Since the step navigator lets users jump back to the template step from any later step, a user can enable HITL, pick approval phases for template A, go back, and pick template B (different style).isHitlConfigValidonly checksapprovalPhases.length, not name validity, so it still passes — but none of the stale names will match template B's phases, andapplyApprovalPhasessilently gates nothing, producing a group where HITL looks enabled but never pauses.🐛 Proposed fix
function selectTemplate(tmpl: GroupTemplate) { update({ name: tmpl.name, description: tmpl.description, style: tmpl.style, maxRounds: tmpl.maxRounds, members: tmpl.roles.map((r, i) => createEmptySlot(i, r.displayName, r.role)), moderator: tmpl.moderatorSuggested ? createModeratorSlot() : null, + approvalPhases: [], }); setCurrentStep(1); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/group-wizard.tsx` around lines 186 - 196, selectTemplate currently updates the template’s name/style/maxRounds/members but leaves approvalPhases untouched, so stale phase names can survive when switching templates. Update the selectTemplate handler to reset or prune approvalPhases based on the newly selected template’s style, matching the cleanup behavior used in the style-card and max-rounds handlers. Use the existing selectTemplate function, the approvalPhases state, and the same phase-mapping logic already used elsewhere in group-wizard.tsx to ensure HITL selections stay consistent after reselecting a template.src/components/groups/__tests__/discussion-transcript.test.tsx (1)
213-215: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMissing
errorKindbreaks the build (TS2741).Both
mockStreamStateandmockStreamStateWithErrorfail type-checking per CI:errorKindis required onGroupStreamStatebut is not set on either mock, causingtsc -bto fail the build.🐛 Proposed fix
tasksInProgress: new Set(), tasksCompleted: new Set(), + errorKind: null, hitlPause: null, hitlResume: null, cancelInfo: null, };Apply the same addition to
mockStreamStateWithErrorat line 250.Also applies to: 250-252
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/groups/__tests__/discussion-transcript.test.tsx` around lines 213 - 215, Both `mockStreamState` and `mockStreamStateWithError` are missing the required `errorKind` field from `GroupStreamState`, which is causing the TypeScript build failure. Add `errorKind` to each mock object in `discussion-transcript.test.tsx`, matching the existing mock shape and any error-specific value already used by the test data so both assignments satisfy the type.Source: Pipeline failures
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/groups/__tests__/discussion-transcript.test.tsx`:
- Around line 213-215: Both `mockStreamState` and `mockStreamStateWithError` are
missing the required `errorKind` field from `GroupStreamState`, which is causing
the TypeScript build failure. Add `errorKind` to each mock object in
`discussion-transcript.test.tsx`, matching the existing mock shape and any
error-specific value already used by the test data so both assignments satisfy
the type.
In `@src/pages/group-wizard.tsx`:
- Around line 186-196: selectTemplate currently updates the template’s
name/style/maxRounds/members but leaves approvalPhases untouched, so stale phase
names can survive when switching templates. Update the selectTemplate handler to
reset or prune approvalPhases based on the newly selected template’s style,
matching the cleanup behavior used in the style-card and max-rounds handlers.
Use the existing selectTemplate function, the approvalPhases state, and the same
phase-mapping logic already used elsewhere in group-wizard.tsx to ensure HITL
selections stay consistent after reselecting a template.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49f42def-3906-4338-b597-a1842c833b19
📒 Files selected for processing (25)
CONTRIBUTING.mdsrc/components/editors/agent-config-sections.tsxsrc/components/groups/__tests__/discussion-transcript.test.tsxsrc/components/groups/group-config-panel.tsxsrc/components/hitl/__tests__/approval-banner.test.tsxsrc/components/hitl/approval-banner.tsxsrc/hooks/__tests__/use-group-discussion-stream.test.tssrc/hooks/use-group-discussion-stream.tssrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/lib/api/groups.tssrc/lib/api/hitl.tssrc/lib/hitl-config.tssrc/pages/__tests__/group-wizard.test.tsxsrc/pages/approvals.tsxsrc/pages/group-wizard.tsx
✅ Files skipped from review due to trivial changes (5)
- src/i18n/locales/en.json
- src/i18n/locales/zh.json
- src/i18n/locales/de.json
- src/i18n/locales/ar.json
- src/i18n/locales/es.json
🚧 Files skipped from review as they are similar to previous changes (14)
- src/pages/tests/group-wizard.test.tsx
- src/hooks/tests/use-group-discussion-stream.test.ts
- src/components/groups/group-config-panel.tsx
- src/lib/api/hitl.ts
- src/pages/approvals.tsx
- src/i18n/locales/hi.json
- src/components/editors/agent-config-sections.tsx
- src/components/hitl/approval-banner.tsx
- src/i18n/locales/fr.json
- src/i18n/locales/ko.json
- src/i18n/locales/ja.json
- src/i18n/locales/th.json
- src/hooks/use-group-discussion-stream.ts
- src/i18n/locales/pt.json
Reselecting a template updated style/maxRounds but left approvalPhases referencing the previous style's phase names. Since users can jump back to the template step, this produced a group where HITL looked enabled but never paused (stale names matched no phases). Mirror the reset already done by the style-card and max-rounds handlers.
…gating, add validator mirror Extend the HITL API contract for the tool-call approval surface: per-callId toolDecisions on HitlDecision, pauseType/toolNames on PendingApprovalSummary, and the structured approval-status pauseDetails types (TOOL_CALL/RULE) with their field-limit constants. Add hitl-tool-approvals.ts, a client-side mirror of the backend ToolApprovalPatterns/HitlConfigValidation save-time validators so a bad tool-approvals config is caught inline instead of bouncing off a 400. Add localized labels for the new onNoProgress/rejection-policy enums.
Add ToolApprovalsEditor — the config-driven tool-call approval gate UI (requireApproval/exempt patterns, pause limits, timeout override, group-turn behavior) — wired into the agent-level HITL config and, via task-tool-approvals-section, as a per-LLM-task full-replace override. Live validation against the client-side ToolApprovalPatterns/HitlConfigValidation mirror surfaces save-time errors inline, and warns when an agent-level AUTO_APPROVE would silently demote a tool pause to WAIT_INDEFINITELY. Fixes a stale-draft glitch where toggling the timeout policy off and back on could leave an invalid unsaved approval-timeout value displayed.
Add GroupHitlEditor — an inline editor for a group's HITL settings (timeout policy/duration, per-phase requiresApproval, and, for TASK_FORCE groups, granularity/on-rejection), wired into the group config panel and detail page. Preset-style groups store phases:null; saving materializes the phase list the same way the create wizard does. Blocks save when approval is enabled but no phase is selected, since that would silently produce a group that looks gated but never pauses. Keys the panel on groupId so switching groups remounts the editor instead of reusing stale in-progress edits from the previous group.
…ails loading state Extend ApprovalBanner for a TOOL_CALL pause: per-call approve/reject toggles (unset = inherit the top-level verdict), an optional argument-amendment editor with client-side JSON/size/truncated-arg validation before submit, and surfacing of executedUngatedCalls / outcomeUnknown (crash-recovery) warnings. Rejecting the batch is always all-or-nothing, since a per-call APPROVED mixed with a top-level REJECTED is contradictory and the backend 400s it. Add a pauseDetailsPending flag that disables Approve (not Reject/Cancel) while the structured pause-status fetch is still in flight, so a reviewer can't blind-approve a gated tool call before its details (RULE vs TOOL_CALL) are known.
…eam/load paths Detect an AWAITING_HUMAN commit on the non-streaming send, streaming done, and conversation-load paths, disabling the input and quick-reply pills and showing the paused banner with the backend's pauseReason. Previously the quick-reply pills stayed clickable while paused (the one send affordance the disabled-input guard missed) and a conversation loaded from history always showed the generic default banner text instead of its real reason, since the load path discarded the snapshot's hitlPauseReason. Also drop the trailing empty streaming placeholder message on a 409 (paused) rejection so no perpetual typing indicator lingers under the pause banner.
…chedule pages Merge regular (1:1) and cross-group pending approvals into one inbox queue, badging TOOL_CALL pauses (routed to Review for detailed per-call inspection, never a blind inline quick-approve) versus RULE pauses (inline Approve/Reject). Wait for both the regular and cross-group queries before leaving the loading state, so an empty regular list can't flash "No pending approvals" before slower-loading group items arrive. Surface decidedBy/ automated (system:timeout, system:retention) on the audit page, wire the per-channel HITL approval-routing fields on channel-detail, and the group approval-timeout field on schedules.
Propagate the HITL feature's en.json keys (tool-approval editor labels, per-tool-call approval strings, group Human Approval editor, channel/audit/ schedule wiring) to de, es, fr, pt, ja, ko, zh, ar, hi, th, per the project's mandatory i18n rule.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
src/components/groups/group-hitl-editor.tsx (1)
74-93: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStale
timeoutDraftpersists after switching away from a finite policy.Switching
timeoutPolicyto e.g.AUTO_REJECTseedstimeoutDraftto"PT15M"(lines 161-166), but switching back toWAIT_INDEFINITELY/AUTO_APPROVEnever clears it. On save, line 81 still sends that leftoverapprovalTimeouteven though it's no longer needed/shown.♻️ Proposed fix
- onChange={(e) => { + onChange={(e) => { const policy = e.target.value as GroupHitlConfig["timeoutPolicy"]; patchHitl({ timeoutPolicy: policy }); if ( requiresApprovalTimeout(policy) && !(timeoutDraft && isValidIsoDuration(timeoutDraft)) ) { setTimeoutDraft("PT15M"); + } else if (!requiresApprovalTimeout(policy)) { + setTimeoutDraft(""); } }}Also applies to: 155-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/groups/group-hitl-editor.tsx` around lines 74 - 93, The save flow in group-hitl-editor’s save function is still using a stale timeoutDraft value after switching away from a finite timeout policy. Update the timeoutPolicy handling in the component’s state logic so that when the policy changes to WAIT_INDEFINITELY or AUTO_APPROVE, timeoutDraft is cleared or ignored, and ensure save only includes approvalTimeout when the selected policy actually requires it. Use the existing timeoutPolicy setter logic and the save() path that builds GroupHitlConfig to keep the draft and persisted config in sync.src/components/editors/agent-config-sections.tsx (1)
1149-1149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared
MAX_PAUSE_REASON_LENGTHconstant instead of a hardcoded500.
tool-approvals-editor.tsxalready importsMAX_PAUSE_REASON_LENGTHfrom@/lib/api/hitlfor the identical constraint. Hardcoding500here duplicates that value and can silently drift if the backend limit changes.♻️ Proposed fix
+import { MAX_PAUSE_REASON_LENGTH } from "`@/lib/api/hitl`"; ... - maxLength={500} + maxLength={MAX_PAUSE_REASON_LENGTH}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editors/agent-config-sections.tsx` at line 1149, Replace the hardcoded maxLength value in the pause reason/editor input within agent-config-sections.tsx with the shared MAX_PAUSE_REASON_LENGTH constant, matching the pattern already used in tool-approvals-editor.tsx. Locate the relevant input by the maxLength={500} prop and update it to reference MAX_PAUSE_REASON_LENGTH from `@/lib/api/hitl` so both editors stay aligned with the same backend limit.src/pages/group-detail.tsx (1)
323-336: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCancel button disables list-wide during any single cancel.
disabled={cancelDiscussionMutation.isPending}is driven by one shared mutation instance, so cancelling conversation A also disables the cancel button on every other unrelated conversation row until it settles. Minor UX nit given how infrequent/fast cancels are.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/group-detail.tsx` around lines 323 - 336, The cancel action in group-detail’s conversation row is using a shared pending flag, which disables every cancel button at once. Update the button logic around the conversation item render so each row only disables its own cancel control based on that row’s in-flight state, using the existing handleCancelDiscussion and cancelDiscussionMutation setup as the reference points. Keep the mutation scoped to the clicked conversation so unrelated rows remain interactive while one cancel is pending.src/components/hitl/approval-banner.tsx (1)
592-602: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the amended-arguments limit for
maxLength.MAX_TOOL_CALL_NOTE_LENGTH * 40lets the textarea accept more input thanAMENDED_ARGS_MAX_BYTESallows at submit time, so the rejection only appears later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/hitl/approval-banner.tsx` around lines 592 - 602, The amend textarea in approval-banner.tsx is using a length cap that exceeds the actual amended-arguments limit, so align its maxLength with the same limit enforced at submit time. Update the textarea in the showAmend block to use the amended-arguments constant already used by the validation logic, keeping the behavior consistent with onAmendChange and the tool-call amend flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/editors/tool-approvals-editor.tsx`:
- Around line 136-171: The numeric cap inputs in ToolApprovalsEditor are
committing on every keystroke, unlike the other fields in this component, which
can trigger stale-version mutation races. Update the `maxPausesPerTurn` and
`maxAutoApprovalsPerTurn` inputs to follow the same local draft state plus
`onBlur` commit pattern used by `requireApproval`, `exempt`, `pauseReason`,
`pendingMessage`, and `approvalTimeout`. Keep the input value bound to local
state while typing, then call the existing `onChange`/`patchHitl` flow only when
the field blurs so `updateAgent.mutate` is invoked once per edit instead of on
every keypress.
In `@src/hooks/use-chat.ts`:
- Around line 422-443: The 409 handling in use-chat’s onError only removes the
empty agent streaming placeholder, so the optimistic user message remains even
though the request was rejected. Update the 409 branch to also remove the last
user turn from store.setState when the send was not consumed, while preserving
the existing paused state updates. Keep the fix localized to onError in use-chat
and make sure the transcript state is rolled back consistently before calling
setPaused, setProcessing, and setQuickReplies.
- Line 75: The chat pause state is not being cleared when resuming or cancelling
a conversation, so `useChatStore` can keep stale `isPaused`/`pauseReason`
values. Update `useResumeConversation` and `useCancelConversation` to reset the
store in their success path by calling the relevant `useChatStore` setter (for
example the `setPaused` action) or by refetching the active conversation after
the mutation completes. Make sure the fix is applied in the hooks that manage
resume/cancel flow so `ChatPanel` reflects the current state immediately.
In `@src/i18n/locales/hi.json`:
- Line 997: The locale entry for allSigned is still in English, so update the
hi.json translation to match the surrounding localized keys. Use the same i18n
JSON structure and replace the allSigned value with the proper Hindi
translation, keeping the key consistent with nearby sibling entries.
In `@src/i18n/locales/ja.json`:
- Line 997: The new allSigned entry in the Japanese locale is still in English;
update the string in the ja.json translation map alongside the nearby decidedBy
and automated keys so it matches the rest of the Japanese localization. Use the
existing locale entries in that object to locate and replace the untranslated
value.
In `@src/i18n/locales/th.json`:
- Around line 997-999: The Thai locale is missing a translation for the
audit.allSigned key, so update the th.json entry that currently uses English to
a proper Thai string. Make the change alongside the matching audit keys in the
locale JSON group, using the existing sibling keys like decidedBy and automated
in th.json as the location reference, and ensure the translated value stays
consistent with the corresponding key in en.json and the other locale files.
In `@src/i18n/locales/zh.json`:
- Around line 997-999: The `audit.allSigned` entry in the Chinese locale is
still untranslated, so update the `allSigned` value in
`src/i18n/locales/zh.json` to the proper Chinese text to match the surrounding
`decidedBy` and `automated` keys. Verify the `allSigned` key is consistently
localized alongside the other `audit` strings in the locale JSON files,
following the same translation pattern used in `src/i18n/locales/*.json`.
---
Nitpick comments:
In `@src/components/editors/agent-config-sections.tsx`:
- Line 1149: Replace the hardcoded maxLength value in the pause reason/editor
input within agent-config-sections.tsx with the shared MAX_PAUSE_REASON_LENGTH
constant, matching the pattern already used in tool-approvals-editor.tsx. Locate
the relevant input by the maxLength={500} prop and update it to reference
MAX_PAUSE_REASON_LENGTH from `@/lib/api/hitl` so both editors stay aligned with
the same backend limit.
In `@src/components/groups/group-hitl-editor.tsx`:
- Around line 74-93: The save flow in group-hitl-editor’s save function is still
using a stale timeoutDraft value after switching away from a finite timeout
policy. Update the timeoutPolicy handling in the component’s state logic so that
when the policy changes to WAIT_INDEFINITELY or AUTO_APPROVE, timeoutDraft is
cleared or ignored, and ensure save only includes approvalTimeout when the
selected policy actually requires it. Use the existing timeoutPolicy setter
logic and the save() path that builds GroupHitlConfig to keep the draft and
persisted config in sync.
In `@src/components/hitl/approval-banner.tsx`:
- Around line 592-602: The amend textarea in approval-banner.tsx is using a
length cap that exceeds the actual amended-arguments limit, so align its
maxLength with the same limit enforced at submit time. Update the textarea in
the showAmend block to use the amended-arguments constant already used by the
validation logic, keeping the behavior consistent with onAmendChange and the
tool-call amend flow.
In `@src/pages/group-detail.tsx`:
- Around line 323-336: The cancel action in group-detail’s conversation row is
using a shared pending flag, which disables every cancel button at once. Update
the button logic around the conversation item render so each row only disables
its own cancel control based on that row’s in-flight state, using the existing
handleCancelDiscussion and cancelDiscussionMutation setup as the reference
points. Keep the mutation scoped to the clicked conversation so unrelated rows
remain interactive while one cancel is pending.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e600ad2-cefc-4495-9499-828858540427
📒 Files selected for processing (41)
src/components/chat/chat-panel.tsxsrc/components/editors/__tests__/hitl-config-section.test.tsxsrc/components/editors/__tests__/tool-approvals-editor.test.tsxsrc/components/editors/agent-config-sections.tsxsrc/components/editors/llm-editor.tsxsrc/components/editors/llm/task-tool-approvals-section.tsxsrc/components/editors/llm/types.tssrc/components/editors/tool-approvals-editor.tsxsrc/components/groups/__tests__/discussion-transcript.test.tsxsrc/components/groups/__tests__/group-hitl-editor.test.tsxsrc/components/groups/group-config-panel.tsxsrc/components/groups/group-hitl-editor.tsxsrc/components/hitl/__tests__/approval-banner.test.tsxsrc/components/hitl/approval-banner.tsxsrc/hooks/__tests__/use-chat-load-pause.test.tsxsrc/hooks/use-chat.tssrc/hooks/use-hitl.tssrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/lib/__tests__/hitl-tool-approvals.test.tssrc/lib/api/hitl.tssrc/lib/hitl-labels.tssrc/lib/hitl-tool-approvals.tssrc/pages/__tests__/approvals.test.tsxsrc/pages/__tests__/chat-pause.test.tsxsrc/pages/approvals.tsxsrc/pages/audit.tsxsrc/pages/channel-detail.tsxsrc/pages/conversation-detail.tsxsrc/pages/group-detail.tsxsrc/pages/group-wizard.tsxsrc/pages/schedules.tsx
✅ Files skipped from review due to trivial changes (4)
- src/i18n/locales/ar.json
- src/i18n/locales/es.json
- src/i18n/locales/de.json
- src/i18n/locales/pt.json
🚧 Files skipped from review as they are similar to previous changes (7)
- src/components/groups/tests/discussion-transcript.test.tsx
- src/lib/hitl-labels.ts
- src/pages/conversation-detail.tsx
- src/pages/approvals.tsx
- src/pages/group-wizard.tsx
- src/i18n/locales/fr.json
- src/i18n/locales/ko.json
# Conflicts: # .gitignore # src/components/chat/chat-panel.tsx # src/hooks/use-chat.ts
- tool-approvals editor: commit the numeric caps (maxPausesPerTurn / maxAutoApprovalsPerTurn) on blur via local draft state, matching every sibling field. Committing on each keystroke fired multiple agent mutations with the same stale version, racing the optimistic-concurrency check and silently dropping edits. - chat 409: remove the optimistic user message (not just the empty placeholder) when a send is rejected as paused — the backend never consumed it, so leaving it looked sent-and-received. - chat pause: clear the app-wide chat store's pause banner when the open conversation is resumed/cancelled from the detail page or inbox; the persistent chat drawer/panel doesn't reload on its own. - group HITL editor: don't persist a leftover approvalTimeout after switching back to a non-finite timeout policy. - use MAX_PAUSE_REASON_LENGTH (not a hardcoded 500) for the agent-level pause-reason cap; cap the amend textarea at AMENDED_ARGS_MAX_BYTES. - document that the cross-group inbox `truncated` flag is a client-side heuristic (the backend returns a plain list with no truncation signal). Adds regression tests for the numeric-cap blur commit, the 409 message removal, and the resume-clears-pause behavior.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks/__tests__/use-hitl.test.tsx (1)
71-103: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd cancel-path coverage too.
useCancelConversationalso callsclearChatPauseIfCurrent, so mirror these same/different conversation cases there as well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/__tests__/use-hitl.test.tsx` around lines 71 - 103, The new coverage only exercises `useResumeConversation`, but `useCancelConversation` uses the same `clearChatPauseIfCurrent` behavior and needs the same protection. Add mirrored tests alongside the existing ones for `useCancelConversation`, covering both the matching conversation case where `useChatStore.isPaused` should be cleared and the different conversation case where the pause banner must remain unchanged. Use the existing `useChatStore`, `makeWrapper`, and `mutateAsync` test pattern to keep the scenarios aligned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/hooks/__tests__/use-hitl.test.tsx`:
- Around line 71-103: The new coverage only exercises `useResumeConversation`,
but `useCancelConversation` uses the same `clearChatPauseIfCurrent` behavior and
needs the same protection. Add mirrored tests alongside the existing ones for
`useCancelConversation`, covering both the matching conversation case where
`useChatStore.isPaused` should be cleared and the different conversation case
where the pause banner must remain unchanged. Use the existing `useChatStore`,
`makeWrapper`, and `mutateAsync` test pattern to keep the scenarios aligned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82ec2577-8e56-4c13-bc5a-3cebfab5f80a
📒 Files selected for processing (24)
src/components/chat/chat-panel.tsxsrc/components/editors/__tests__/tool-approvals-editor.test.tsxsrc/components/editors/agent-config-sections.tsxsrc/components/editors/llm-editor.tsxsrc/components/editors/llm/types.tssrc/components/editors/tool-approvals-editor.tsxsrc/components/groups/group-hitl-editor.tsxsrc/components/hitl/approval-banner.tsxsrc/hooks/__tests__/use-chat.test.tsxsrc/hooks/__tests__/use-hitl.test.tsxsrc/hooks/use-chat.tssrc/hooks/use-hitl.tssrc/i18n/locales/ar.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/zh.jsonsrc/test/mocks/handlers.ts
✅ Files skipped from review due to trivial changes (6)
- src/i18n/locales/ko.json
- src/i18n/locales/th.json
- src/i18n/locales/de.json
- src/i18n/locales/hi.json
- src/i18n/locales/pt.json
- src/i18n/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes (15)
- src/components/editors/llm-editor.tsx
- src/components/editors/llm/types.ts
- src/components/editors/tests/tool-approvals-editor.test.tsx
- src/components/chat/chat-panel.tsx
- src/test/mocks/handlers.ts
- src/hooks/use-hitl.ts
- src/components/groups/group-hitl-editor.tsx
- src/i18n/locales/es.json
- src/hooks/use-chat.ts
- src/components/editors/tool-approvals-editor.tsx
- src/components/editors/agent-config-sections.tsx
- src/i18n/locales/en.json
- src/i18n/locales/ar.json
- src/components/hitl/approval-banner.tsx
- src/i18n/locales/fr.json
This was referenced Jul 21, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the Human-in-the-Loop (HITL) approval experience to EDDI-Manager across both the 1:1 conversation surface and the multi-agent group-discussion surface, plus the configuration UI to enable it. Cross-checked against the EDDI backend's
feat/hitl-frameworkHITL contract.What's included
Runtime — approval flow
AWAITING_HUMANconversations (approve/reject with optional note, cancel), and anAWAITING_HUMANstate + filter on the conversations list.approve/streamSSE endpoint; cancel; per-task approvals for TASK granularity; a live countdown / timeout-policy banner; the task board's "Awaiting Approval" column; and handling for theawaiting_approval,hitl_resume,cancelled,error/group_error, andmember_pause_skippedSSE events./manage/approvals): a single inbox of every pending approval (1:1 + group) via the backend'sGET /groups/pending-approvals, with search, quick actions, and per-surface routing.Configuration
pauseReason.i18n: all new strings across the 11 supported locales.
Backend contract
Verified against EDDI
feat/hitl-framework(endpoints, DTOs, SSE event names/payloads, config models, validation). Adopted the new cross-groupGET /groups/pending-approvalsinbox and mirrored the new agent-levelpauseReasonfield; verdict handling is now case-insensitive on the backend (the uppercaseAPPROVED/REJECTEDsent here is fine).Testing
tsc -b(also fixed thetypecheckscript, which was previously a no-op on the solution-style tsconfig),eslint, andvite buildall clean.AWAITING_APPROVALbanner render path.Review response
All 9 CodeRabbit + 2 Copilot actionable findings addressed and resolved (see thread replies for details):
taskBoard.awaitingApprovali18n key (all 11 locales), silently-swallowed cross-group approvals fetch errors (now surfaced as a non-blocking warning), staletimeoutDraftresync on policy toggle, nullablePendingApprovalSummaryfields, and the staleCONTRIBUTING.mdtypecheck doc.GroupConversation..ds-sync/findings (5 threads) — that tooling is unrelated to HITL and tracked for the separate extraction PR noted below; the duplicate-display-name task-matching finding — a backend data constraint (the SSE payload has no stable per-task agent id), not fixable frontend-only.Note on scope
The first commit (
14439b16) also bundles ~9.5k lines of unrelated.ds-syncdesign-system tooling. As discussed, that can be extracted into its own PR — it is not part of the HITL feature and has no runtime impact.Summary by CodeRabbit
New Features
Bug Fixes