Skip to content

Conversation

@dandavison
Copy link
Contributor

@dandavison dandavison commented Dec 18, 2025

What changed?

In chasm.PollComponent, if the passed component ref has empty run ID, resolve this to the current run ID.

Why?

The previous code state was broken: it was possible to submit PollActivityExecution with no run ID, which led to Subscribe(key) being called where key lacked a run ID. But the notifications were being sent to a different key (one with the run ID) which led to the long-poll never receiving any notification and hence always timing out.

Existing chasm APIs ReadComponent and UpdateComponent both accept empty run ID and silently resolve it to current using getExecutionLease. It was a bug that PollComponent resolved to current using getExecutionLease, but then subscribed to notifications using a key that did not contain the resolved run ID.

At the external API level, in several of our non-CHASM user-facing gRPC APIs, empty run ID means "use latest at the time the request is processed" (e.g. GetWorkflowExecutionHistory). And we want these semantics for CHASM APIs such as PollActivityExecution. This change gives those semantics.

How did you test it?

  • built
  • added new unit test(s)
  • added new functional test(s)

Note

PollComponent now subscribes using the resolved execution key (including RunID) to avoid missed notifications; adds unit/functional tests including PollActivityExecution with empty RunID.

  • History/CHASM:
    • PollComponent: subscribe via notifier using resolved workflow key from executionLease.GetContext().GetWorkflowKey() (NamespaceID, BusinessID, RunID) instead of requestRef.ExecutionKey.
    • Minor loop simplification when rechecking predicate after notifications.
  • Tests:
    • service/history/chasm_engine_test.go:
      • Parameterized wait test for PollComponent with empty vs non-empty RunID; resolves empty RunID via GetCurrentExecution and verifies notifications use resolved ExecutionKey.
      • Uses context timeouts for long-poll and validates returned ref contains actual RunID.
    • tests/standalone_activity_test.go:
      • Add TestPollActivityExecution_EmptyRunID to verify empty RunId resolves to current and returns expected outcome.

Written by Cursor Bugbot for commit 9cfa128. This will update automatically on new commits. Configure here.

@dandavison dandavison requested review from a team as code owners December 18, 2025 01:42
return ref, nil
ref, err = checkPredicateOrSubscribe()
if err != nil || ref != nil {
return ref, err
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just some minor code cleanup, irrelevant to the PR.

@dandavison dandavison force-pushed the saa-poll-activity-execution-bug-fix branch from 9f6a22b to a8952b1 Compare December 18, 2025 16:45
@dandavison dandavison changed the title Require Run ID in PollActivityExecution and PollComponent CHASM: bug fix: handle empty run ID in PollComponent Dec 18, 2025
@dandavison dandavison force-pushed the saa-poll-activity-execution-bug-fix branch from 024499c to 9cfa128 Compare December 18, 2025 18:17
@dandavison dandavison merged commit 321c003 into standalone-activity Dec 18, 2025
55 of 58 checks passed
@dandavison dandavison deleted the saa-poll-activity-execution-bug-fix branch December 18, 2025 18:52
dandavison added a commit that referenced this pull request Dec 19, 2025
## What changed?
In `chasm.PollComponent`, if the passed component ref has empty run ID,
resolve this to the current run ID.

## Why?
The previous code state was broken: it was possible to submit
`PollActivityExecution` with no run ID, which led to `Subscribe(key)`
being called where key lacked a run ID. But the notifications were being
sent to a different key (one with the run ID) which led to the long-poll
never receiving any notification and hence always timing out.

Existing chasm APIs `ReadComponent` and `UpdateComponent` both accept
empty run ID and silently resolve it to current using
`getExecutionLease`. It was a bug that `PollComponent` resolved to
current using `getExecutionLease`, but then subscribed to notifications
using a key that did not contain the resolved run ID.

At the external API level, in several of our non-CHASM user-facing gRPC
APIs, empty run ID means "use latest at the time the request is
processed" (e.g. `GetWorkflowExecutionHistory`). And we want these
semantics for CHASM APIs such as `PollActivityExecution`. This change
gives those semantics.


## How did you test it?
- [x] built
- [x] added new unit test(s)
- [x] added new functional test(s)
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.

4 participants