[TINGyu123644]

Fixes #45238

Summary

change_segment_status in the Console segment-status API committed the enabled/disabled_at/disabled_by fields before publishing the enable/disable Celery task. When the broker is unavailable, the publish raises after the commit, so the persisted state already matches the request. A retry is then skipped as a no-op (segment.enabled == (action == "enable")), the API returns success, and the segment is never restored to (or removed from) the index - the exact sequence reported in the issue.

This orders the operation the other way: schedule the index side effect first, persist the desired state afterwards.

• If the task cannot be published (e.g. broker down), nothing is committed, so an explicit retry re-dispatches.
• The enable/disable tasks select segments by id (status == COMPLETED) and perform idempotent index loads/removals, so they do not depend on the committed enabled value; dispatch-before-commit is therefore safe.
• If the DB commit fails after a successful publish, the in-flight task already did the work and the retry re-dispatches (idempotent), so both failure modes converge on retry.

Testing

• Added two regression tests to api/tests/unit_tests/services/knowledge/segments/test_application.py:
  • test_status_change_does_not_persist_state_when_dispatch_fails - dispatch raises, nothing is persisted (store.set_segments never called), and the error is translated to SegmentStatusUpdateError.
  • test_status_change_dispatches_index_change_before_persisting_state - ordering assertion (dispatch before persist) plus persisted values check.
• Both tests fail on unpatched code (RED: persist runs before dispatch / state committed on dispatch failure) and pass with the fix (GREEN); the 22 pre-existing tests in the file keep passing (24 passed total, Python 3.13, pytest).
• ruff check and ruff format --check (repo .ruff.toml: line-length 120) are clean for both touched files.

AI disclosure

This change was produced with AI assistance (Claude, via an automated contribution workflow) and reviewed by the account owner.

[GYJ99]

I see the intended fix for the broker-publication failure case, but moving _index.change_status() before _store.set_segments() appears to introduce the reverse partial-failure window: if the task is published successfully and the DB write then fails (e.g. transient DB error/deadlock), Celery may already remove or add the index while the persisted enabled state remains unchanged. For example, a failed disable request could leave a segment marked enabled in the DB but absent from the retrieval index. The new tests cover dispatch failure and call ordering, but not persistence failure after a successful dispatch.

Could we cover that failure path or avoid the ordering change? The explicit retry/reconciliation approach in #45239 seems worth comparing, since it keeps the existing persist-then-dispatch sequence and repairs retries even when status already matches. This is a code-path concern, not a claim of having reproduced the race in a live Celery environment.