test(api): wait for complete app session replies - #1097
Merged
Merged
Conversation
Contributor
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_4e72fa79-5dcd-4f44-acf4-8ecb7dda1c46) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
App sharing tests could send their next request after receiving only
api.head, while the previous request still held the session busy. CI then reportedE_UNAVAILABLEinstead of reaching the expected permission check.Wait for the matching
api.end, read the complete body, and check its byte count and truncation flag. Give independent access attempts their own sessions against the same real enrollment. The owner, viewer and last-owner assertions keep their original status and persisted-role checks. This changes tests only.A transport that pauses after
api.headreproduces the premature return before the fix. The new regression passes after it. Focused tests passed 30 runs, race tests passed 10 runs, the full API suite passed, andmake verifypassed on the standalone branch.This fixes the test failure seen in Core #1096 while preparing the next beta.
no-changesetapplies because no shipped code changes.Note
Low Risk
Test-only changes to the app session harness; no production API or enrollment behavior is modified.
Overview
Fixes flaky app-link session tests that could fire the next API call after only
api.head, while the session was still finishing the prior response (CI sawE_UNAVAILABLEinstead of expected 403/409 checks).appRig.callnow blocks on the matchingapi.end, assemblesapi.chunkbodies, and fails ifTruncatedorBytesdo not match the collected payload.TestAppRigCallWaitsForTheCompleteResponseuses a sender that pauses afterapi.headto lock in that behavior.Multi-step permission tests (owner escalation, guest roster, last-owner self-removal) create a fresh
newAppSessionper sub-case so sequential attempts do not share one busy session against the same enrollment. Assertions on status codes and persisted roles are unchanged; tests only.Reviewed by Cursor Bugbot for commit e865108. Bugbot is set up for automated code reviews on this repo. Configure here.