fix: human tool approval requests omit policy reasons - #19658
Conversation
Co-authored-by: ladanjohari <13539962+ladanjohari@users.noreply.github.com>
Bugfix reviewOutcome: approved Reproduction replayStatus: no-longer-reproduces The exact original reproduction completed successfully and the original bug signal did not appear. Fixes issueStatus: fully-addresses The OPA normalizer now preserves requires-approval reasons, and the reason propagates through generateText, streamText, response messages, UI chunks, UI state, validation, and model-message conversion. Side effectsRisk: low Existing approval behavior is unchanged when no reason is supplied; the only observable addition is the documented optional reason on human approval requests. PerformanceRisk: low The change only carries and serializes one optional string per applicable approval request, with no new loops, retained collections, network operations, or expensive computation. Backwards compatibilityRisk: low Existing stored messages remain valid, while newly created pending approval messages may contain an additional optional reason field that validation and round-trip conversion now support without migration. Breaking changesRisk: none Public types and accepted message schemas are broadened additively; no APIs, exports, accepted inputs, defaults, errors, or persisted formats are removed or narrowed. ArchitectureRisk: low Ownership remains sound: policy-opa normalizes the policy decision, provider-utils owns the shared model-message request type, and ai owns generation and UI propagation. No cross-package src imports or dependency-direction violations were introduced. Change scopeStatus: minimal Each production hunk participates in preserving, typing, validating, serializing, or testing the reason across an existing approval path, and the required patch changesets cover all affected published packages. SecurityRisk: low The change exposes policy-provided text only through the already documented human-approval channel and does not alter execution authorization, signatures, URL handling, parsing, or trust decisions. TestingStatus: appropriate Regression tests cover OPA normalization, generateText, streamText, response and UI serialization, UI processing, stored-message validation, and model-message conversion in both Node and Edge configurations. VerificationInspected all 23 changed files against origin/main, public exports and surrounding approval flows, and package boundaries. The focused AI tests passed 943 tests in both Node and Edge configurations; policy-opa tests passed, affected package type checks passed, formatting and lint checks passed, and the diff had no whitespace errors. Relevant Documentation |
ai-sdk-factory
Bot
changed the title
|
Addressed PR comment 5427242123.
No change was made because the review approved the existing propagation, compatibility, architecture, and coverage. I reran the focused Node and Edge tests, affected and full type checks, formatting and lint checks, and the immutable reproduction; all passed and the replay remains |
Bugfix reviewOutcome: approved Reproduction replayStatus: no-longer-reproduces The exact original reproduction completed successfully and the original bug signal did not appear. Fixes issueStatus: fully-addresses The OPA normalizer preserves requires-approval reasons, and the reason reaches core outputs, response messages, UI chunks, pending UI state, validation, and model-message round trips. Side effectsRisk: low Requests without a reason retain their existing shapes and behavior; only requests supplied with a reason gain the documented optional field. PerformanceRisk: low The change carries one optional string through existing objects and serialization paths without adding loops, network operations, retained collections, or expensive computation. Backwards compatibilityRisk: low Existing stored messages remain valid without migration, while newly stored pending approvals may add an optional reason that the updated validators and converters accept. Breaking changesRisk: none Public status, request, output, UI, and message types are widened additively; no exports, accepted inputs, defaults, errors, configuration, or existing persisted fields are removed or narrowed. ArchitectureRisk: low Ownership remains consistent: policy-opa normalizes policy decisions, provider-utils owns the shared model-message request type, and ai owns generation and UI propagation. No cross-package src imports or dependency-direction violations were introduced. Change scopeStatus: minimal All 23 changed files either propagate, type, validate, test, or release-note the optional reason across the existing approval pipeline. SecurityRisk: low The change exposes policy-provided text only through the documented human-approval channel and does not alter authorization decisions, approval signatures, parsing, URL handling, or tool execution. TestingStatus: appropriate Regression tests cover normalization, generateText, streamText, response and UI serialization, UI processing, stored-message validation, and model-message conversion in Node and Edge runtimes. VerificationInspected the complete diff and surrounding public APIs and package boundaries. Focused Node and Edge runs each passed 943 AI tests; policy normalization passed 13 tests in each runtime. Affected package and workspace type checks, formatting, linting, and diff whitespace checks passed. Relevant Documentation |
|
🚀 Published in:
|
2 participants
Background
OPA documentation promises that
requires-approvalreasons reach human approvers, but published generation and UI approval requests omitted them.Root Cause
The OPA normalizer discarded
requires-approvalreasons, while user-approval status and request types could not carry them; the original reproduction confirmed bothgenerateTextandstreamTextlost the reason.Summary
generateTextandstreamTextoutputs, response messages, UI serialization, processing, validation, and model-message conversion.approval.requestReason, separate from the approver's responsereason, so both survive approval state transitions and persisted-message round trips.ai,@ai-sdk/policy-opa, and@ai-sdk/provider-utils.Compatibility
Existing approval inputs and stored messages remain valid. Requests without a reason keep their previous runtime shape. The UI stream adds only an optional
reasonfield, while UI messages add the optionalapproval.requestReasonfield and retain the existing responseapproval.reasonsemantics.Testing
Regression coverage verifies OPA normalization and integration,
generateText,streamText, response-message conversion, UI chunk conversion and processing, request/response reason lifecycle preservation, stored-message validation, public types, and model-message conversion.End-to-end Validation
replay_original_reproduction: bothgenerateTextandstreamTexthuman approval requests contained the policy reason; classifiedno-longer-reproduces.Related Issues
Fixes #19654
Closes #19655