Skip to content

fix: match answered questions to originating responsibility run - #95

Open
shanmukhdatta wants to merge 2 commits into
CopilotKit:mainfrom
shanmukhdatta:fix/route-answers-to-originating-responsibility-run
Open

shanmukhdatta wants to merge 2 commits into
CopilotKit:mainfrom
shanmukhdatta:fix/route-answers-to-originating-responsibility-run

Conversation

@shanmukhdatta

Copy link
Copy Markdown

Bug: the wrong background run can consume a person's answer

One responsibility can have multiple concurrent runs (each from a separate event). When a person answers a question from an older run, resumePersonQuestion currently picks listRuns()[0], the latest run. If it happens to be waiting for a different question, the reply is delivered as the newest run's continuation instead of the one that asked, leaving the original suspended. This is a data-integrity issue for asynchronous agent workflows.

Change

  • Find a suspended person_question run with a saved AG-UI continuation whose runId equals the saved question's run ID.
  • Preserve the existing fallback for older records lacking a continuation only when there is exactly one run, avoiding ambiguous cross-run matches.
  • Treat a failed resumeWaiting as a concurrent resumption; fall back to a separate answer instead of silently discarding the reply.
  • Do not bind detached replies to another waiting run's progress tool context.

Regression coverage

server/tests/approvals-question-resume-correlation.test.ts adds five tests: matching an older run when newer runs wait, refusing a mismatched waiting run, single-run legacy fallback, ambiguous legacy runs, and a concurrent-resume race. Existing single-run tests remain supported.

Validation

Tests are added but not executed here: this execution environment lacks Bun and the full repository dependencies. Please run from openbot/:
bun test server/tests/approvals-question-resume-correlation.test.ts
and the existing database integration test server/tests/approvals-question-resume.integration.test.ts.

Focused change: one service file plus targeted tests, no new dependencies.

@STiFLeR7 STiFLeR7 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correlation fix works when both runs are in the history returned by the store. I ran the five new tests and two existing question tests on Windows with Bun 1.4.3, and all seven passed.

One remaining case is an unanswered question with 50 newer runs. The lookup at question-resume.ts:71 uses a history query capped at 50. With the real store and an in-memory PostgreSQL database, 49 newer completed runs still let the answer requeue the original run. With 50, the same answer starts the fallback turn, the original run stays waiting, and no resume work item is queued.

Could the store look up the waiting question by responsibility ID and continuation run ID before applying the history limit? A regression with 50 newer completed runs would cover the boundary while keeping the run history endpoint capped.

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.

2 participants