fix(core): publish domain updates after committed state is readable - #37987
IbrahimKhan12 wants to merge 1 commit into
Conversation
State.commit ran options.finalize before assigning state = next, and the catalog, integration, and reference finalizers publish their Event.Updated inside finalize. A client that reacts to the event and immediately refetches could therefore read the previous snapshot with no later invalidation, so a catalog/integration change was only observed once an unrelated later event triggered another refresh. Add a post-commit notify hook that runs after state = next and move each Event.Updated publish from finalize into notify. Finalize keeps the work that must complete before the state becomes visible (catalog provider removal, reference materialization); notify announces the rebuilt, readable snapshot. Because the event is emitted after the assignment, every public update event is a safe invalidation boundary regardless of fiber scheduling. Refs anomalyco#37422
|
The following comment was made by an LLM, it may be inaccurate: Found a related PR: PR #37480 - This is an alternative approach to the same issue (#37422). The current PR #37987 explicitly chooses not to use that approach—the description notes that #37480 assigns the snapshot before running These are related but distinct solutions to the same problem. |
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
1 participant
Issue for this PR
Fixes #37422 on
dev. #38983 fixes the same issue onv2.Type of change
What does this PR do?
State domains could publish update events from
finalizebefore the rebuilt state replaced the readable snapshot. A subscriber reacting immediately to a catalog, integration, or reference event could therefore refetch the previous value with no later event to invalidate it.This change adds a post-commit
notifyhook that runs only afterstate = next. Catalog, integration, and reference event publication moves to that hook, while asynchronous finalization still completes before the new snapshot becomes visible. An observed update event is therefore a safe boundary for refetching committed state.How did you verify your code works?
bun typecheckfrompackages/core: passes.Screenshots / recordings
N/A, this changes state/event ordering only.
Checklist