Skip to content

fix(approvals): stop empty fields tripping the safety rules - #930

Open
sx4im wants to merge 3 commits into
CopilotKit:mainfrom
sx4im:fix-890-empty-fields
Open

sx4im wants to merge 3 commits into
CopilotKit:mainfrom
sx4im:fix-890-empty-fields

Conversation

@sx4im

@sx4im sx4im commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

What this changes

Finishes the false-positive half of #890. Writing only a note on a Shopify order was still handed to the person as money-moving: the Bot passes billing_address left empty, and the safety matcher fired on the field name even though nothing was in it.

flatten in server/src/approvals/policy.ts no longer counts a field name when the field holds nothing (null, undefined, empty string, object or array). The rules now match the arguments actually sent, not names of fields left empty. A billing_address with something in it still matches exactly as before.

Where it runs

Pure function, no state involved.

  • New state that outlives a request? None.
  • What happens on the second replica? Identical outcome, no shared state involved.
  • Anything serialised? Nothing new.
  • Anything fanned out to a browser? No.
  • New listener, port, or schedule? No.

Boundary and audit

  • Every acting call still goes through the gateway: resolve, decide, audit, then act.
  • New refusals and new failures each write a row. Fewer false hand-offs; no new refusal paths.
  • Nothing new is trusted from the client that the server can resolve itself.

Changelog

  • A line in CHANGELOG.md under Unreleased.

Proof

bun test server/tests/approvals-policy.test.ts: 33 pass, 0 fail. The new assertions (order note with billing_address: null and with billing_address: {}) fail before the fix (came back payments) and pass after. tsc --noEmit clean on server.

Refs #890

A field the Bot left empty says nothing about the action. Writing only
a note on a Shopify order was handed to the person as money-moving
because the action's schema has a billing_address field that was never
filled in; field names now only count when they hold something.

Refs CopilotKit#890
sx4im added 2 commits October 31, 2026 22:51
Resolve CHANGELOG.md Unreleased entry collision: keep upstream's
three entries and the PR's empty-fields entry (mechanical merge).
The merge of main (20de77d) rebuilt the tree from upstream main
plus the merged CHANGELOG.md, silently dropping this PR's
server/src/approvals/policy.ts and server/tests/approvals-policy.test.ts
changes. Restore both files from the pre-merge head.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant