revert(flows): restore require_approval default to true (merge LAST, after approval surfacing) (#4672)

This commit is contained in:
Cyrus Gray
2026-07-08 02:52:02 +05:30
committed by GitHub
parent dfcf71a7fa
commit 916836567c
7 changed files with 23 additions and 24 deletions
@@ -35,15 +35,15 @@ describe('WorkflowPromptBar', () => {
// The created flow is the standard blank graph (single manual trigger).
expect(graph.nodes).toHaveLength(1);
expect(graph.nodes[0].kind).toBe('trigger');
// Prompt-authored flows default to NOT requiring approval, so a
// Run-button/scheduled run doesn't deadlock on an unsurfaceable approval.
expect(requireApproval).toBe(false);
// Parity with the proposal-card path: prompt-authored flows must default
// to requiring approval, matching WorkflowProposalCard's default.
expect(requireApproval).toBe(true);
expect(navigateMock).toHaveBeenCalledWith('/flows/flow-1', {
state: { copilotBuild: { description: 'digest my Slack every morning' } },
});
});
it('defaults requireApproval to false so runs do not deadlock on an unsurfaceable approval', async () => {
it('defaults requireApproval to true so outbound side-effects need explicit approval', async () => {
render(<WorkflowPromptBar />);
fireEvent.change(screen.getByTestId('workflow-prompt-input'), {
target: { value: 'auto-reply to every gmail thread' },
@@ -51,9 +51,9 @@ describe('WorkflowPromptBar', () => {
fireEvent.click(screen.getByTestId('workflow-prompt-submit'));
await waitFor(() => expect(createFlowMock).toHaveBeenCalledTimes(1));
// Third positional arg must be `false` — prompt-authored flows default to
// not requiring approval so a Run-button/scheduled run can execute.
expect(createFlowMock.mock.calls[0][2]).toBe(false);
// Third positional arg must be `true` — flows_create's server default is
// `false`, so omitting it would silently disarm the approval gate.
expect(createFlowMock.mock.calls[0][2]).toBe(true);
});
it('submits on Enter (Shift+Enter reserved for newlines)', async () => {
@@ -44,14 +44,13 @@ export default function WorkflowPromptBar({ variant = 'compact', autoFocus = fal
const name = deriveWorkflowName(trimmed, t('flows.page.newWorkflow'));
log('submit: creating flow name=%s', name);
try {
// Prompt-authored flows default to NOT requiring approval — a
// Run-button/scheduled run has no chat context, so a parked approval
// card can't surface and the run would deadlock. The user can still
// turn approval on per-flow afterwards.
// Safe default: prompt-authored flows require approval so outbound
// Slack/Gmail/HTTP/code nodes cannot fire unattended. Omitting this
// arg would fall back to the server default of `false`.
const flow = await createFlow(
name,
createBlankWorkflowGraph(name, t('flows.nodeKind.trigger')),
false
true
);
log('submit: created id=%s — opening canvas with build seed', flow.id);
navigate(`/flows/${flow.id}`, { state: { copilotBuild: { description: trimmed } } });
+4 -4
View File
@@ -324,10 +324,10 @@ function parseWorkflowProposal(output: string): WorkflowProposal | null {
return {
name: obj.name,
graph: obj.graph,
// require_approval defaults to `false` — a proposal only requires approval
// if the builder explicitly set it so treat anything other than an
// explicit `true` as `false`, in lockstep with the server default.
requireApproval: obj.require_approval === true,
// The Rust tool defaults `require_approval` to `true` when the caller
// omits it, so treat anything other than an explicit `false` as `true`
// here too — keeps the client's fallback in lockstep with the server's.
requireApproval: obj.require_approval !== false,
summary: { trigger: typeof summary.trigger === 'string' ? summary.trigger : '', steps },
};
}
+2 -2
View File
@@ -114,7 +114,7 @@ impl Tool for ReviseWorkflowTool {
},
"require_approval": {
"type": "boolean",
"description": "Force a human-approval gate on every outbound action once saved. Defaults to false; set true only when the user explicitly asks for an approval step."
"description": "Force a human-approval gate on every outbound action once saved. Defaults to true for agent-proposed flows."
}
},
"required": ["name", "graph"]
@@ -146,7 +146,7 @@ impl Tool for ReviseWorkflowTool {
let require_approval = args
.get("require_approval")
.and_then(Value::as_bool)
.unwrap_or(false);
.unwrap_or(true);
tracing::debug!(
target: "flows",
+2 -2
View File
@@ -58,7 +58,7 @@ async fn revise_workflow_validates_and_returns_revision_proposal() {
}
#[tokio::test]
async fn revise_workflow_omitted_require_approval_defaults_false() {
async fn revise_workflow_omitted_require_approval_defaults_true() {
let tmp = TempDir::new().unwrap();
let tool = ReviseWorkflowTool::new(test_config(&tmp));
@@ -69,7 +69,7 @@ async fn revise_workflow_omitted_require_approval_defaults_false() {
assert!(!result.is_error, "{}", result.output());
let parsed: Value = serde_json::from_str(&result.output()).unwrap();
assert_eq!(parsed["require_approval"], false);
assert_eq!(parsed["require_approval"], true);
}
#[tokio::test]
+2 -2
View File
@@ -120,7 +120,7 @@ impl Tool for ProposeWorkflowTool {
},
"require_approval": {
"type": "boolean",
"description": "Force a human-approval gate on every outbound tool/HTTP action this flow takes once saved. Defaults to false; set true only when the user explicitly asks for an approval step."
"description": "Force a human-approval gate on every outbound tool/HTTP action this flow takes once saved. Defaults to true for agent-proposed flows."
}
},
"required": ["name", "graph"]
@@ -152,7 +152,7 @@ impl Tool for ProposeWorkflowTool {
let require_approval = args
.get("require_approval")
.and_then(Value::as_bool)
.unwrap_or(false);
.unwrap_or(true);
tracing::debug!(
target: "flows",
+2 -2
View File
@@ -112,7 +112,7 @@ async fn missing_graph_is_an_error() {
}
#[tokio::test]
async fn omitted_require_approval_defaults_false_in_result() {
async fn omitted_require_approval_defaults_true_in_result() {
let tmp = TempDir::new().unwrap();
let tool = ProposeWorkflowTool::new(test_config(&tmp));
@@ -122,7 +122,7 @@ async fn omitted_require_approval_defaults_false_in_result() {
.unwrap();
let parsed: Value = serde_json::from_str(&result.output()).unwrap();
assert_eq!(parsed["require_approval"], false);
assert_eq!(parsed["require_approval"], true);
}
#[tokio::test]