Skip to content

keep the bot's next step out of a failed step's error with an open parenthesis - #1059

Open
solquin wants to merge 1 commit into
CopilotKit:mainfrom
solquin:fix/not-carried-out-leak
Open

solquin wants to merge 1 commit into
CopilotKit:mainfrom
solquin:fix/not-carried-out-leak

Conversation

@solquin

@solquin solquin commented Nov 6, 2026

Copy link
Copy Markdown

what this changes

when an approved step runs out of tries, notCarriedOut sends Not done: this action could not be carried out (<reason>). Check whether it happened before asking the person again. outcomeForPerson cuts the second sentence off for the person by finding the first . outside parentheses. the reason is error.message as thrown (resume-sweep.ts), so the parentheses inside it are not ours to balance:

  • Unexpected token ( in JSON at position 4 leaves the depth at 1, no sentence end is found, and the person reads the whole text, bot's step included
  • a reason over 300 characters is cut by slice(0, 300), which can drop its own ), with the same result. long provider and database errors tend to carry parentheses

server/tests/approval-outcomes-for-person.test.ts guards exactly this leak, but its sample reason is The disk is full..

the fix:

  • outcomeForPerson strips the two known endings (ASK_AGAIN, CHECK_BEFORE_ASKING) by endsWith before the parenthesis scan. the builders use the same constants, so the strings cannot drift. the scan stays as the fallback for any other Not done: text
  • notCarriedOut cuts the reason by code point, so slice no longer leaves half an emoji (a lone surrogate) in the sentence

where it runs

  • new state that outlives a request? none. two pure functions in shared/
  • what happens on the second replica? the same: no state
  • anything serialised? no
  • anything fanned out to a browser? no. the app already calls outcomeForPerson, it now gets the shorter text
  • new listener, port, or schedule? no

boundary and audit

  • every acting call still goes through the gateway: untouched
  • new refusals and new failures each write a row: none added
  • nothing new is trusted from the client

changelog

  • a line in CHANGELOG.md under Unreleased

proof

  • bun test shared/approval-outcomes.test.ts: two new tests (an unclosed (, a 320-character reason, an emoji at the 300th character) fail on main and pass with the fix
  • bun test server/tests/approval-outcomes-for-person.test.ts app/tests/computer-activity-conversation.test.tsx: pass
  • bunx biome lint --error-on-warnings and bunx biome format on the changed files: clean. tsc --noEmit in shared/: clean

…renthesis

outcomeForPerson found the end of the first sentence by tracking parentheses. an error with a stray '(' or one cut at 300 characters before its ')' left the depth above zero, so the person read 'check whether it happened before asking the person again'. the known endings now come off first, and the reason is cut by code point.

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