Repository navigation
fix(core): honest close-path state when eviction cancels pending questions - #52600
Open
thiago-carneiro wants to merge 2 commits into
Open
thiago-carneiro wants to merge 2 commits into
thiago-carneiro wants to merge 2 commits into
Conversation
Contributor
|
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. |
Contributor
|
The following comment was made by an LLM, it may be inaccurate: |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #52599
Type of change
What does this PR do?
When a location closes and cancels pending questions, the cancellation carries no reason: the form state is a bare
{ status: "cancelled" }, and the tool failure reported to the model never names the pending question. The branch builds on #52587, which fixed the tool-part message; this PR covers the close path.I pass an eviction cause through the close path: pending forms cancelled by an eviction close now carry
cause: "evicted"and a message naming the pending question. The cancelled form state now has an optionalcausefield, and I regenerated the API clients. In the tool failure reported back to the model, a question cancelled while waiting now names the pending question instead of using the generic close message; user dismissals keep the same wording, now also naming the question. Non-eviction closes (reload, debug) still cancel without a cause — their form state stays bare.Implementation note: with no borrowers the entry's scope closes synchronously inside the invalidate call, so the cause-carrying cancel runs in a narrow hook just before it — non-eviction invalidations are untouched.
I did not change the eviction policy or the 60-minute deadline; those are #51343's questions. Restoring pending forms across reconnects (#44471) stays out of scope.
How did you verify your code works?
bun run testinpackages/core: full suite passes (5628 tests, 0 failing), including 7 new tests covering the eviction-cancelled form state and the tool-failure messagebun turbo typecheckandbunx oxlintpassbun run checkpassesThe new tests cover the behavior. To see it manually, leave a question form pending in an idle session until eviction — the cancelled form then names the eviction and the pending question.
Screenshots / recordings
Not a UI change.
Checklist