feat(frontend): parallel-approval hold, reload integrity, and context-driven config sections#5426
feat(frontend): parallel-approval hold, reload integrity, and context-driven config sections#5426ardaerzin wants to merge 11 commits into
Conversation
When the model fires several tool calls that each need approval, the runner pauses on one and force-settles the rest with DEFERRED_NOT_EXECUTED. On cold replay messageTranscript rendered that as `[<tool> error: ...]`, which the model read as a denial and abandoned the call. Render it instead as the same "call it again with the same arguments" nudge an approved-but-unexecuted call gets, so the model re-issues it and its own gate surfaces.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesRunner approval lifecycle
Transcript replay and approval dock
Agent configuration workflows
Shared UI primitives
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/toolPermission.ts (1)
187-187: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPreserve reference equality when no permissions change.
Both permission-update functions recreate the configuration object graph even when the requested permission matches the current state. Returning the original
parametersobject preserves reference equality, preventing unnecessary React re-renders downstream.
web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/toolPermission.ts#L187-L187: returnparametersdirectly whenallowed === has.web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/toolPermission.ts#L224-L229: returnparametersdirectly when the tool's existing permission matches the requested one.♻️ Proposed fixes
For
withHarnessToolAllow:- if (allowed === has) return wrap({...template, harness: {...harness, permissions}}) + if (allowed === has) return parametersFor
withToolPermission:- const entry = isRecord(tools[i]) ? {...(tools[i] as Record<string, unknown>)} : {} - if (permission === undefined) delete entry.permission - else entry.permission = permission - const nextTools = tools.slice() - nextTools[i] = entry - return wrap({...template, tools: nextTools}) + const currentEntry = tools[i] as Record<string, unknown> + if (currentEntry.permission === permission) return parameters + + const entry = {...currentEntry} + if (permission === undefined) delete entry.permission + else entry.permission = permission + + const nextTools = tools.slice() + nextTools[i] = entry + return wrap({...template, tools: nextTools})web/oss/src/components/AgentChatSlice/hooks/useAlwaysAllowTool.tsx (1)
1-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCross-module hook placement.
This hook is now imported by
AlwaysAllowedNotice.tsxinPlayground/Components, outside its ownAgentChatSlicemodule. Per coding guidelines, module-specific code shared across modules should move to a root hooks location rather than staying under a feature module'shooks/folder.As per coding guidelines: "Keep module-specific code within its module; move functionality shared across modules to the appropriate root components, hooks, assets, or types location."
Source: Coding guidelines
web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx (1)
120-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the dead
variantpropvariantis unused here, and both branches render the same drawer body. If this is meant to cover both inline and drawer sections, drop the prop and tighten the JSDoc; otherwise add an inline-specific body.services/runner/src/engines/sandbox_agent/run-turn.ts (1)
478-481: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueSequential
respondPermissionper gate — consider whether this must be serialized.Each gate is answered with a sequential
awaitinside the loop. The comment explains the harness is blocked on all of them, so correctness isn't at risk, but ifrespondPermissioncalls are independent network/IPC round-trips, batching them (e.g.,Promise.all) would cut resume latency proportional to gate count. Low priority given current batch sizes are likely small.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 27d9cbec-d4b6-48bd-8acd-59b6208108b5
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (55)
hosting/docker-compose/ee/docker-compose.dev.ymlservices/runner/src/engines/sandbox_agent/acp-interactions.tsservices/runner/src/engines/sandbox_agent/environment-setup.tsservices/runner/src/engines/sandbox_agent/pause.tsservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/engines/sandbox_agent/runtime-contracts.tsservices/runner/src/engines/sandbox_agent/transcript.tsservices/runner/src/responder.tsservices/runner/src/server.tsservices/runner/src/tracing/otel.tsservices/runner/tests/unit/session-keepalive-approval.test.tsservices/runner/tests/unit/stream-events.test.tsservices/runner/tests/unit/transcript.test.tsweb/oss/src/components/AgentChatSlice/assets/transcriptToMessages.test.tsweb/oss/src/components/AgentChatSlice/assets/transcriptToMessages.tsweb/oss/src/components/AgentChatSlice/components/ApprovalDock.tsxweb/oss/src/components/AgentChatSlice/components/RevealCollapse.tsxweb/oss/src/components/AgentChatSlice/hooks/useAlwaysAllowTool.tsxweb/oss/src/components/Playground/Components/AgentCommitNotice.tsxweb/oss/src/components/Playground/Components/AlwaysAllowedNotice.tsxweb/oss/src/components/Playground/Components/MainLayout/index.tsxweb/oss/src/components/Playground/Components/ProviderKeyNotice.tsxweb/oss/tailwind.config.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentOperationsSections.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/ClaudePermissionsControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/PiAutoApproveControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/SandboxPermissionControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/SectionQuickAction.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ProviderCredentialsSection.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ProviderKeyField.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/SectionChangeBody.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/sectionChanges.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/index.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/toolPermission.tsweb/packages/agenta-entity-ui/src/DrillInView/components/PlaygroundConfigSection.tsxweb/packages/agenta-entity-ui/src/DrillInView/index.tsweb/packages/agenta-entity-ui/src/drawers/shared/ChangedPathsContext.tsxweb/packages/agenta-entity-ui/src/drawers/shared/FocusPathsContext.tsxweb/packages/agenta-entity-ui/src/drawers/shared/RailField.tsxweb/packages/agenta-entity-ui/src/drawers/shared/index.tsweb/packages/agenta-entity-ui/src/gatewayTrigger/drawers/TriggerDeliveriesDrawer.tsxweb/packages/agenta-entity-ui/tests/unit/formatCommitted.test.tsweb/packages/agenta-entity-ui/tests/unit/sectionChanges.test.tsweb/packages/agenta-entity-ui/tests/unit/toolPermission.test.tsweb/packages/agenta-shared/src/state/draftConfigChangeSignal.tsweb/packages/agenta-shared/src/state/index.tsweb/packages/agenta-shared/src/state/providerKeyAddedSignal.tsweb/packages/agenta-ui/package.jsonweb/packages/agenta-ui/src/components/HeightCollapse.tsxweb/packages/agenta-ui/src/components/presentational/index.tsweb/packages/agenta-ui/src/components/presentational/section/ConfigAccordionSection.tsxweb/packages/agenta-ui/src/components/presentational/section/index.tsxweb/packages/agenta-ui/src/components/presentational/section/useRecentFlag.ts
| private readonly pausedToolCallIds = new Set<string>(); | ||
| private resolvePause: (() => void) | undefined; | ||
| private eventDrain: Promise<void> = Promise.resolve(); | ||
| private pauseTimer: ReturnType<typeof setTimeout> | undefined; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Debounce timer is never cancelled on non-paused turn exit — can destroy a later turn's live session.
pauseTimer is only cleared inside pause()/schedulePause() themselves. If a turn that armed this timer exits via any other path (run-limit trip, thrown error, etc.) before the collect window elapses, the timer keeps running against this now-orphaned controller. Since env (in run-turn.ts) is a pooled/reused object whose parkedApprovals is reset at the top of the next runTurn call, the stale timer's eventual pause() call can find env.parkedApprovals.length === 0 and fall through to env.mcpAbort.abort() + env.sandbox.destroySession?.(env.session.id) — tearing down a completely different, currently-live turn's session.
Add a cancellation method that only clears the timer (no side effects), and invoke it from run-turn.ts's exit paths that did not pause (this requires hoisting pause out of the try block so finally can reach it, similar to otel/activeTurn).
🔒 Proposed fix
+ /** Cancel any armed collect-window timer without pausing. Call on every turn-exit path
+ * that did NOT pause, so a stale timer from THIS turn can never fire against whatever
+ * turn/session is live on the shared `env` afterwards. */
+ cancelSchedule(): void {
+ if (this.pauseTimer) {
+ clearTimeout(this.pauseTimer);
+ this.pauseTimer = undefined;
+ }
+ }
+
pause(): void {- let otel: ReturnType<typeof createSandboxAgentOtel> | undefined;
- let activeTurn: CurrentTurn | undefined;
+ let otel: ReturnType<typeof createSandboxAgentOtel> | undefined;
+ let activeTurn: CurrentTurn | undefined;
+ let pause: PendingApprovalPauseController | undefined;
...
- const pause = new PendingApprovalPauseController(() => {
+ pause = new PendingApprovalPauseController(() => { } finally {
runLimits.dispose();
+ if (pause && !pause.active) pause.cancelSchedule();
await activeTurn?.toolRelay?.stop().catch(() => {});
if (activeTurn) activeTurn.toolRelay = undefined;
}Also applies to: 28-58
| <Button | ||
| type="primary" | ||
| loading={responding} | ||
| loading={responding && resolveSource === "one"} | ||
| onClick={() => respond(true)} | ||
| > | ||
| {renderer?.approveLabel ?? "Approve"} | ||
| </Button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Disable Approve while the batch is resolving. It stays enabled during approveAll, so the primary action looks available even though respond() ignores it. Add disabled={responding} to match the other buttons.
| /** | ||
| * The committed value, as a reader recognises it rather than as it travels. | ||
| * | ||
| * The classifier stores scalars through `JSON.stringify` (`commitDiff/classify.ts`), which is right | ||
| * for comparing but wrong for showing: it renders an empty list as `[]` and a rule list as | ||
| * `["Terminal","Write"]` — wire syntax the control itself never displays. Unpack it back to what the | ||
| * field shows (one rule per line, matching the textarea), and name the empty cases instead of | ||
| * printing punctuation at someone. | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Condense verbose comments.
These comments explain design rationales and expected behavior in detail, which violates the coding guideline: "Keep in-code comments to at most one short line; use longer comments only for genuinely surprising constraints." As per coding guidelines, please keep them brief.
web/packages/agenta-entity-ui/src/drawers/shared/RailField.tsx#L47-L55: Shorten this JSDoc to a single summary line, as it describes standard presentation logic rather than a surprising constraint.web/packages/agenta-entity-ui/src/drawers/shared/RailField.tsx#L78-L85: Shorten this JSDoc to a single line.web/packages/agenta-entity-ui/src/drawers/shared/RailField.tsx#L126-L128: Condense this architectural note to a single line.web/packages/agenta-entity-ui/src/drawers/shared/RailField.tsx#L138-L147: Remove or condense these design rationale notes to a single short line.
📍 Affects 1 file
web/packages/agenta-entity-ui/src/drawers/shared/RailField.tsx#L47-L55(this comment)web/packages/agenta-entity-ui/src/drawers/shared/RailField.tsx#L78-L85web/packages/agenta-entity-ui/src/drawers/shared/RailField.tsx#L126-L128web/packages/agenta-entity-ui/src/drawers/shared/RailField.tsx#L138-L147
Source: Coding guidelines
| const sandboxChanged = useHasChangedUnder("sandbox") | ||
| const runnerChanged = useHasChangedUnder("runner") | ||
| const harnessChanged = useHasChangedUnder("harness") | ||
| const permissionsChanged = runnerChanged || harnessChanged | ||
| const changedIndicator = (changed: boolean) => | ||
| changed ? ({tone: "draft", tooltip: "Unsaved changes in this group."} as const) : undefined | ||
|
|
||
| // Section-scoped undo: restore every changed property in this group to its committed value. The | ||
| // per-row dot reverts ONE property; this is "undo the whole group". Null when nothing changed or | ||
| // the surface offers no revert, so the header keeps its normal actions. | ||
| const revertSandbox = useRevertUnder("sandbox") | ||
| const revertRunner = useRevertUnder("runner") | ||
| const revertHarness = useRevertUnder("harness") | ||
| const revertPermissions = useCallback(() => { | ||
| revertRunner?.() | ||
| revertHarness?.() | ||
| }, [revertRunner, revertHarness]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file and search for the change-tracking helpers.
ast-grep outline web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsx --view expanded || true
printf '\n--- matches ---\n'
rg -n "useHasChangedUnder|useRevertUnder|useHasFocusUnder|permissionsChanged|permissionsInFocus|harness\.kind|runner\.permissions|harness\.permissions" web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsxRepository: Agenta-AI/agenta
Length of output: 3543
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the provider/helpers that define how dot-paths are flattened.
rg -n "flatten|ChangedPathsProvider|useHasChangedUnder|useRevertUnder|useHasFocusUnder|ChangedPaths" web/packages/agenta-entity-ui/src web/packages/agenta-entity-ui -g '!**/dist/**' -g '!**/build/**'Repository: Agenta-AI/agenta
Length of output: 20490
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- ChangedPathsContext.tsx ---'
sed -n '1,180p' web/packages/agenta-entity-ui/src/drawers/shared/ChangedPathsContext.tsx
printf '\n%s\n' '--- FocusPathsContext.tsx ---'
sed -n '1,140p' web/packages/agenta-entity-ui/src/drawers/shared/FocusPathsContext.tsx
printf '\n%s\n' '--- useModelHarness.tsx around the relevant section ---'
sed -n '400,520p' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsx
printf '\n%s\n' '--- sectionChanges.ts around harness permissions revert docs ---'
sed -n '130,210p' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/sectionChanges.tsRepository: Agenta-AI/agenta
Length of output: 18545
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the structure of the Advanced section to see what "runner" and "harness" cover.
sed -n '520,1045p' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsxRepository: Agenta-AI/agenta
Length of output: 24824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- runner references in useModelHarness.tsx ---'
rg -n '\brunner\b|runner\.' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsx
printf '\n%s\n' '--- runner references in surrounding agentTemplate files ---'
rg -n '\brunner\b|runner\.' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate -g '!**/dist/**' -g '!**/build/**'
printf '\n%s\n' '--- sectionChanges helpers around path handling ---'
sed -n '1,140p' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/sectionChanges.tsRepository: Agenta-AI/agenta
Length of output: 9113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the runner shape and any other runner-owned paths in this hook.
sed -n '90,160p' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsx
printf '\n%s\n' '--- runner-owned paths in the file ---'
rg -n '\brunner(\.[A-Za-z0-9_]+)+' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsx
printf '\n%s\n' '--- harness-owned paths in the file ---'
rg -n '\bharness(\.[A-Za-z0-9_]+)+' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsxRepository: Agenta-AI/agenta
Length of output: 4563
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the commit-diff section mapping for the Advanced group and any docs/comments that specify
# whether the "runner" group is intentionally broad or only permission-specific.
rg -n 'runner / sandbox / harness|runner\.permissions|harness\.permissions|params"?: "advanced"|Advanced group|Permissions' \
web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate \
web/packages/agenta-entity-ui/src/drawers/shared \
web/packages/agenta-entity-ui/tests/unitRepository: Agenta-AI/agenta
Length of output: 11892
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# See whether any other runner properties exist in this area besides runner.permissions.*.
rg -n '\brunner\.[A-Za-z0-9_.]+' web/packages/agenta-entity-ui/src web/packages/agenta-entity-ui/tests/unit -g '!**/dist/**' -g '!**/build/**'Repository: Agenta-AI/agenta
Length of output: 2629
Scope the Permissions group to harness.permissions useHasChangedUnder("harness"), useRevertUnder("harness"), and useHasFocusUnder("harness") all catch harness.kind, so a harness selection change lights the Permissions header, gets reverted with it, and pulls the group into focus. Keep the runner policy on runner.permissions.default, but narrow the harness side to harness.permissions and use that same prefix for focus.
| const changeBodyFor = useCallback( | ||
| (key: PanelSectionKey) => { | ||
| const paths = sectionChanges.draft.pathsUnder() | ||
| // `model` (llm.* + harness.kind) and `params`/Advanced (everything else) are separate | ||
| // classifier buckets — see SECTION_ID_TO_PANEL_KEY. | ||
| const isModelPath = (p: string) => p.startsWith("llm.") || p === "harness.kind" | ||
| const sectionPaths = | ||
| key === "advanced" | ||
| ? paths.filter((p) => !isModelPath(p)) | ||
| : paths.filter(isModelPath) | ||
| if (!sectionPaths.length) return null | ||
| return ( | ||
| <SectionChangeBody | ||
| paths={sectionPaths} | ||
| onOpenDetails={() => openSectionDrawer(key as "model-harness" | "advanced")} | ||
| disabled={disabled} | ||
| changes={panelChangedPaths} | ||
| > | ||
| <ModelHarnessSectionBody | ||
| section={key as "model-harness" | "advanced"} | ||
| variant="inline" | ||
| schema={schema} | ||
| config={config} | ||
| onChange={onChange} | ||
| disabled={disabled} | ||
| withTooltip={withTooltip} | ||
| revisionId={revisionId} | ||
| savedHarnessValue={savedHarnessValue} | ||
| /> | ||
| </SectionChangeBody> | ||
| ) | ||
| }, | ||
| [ | ||
| sectionChanges.draft, | ||
| openSectionDrawer, | ||
| panelChangedPaths, | ||
| disabled, | ||
| schema, | ||
| config, | ||
| onChange, | ||
| withTooltip, | ||
| revisionId, | ||
| savedHarnessValue, | ||
| ], | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm which sections emit scalarChanges dot-paths (the leak trigger).
fd classify.ts -p commitDiff --exec rg -nP -C3 'scalarChanges|SectionId|instructions|agents_md' {}Repository: Agenta-AI/agenta
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file structure and locate relevant symbols.
ast-grep outline web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx --view expanded || true
echo
echo "---- nearby references ----"
rg -n "pathsUnder\(|sectionsByKey|scalarChanges|SECTION_ID_TO_PANEL_KEY|openSectionDrawer|changeBodyFor" \
web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx \
web/packages/agenta-entity-ui/src -SRepository: Agenta-AI/agenta
Length of output: 7046
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== sectionChanges.ts =="
sed -n '1,180p' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/sectionChanges.ts
echo
echo "== AgentTemplateControl.tsx slice =="
sed -n '480,560p' web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx
echo
echo "== pathsUnder usage =="
rg -n "pathsUnder\(" web/packages/agenta-entity-ui/src -SRepository: Agenta-AI/agenta
Length of output: 11500
Advanced is using the wrong change bucket. pathsUnder() returns every changed scalar path, so the advanced branch also picks up instructions/tools/mcp/skills edits and expands Advanced even when Advanced itself didn’t change. Scope this to the section’s own scalarChanges instead.
The harness delivers parallel tool-call permission requests concurrently (~0.5s apart), but the runner paused on the first and dropped the rest, so a multi-file approval took one round-trip per file. Collect-then-pause: emit every concurrent gate and debounce the pause (env AGENTA_RUNNER_APPROVAL_COLLECT_MS, default 800ms) so the whole batch parks together; the resume answers every gate on the live session before the blocked prompt continues. A gate that arrives after the window still falls back to the cold retry-nudge path.
One CSS-native, reduced-motion-proof collapse primitive (HeightCollapse) for everything that enters/leaves the composer region and config pane, plus the config-section header shimmer + draft/agent indicator tones on ConfigAccordionSection (and a useRecentFlag helper). Migrates RevealCollapse and AgentCommitNotice onto it so the motion language is consistent. Adds the motion dependency.
Reusable, dependency-free primitives for showing 'what changed' in a config surface: ChangedPathsContext (which dot-paths have uncommitted changes + a host-supplied revert), FocusPathsContext (narrow a surface to just the paths that matter — the same idea as rendering an input only when needed), and RailField opting into both via a path. sectionChanges maps the commit-diff classifier to the config panel's sections. Adds the draft-change and provider-key-added signal atoms the config pane consumes.
…nline provider key Drawer-backed config sections (Model & harness, Advanced) surface only what needs attention inline instead of routing to the drawer: a missing provider key shows the key field right there, and uncommitted changes show the changed controls (focus-filtered via FocusPaths/ChangedPaths), each row marked and revertable, with a link out to the full drawer. Wires the sandbox/harness permission controls and the provider-credentials/key fields with their dot-paths; adds the provider-key-added notice.
…olve Approval dock gains an 'Always allow <tool> for this agent' toggle that writes a per-tool permission (gateway/custom-function) or harness allow-rule into the draft config (toolPermission helpers), so the tool stops gating on resume and future runs; a contained notice offers Undo. Answering resolves the whole parked batch in one step — 'Approve all', or 'Approve' with the toggle on also clears that tool's sibling gates — instead of stepping through 1-of-N.
A paused turn persisted a terminal `done` and the resume run re-persisted the inbound user message. On reload that split the parked gate from the resume that settles it (gate stuck at 'Awaiting approval') and duplicated the user turn. Tag the paused turn's `done` with stopReason so replay can tell it isn't a real boundary, and skip re-persisting the user turn on a resume run (only a fresh user message is recorded, via tailIsFreshUserMessage).
transcriptToMessages keyed tool parts per-draft and closed a turn on any `done`, so a gate parked in one run and settled in the resume run landed in different drafts — the settling tool_result was dropped and the gate stuck at 'Awaiting approval'. Key tool parts transcript-wide (a resume's result settles the earlier gate in place) and don't treat a paused `done` as a turn boundary. Defense in depth alongside the runner-side persist fix — un-sticks the gate even for records written before it.
a7524a8 to
b1e05c5
Compare
|
Hi Arda, thank you for this work. The machinery ruling and every other decision here are explained, with their reasons, in a handoff document written for you: https://github.com/Agenta-AI/agenta/blob/plan/concurrent-approvals/docs/design/agent-workflows/projects/sessions-takeover/arda-handoff.md Your UX and configuration work has been ported, with your authorship preserved, onto a new branch, fe-enhance/approval-ui-onstack, which was cut from the top of the approvals stack. That branch is yours to continue on. This branch, approval-ui-onbig, stays on origin untouched as the archive of your version. Please read the handoff for the full context before picking things back up. |
2 participants
What & why
Agent-chat HITL approvals had three sharp edges, and the config panel made it hard to see or trust an uncommitted change. This branch fixes both, plus a couple of small UI/hosting follow-ups.
Two of the approval bugs only surfaced with parallel tool calls and on session reload, so they were easy to miss in a single-tool happy path:
doneand the resume re-persisted the user turn, so the reloaded chat dropped tool results or re-asked a prompt.Changes
Runner — parallel approvals & reload integrity
AGENTA_RUNNER_APPROVAL_COLLECT_MS, default 800ms), parks them, and resumes them as one batch — every sibling is answered.done, and resume no longer re-persists the user turn.Agent-chat FE — approval friction & restore
commit_revisionstays gated) and correct batch resolve — arming the toggle and clicking Approve clears the covered sibling gates in one step, instead of stepping1 of 3 → 1 of 2.Config sections — see & trust what changed
HeightCollapse,ConfigAccordionSection) underpin the above.remove-config-view-optionsremoved the tabs/cards view modes).Misc
fix(ui): use thesizeprop forTriggerDeliveriesDrawerwidth.chore(hosting): Claude Code harness config in the EE dev compose.Testing
tscclean; unit suite 1206/1206 (the lone flake is a pre-existing timing test inrelay-watch.test.ts, untouched here, green in isolation). New coverage for parked/resumed approvals and the deferred-sibling nudge.@agenta/entity-uitscclean;@agenta/osstscat baseline (no new errors);pnpm lint-fixclean. New package unit tests forsectionChanges,toolPermission,formatCommitted, and the transcript-restore adapter.Notes for reviewers