Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 20, 2025

The terminalUpdateStatuses array included "failed", which could cause failed operations to be incorrectly marked as succeeded if the explicit failure check was bypassed.

Change

Removed "failed" from terminalUpdateStatuses in memoryStoreUpdatePoller.ts:

// Before
const terminalUpdateStatuses: MemoryStoreUpdateStatus[] = ["completed", "superseded", "failed"];

// After
const terminalUpdateStatuses: MemoryStoreUpdateStatus[] = ["completed", "superseded"];

Logic Flow

The applyUpdateState function handles statuses in this order:

  1. Lines 111-118: Explicit "failed" check → sets state.status = "failed"
  2. Lines 121-128: terminalUpdateStatuses check → sets state.status = "succeeded"
  3. Line 131: Default → sets state.status = "running"

With "failed" in terminalUpdateStatuses, any logic error allowing "failed" to reach line 121 would incorrectly mark it as succeeded. This fix ensures only non-error terminal statuses ("completed", "superseded") are treated as success.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Only include non-error terminal statuses ('completed', 'superseded') in terminalUpdateStatuses to prevent failed operations from being incorrectly marked as succeeded.

Co-authored-by: glharper <[email protected]>
Copilot AI changed the title [WIP] Update LRO poller memory ops based on review feedback Fix terminalUpdateStatuses to exclude failed status Nov 20, 2025
Copilot AI requested a review from glharper November 20, 2025 21:57
Copilot finished work on behalf of glharper November 20, 2025 21:57
@glharper glharper marked this pull request as ready for review November 21, 2025 00:44
@glharper glharper merged commit 53e4c06 into glharper/proj-mem-store-update Nov 21, 2025
8 checks passed
@glharper glharper deleted the copilot/sub-pr-36703-another-one branch November 21, 2025 00:45
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