State restoration and continuity across devices - #5663
Conversation
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: def44c429d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Compared 12 screenshots: 12 matched. |
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9431117ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Cloudflare Preview
|
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
|
Compared 166 screenshots: 166 matched. Benchmark ResultsDetailed Performance Metrics
|
|
Compared 166 screenshots: 166 matched. Benchmark ResultsDetailed Performance Metrics
|
|
Compared 166 screenshots: 166 matched. |
|
Compared 166 screenshots: 166 matched. |
|
Compared 166 screenshots: 166 matched. Benchmark ResultsDetailed Performance Metrics
|
|
Compared 181 screenshots: 181 matched. |
|
Compared 160 screenshots: 160 matched. Benchmark Results
Detailed Performance Metrics
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85e4b48ba4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c5a39af91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecf68fc364
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 646d895af1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f2b72c7e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e121fdcef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1185b5b3f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5eaf1db562
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34598c5aa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f730882180
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33e4b0d530
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dca8d2d773
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 217be7b114
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ync opt-in routeStackChanged() returns early while a restore is being applied, and it has to: without that the rebuild checkpoints and republishes the state it is applying, and the two devices bounce it back and forth. But the restored form's show callback is application code and may navigate -- a screen that redirects to a newer one, an expired detail page sending the user to a list. Both notifications for that navigation land inside the window and are dropped, so the checkpoint recorded the routes that ARRIVED instead of the ones the user is on, and a process death before the next one restored the screen the application had redirected away from. The reconciliation goes AFTER commit(), and the ordering is the whole of it. commit() clears the pending flag as part of settling the arrival, so asking before it set a flag commit then wiped and the scheduled flush found nothing owed -- the fix looked right and did nothing, which the test caught. It is also after the lifecycle branch, because a callback that ends the session leaves the stack different from what was restored too, and checkpointing there writes for a session that has just ended -- an existing test caught that one. Separately, ios.continuity.sync=true is a DECLARATION and the build ignored it. The hint documents itself as "set true to say so explicitly", and the signing preflight already reads it that way -- it is how a project says it wants the store without that check having to read bytecode. The builder used it only as a veto, so a project that says so and whose usage the scan cannot see got neither the entitlement nor the define, while the preflight warned about a profile for a capability the build was never going to ask for. Both flags now, for the reason the scan's own comment gives: an entitlement without the define is a SyncedStore that reports itself unsupported on the device. Only an explicit true does it. Unset still means "the bytecode decides", which is what keeps an app that merely hands work to a nearby device from being given an iCloud entitlement its App ID may not carry. That half has no unit-test seam: the flag resolution lives inside build(), and the plist tests beside it drive static helpers rather than the build flow. I verified the placement instead -- both consumers, the entitlement block and injectToPlist(), run later in the same method -- and the plugin's 1938 tests still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d667acb232
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…y enable() The door my own parking change opened, and the symmetry I missed when I closed the same one for clear(). Callback.decide() parks an arrival that reaches the seam before the application has chosen -- a synced-store listener installs that seam without enabling continuity -- so by the time a logged-out app says "off" there can be a copy here as well as at the port. disable()'s early return drained only the port's, and enable() drains this slot on purpose, so the login restored a payload and routes that arrived before the application said it wanted none. disable() documents the opposite. The full path below already clears it as part of ending the session; the early return leaves before reaching that, which is the whole of the difference between the two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f1aaf2ca3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The previous commit put `parked = null` inside `if (!enabled)`, which is check-a-static-then-assign-a-static and trips the forbidden PMD rule NonThreadSafeSingleton. Both paths want it -- the full one clears the slot a few lines further down anyway -- so it is hoisted above the split rather than suppressed: one statement, said once, and the shape the rule objects to is gone. Recording the process failure rather than only the fix: I ran the quality gate and the commit in one chained command, so the push went out while the gate was reporting a violation. The gate's exit code has to be read before the commit, not beside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b44659b47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Hands an arrival to the framework, or holds it. Called on whatever thread the activity | ||
| /// arrived on; the framework marshals what it needs to. | ||
| private static boolean deliverToFramework(String activityType, String userInfoJson) { | ||
| ContinuityCallback c = callback; |
There was a problem hiding this comment.
When continuity is already enabled and iOS invokes nativeContinuation() on its platform thread, this unsynchronized read has no happens-before relationship with the EDT write in setCallback(), so it may still observe null. The arrival is then reported unclaimed and placed in pendingType/pendingJson, but no later callback installation is guaranteed to re-offer it, leaving a valid Handoff ignored indefinitely; publish the callback across threads or synchronously obtain it through EDT coordination.
Useful? React with 👍 / 👎.
ContinuityCallback lets a port call from any thread and says nothing about the map outliving the call, so a port that recycles one buffer per arrival -- or clears it once the call returns -- is within its rights. The off-EDT branch queued only the reference and had ALREADY told that port the activity was claimed, so the decision read whatever the bridge had put there since: a different state, or none at all. Read into an AppState before returning instead. fromMap() detaches it completely, which is why no copy helper is needed here -- setPayload deep-copies, the routes are rebuilt into a new list, and everything else a payload may hold is immutable -- so what is queued is the framework's own object. The claim gets honest with it, which is the part worth having: a document that yields no state is now declined rather than claimed, because nothing was taken. The test drives a bridge that empties and refills its map the instant the call returns, and checks the payload that reaches the provider is the one that was handed over. Without the fix nothing is delivered at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 285561b401
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
I gated the display half of this rollback last round and left the stack half unconditional, which made the two disagree. A show callback that navigates somewhere of its own and then throws -- or navigates and has a later listener throw -- has already changed both, so erasing the stack while leaving that screen up describes a place the user is not: back() then works on a history that does not include what is in front of them. Same rule as the ordinary navigations already use, and for the same reason: whatever ran later and changed the stack meant to, and it wins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3878e3654
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A state carrying a good payload and nothing storable beside it takes the payload-only return, and that return reached commit() before the filtered set was applied. persist() then threw on the original oversized route every time, so the arrival stayed parked, was re-applied on every retry, and held every relay publication behind it -- after the provider had already taken the payload. This is the second time this one reconciliation was applied to one path and not the other, so it stops being a statement placed near a path and becomes the statement immediately after the filter it belongs to. Every exit below now carries it, including ones nobody has thought of yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0c90120fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The slot holds one arrival and that is right: getRestorableState() answers with a state, and an application that has not dealt with the last one does not want a queue growing behind it. Replacing it is right too when the two come from the SAME device -- that is supersession, and the newer sequence is the one worth showing. Two different devices are not that. With automatic restoration off, both can be dispatched before the application calls restore(), and the second simply overwrote the first. That would be survivable if the first could come back, and it could not: its (origin, sequence) went into the in-memory map at admission, so a redelivery in the same run was refused as already seen. Recorded as handled, then dropped, and gone for the rest of the process. So the mark goes with it. Only the in-memory one -- durableSeen is written when a state COMPLETES and this one never did, so nothing durable claims it -- and only while it still names the dropped state, so a newer mark for that origin is left alone. Kept as one slot rather than a queue per origin, which is where the report left the choice open: the single slot is the documented shape of getRestorableState(), and the defect was that dropping was irreversible, not that dropping happens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b07b136a2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Last commit made a replaced offer recoverable and applied that at two of the FIVE places a state is put on offer. A listener that returns false for device A and then for device B before A is resolved goes through a third, so B replaced A with A's mark still recorded and A could never be offered again in that run -- the same defect, one call site over. All five go through placeOnOffer() now: the cold-launch hold before the event thread exists, the wait for a first window, the listener's hold, the pre-enable hold, and the deferred-restore hold. Each is a place where a second arrival can find one already waiting, which is why fixing them one at a time kept missing one. The test seam goes through it too, so a test that parks twice exercises what the framework actually does rather than a shortcut past it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a3177ddc2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
dispatch() already keeps an ARRIVAL whose restore failed, for the reason its own comment gives: a provider that throws is usually transient, so the state is worth holding for a retry. The application-driven restore() did not do the same for a state that came from STORAGE. So a cold start whose provider threw -- a dependency not up yet, which is the transient the whole failure branch exists for -- left the on-device checkpoint as the only copy, and the next navigation checkpointed the fallback screen over it. The draft the user was promised is gone at exactly the moment "restore, or else begin" is meant to protect it. A no-op when the state came from the slot, because placeOnOffer() returns immediately when asked to replace something with itself, so the arrival path is unchanged. It does hold relay publication until the application resolves the state, by retrying or acknowledging. That is the same hold a failed arrival already takes and for the same reason: what is on the relay is worth more than what this device would write over it while it cannot even load its own payload. Saying so here because it is a real cost, not a free win. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a25f362063
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ctory redirect win dispatch() and getRestorableState() both check maxAge and neither is the last word. The documented flow is that a listener returns false, puts a prompt in front of the user, and calls restore(state) when they accept -- and the deciding is exactly the time that passes. A state fresh when it was offered can be stale when it is taken, and an expired checkout or booking hold is precisely what maxAge exists to refuse. It is discarded the way getRestorableState() discards one, rather than left on offer to be handed back again: the slot is released and the publisher let go, because the hold existed for a state that will never be applied. Separately, a route FACTORY may redirect -- an expired detail page sending the user to a list -- and it does so before restoreStack() has installed anything, so the rebuild replaced both its stack entry and its screen. Its choice wins now, the same rule the rollback and the ordinary navigations already use. Returning false means Continuity treats the restore as showing nothing, and the reconciliation added earlier checkpoints the stack the factory left behind, so the two compose rather than needing a second mechanism. The factory test needed a second look: the first version had the factory answer null, which the empty-rebuild check already covers, so it passed with the new guard removed. It answers with a form now, which is what makes the rebuild non-empty and the guard the thing under test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65d0414c4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Both of these are the cost of the factory-redirect guard I added an hour ago, and both are things that guard should have done from the start. The stack comparison sat after the whole loop, so every later factory still constructed its screen and touched whatever the application keeps behind it -- an unavailable parent redirecting to a safe list while its child factories go on reading the record that is unavailable -- and all of it was then discarded in favour of the redirect. It is asked per iteration now, and once more after the loop because the last factory has no next iteration to be stopped by. That is the same pairing the session check beside it already uses, which is what it should have been copied from. And restoreStack() returning false read as "nothing happened", so a route-only arrival took the failure branch: parked, holding relay publication, and offered again after every launch to redirect again. The application DID handle it, by going somewhere else. Continuity compares the live stack with what it was before the rebuild, so a redirect from a factory OR from a show callback settles the arrival and checkpoints where the user actually ended up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd5abcbdda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd5abcbdda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nstalled
Replacing a same-origin offer is supersession, and supersession has a direction.
The comment there has always said the newer sequence is the one worth showing
and nothing checked: arrivals do not land in the order they were sent, so a
delayed sequence 10 landing after 11 replaced it and moved the user backward.
admit() has this check; the pre-enable path does not go through admit(), so the
states a synced-store listener's seam collects before enable() arrive here
unordered -- and both copies have already been claimed from the port, so the
newer one is simply gone.
And the lifecycle branch emptied the stack unconditionally. A callback that ends
the session and then goes somewhere -- clear() and then navigate("/login"), the
ordinary shape of a logout discovered mid-restore -- has already replaced it, so
emptying removed the login entry too: the display guard kept the login FORM, and
getCurrent() showed it while Navigation.getCurrent() was null and back() had
nothing. disable() during a restore did worse, destroying the pre-restore history
for something that is not a logout at all. Same rule as the two rollbacks in
Navigation: undo what this restore installed, leave what application code chose.
The tests in this class also clean the navigation stack now. Nothing resets
Navigation between them, so a test that leaves entries behind breaks the NEXT
test's fixture rather than its own assertions -- which is exactly what happened
when the unconditional clear stopped covering for it, and the full-suite run
caught it where the single-test runs could not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b124a514d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The offer slot holds one arrival, which is the right shape for getRestorableState() -- the application is asked about one thing at a time. It is the wrong shape for HOLDING, and those two jobs shared one field. Two devices can each offer work while automatic restoration is off, or while a listener defers both, and the second arrival simply overwrote the first. The previous fix forgot the displaced state's admission mark so that a redelivery could bring it back. That bet on a delivery which is not coming. The off-EDT callback claims what it queues -- it has to, the decision is made later on the event thread and the port is owed an answer now -- so a conforming bridge is entitled to drop its copy the moment it hands over. Nothing would ever deliver that state again. So displacement shelves rather than drops, one entry per origin, and getRestorableState() promotes the newest shelved arrival once the slot empties. The public shape is unchanged: still one state at a time, still the newest first. Every way an arrival ends had to reach the shelf as well as the slot, which is where the increments in this area have kept going wrong -- a fix applied to one exit of several. All of them, enumerated: a restore that commits, an acknowledge(), a tombstone from that origin, expiry, clear(), disable() on both its paths, and reset(). Relay publication is held for a shelved arrival too, and more obviously than for a parked one: the port has already been told the framework took it, so this process holds the only copy there is. Bounded at eight, oldest evicted. The shelf holds whole states, payloads included, and the device ids that key it come off the wire -- an unbounded one lets whatever is on the other end of the relay decide how much memory this process uses. The shelf takes only states that carry an origin. Keying an unidentified one under null looked harmless and was not: with two of them in a row, the same call that shelved the first looked a null origin straight back up, pulled it out again, and -- its sequence being the higher of the two -- handed it the slot and dropped the arrival that had just displaced it. The loss this mechanism exists to prevent, produced by the mechanism. Nothing the shelf does works without an origin, and a continuation always carries one; what arrives here without one is a state the application built and handed to restore(), which it still holds a reference to. Seven tests, each probed by mutating the guard it covers and confirming it fails: the two rewritten cross-origin tests now assert the state comes back with NOTHING redelivering it, and the new ones cover the tombstone, the publication hold, expiry with the application never asking (the case the slot's own expiry cannot reach, since asking is what discards a parked state), clear(), the bound, and the unidentified-state swap above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… other port has Unrelated to continuity, and carried here because it is what has been keeping this branch's javascript-screenshots leg red. It also fails on branches with no continuity code at all, so it is not this work's doing. The test encodes six frames and decodes them back, comparing whatever the platform codec returns -- lossy, and not bit-reproducible. Its own javadoc says each baseline ships a generous .tolerance file, and every comparing port does, at identical values. scripts/javascript/screenshots/ was the one that did not, so readTolerance fell back to the harness defaults in ProcessScreenshots -- maxChannelDelta=4, maxMismatchPercent=0.30 -- and ordinary decode noise failed the build. Measured on the failure rather than assumed: the worst single-channel delta anywhere was 20/255, 2.611% of pixels exceeded delta 4, and 0.000% exceeded delta 72. The regressions this test exists to catch -- frames blank, reordered, wrong colour -- move whole blocks across most of the channel range, so this does not stop catching them. The baseline PNG is untouched, so this is not a reseed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3de7bc2704
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Display.getInstance().callSeriallyAndWait(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| present[0] = Display.getInstance().getCurrent() != null; | ||
| } | ||
| }); |
There was a problem hiding this comment.
When the EDT remains busy for longer than 15 seconds during a cold launch, this non-timeout callSeriallyAndWait() blocks the waiter indefinitely, so the loop cannot recheck WINDOW_WAIT_MILLIS. If the EDT recovers minutes later, windowWaitFinished() can still dispatch the old continuation and unexpectedly replace whatever the user is then doing, despite the documented bounded window; use the timed overload with the remaining deadline.
Useful? React with 👍 / 👎.
| discardHeldArrival = true; | ||
| try { | ||
| installCallback(true); | ||
| } finally { | ||
| discardHeldArrival = false; |
There was a problem hiding this comment.
When a custom bridge re-offers its held continuation asynchronously from setCallback()—permitted because ContinuityBridge allows callbacks from any thread and does not require inline delivery—this finally clears discardHeldArrival before that callback runs. Because clear() leaves enabled unchanged, the delayed callback captures the new lifecycle and accepts the pre-clear state, or parks it for a later enable() when clearing occurred before enablement, allowing prior-account work to survive logout. Fresh evidence beyond the earlier clear-before-enable report is that the current fix relies on the re-offer completing synchronously even though the bridge contract does not impose that ordering.
Useful? React with 👍 / 👎.
|
Compared 217 screenshots: 217 matched. |
2 participants
What this is
A Codename One app that the operating system kills comes back to its first screen.
Lifecycle.stop()kept the currentFormin a plain field, so a suspend and resume looked right and a reclaimed process lost everything -- which on Android is the ordinary outcome of a few minutes in another app.com.codename1.continuitysaves what the user was doing and brings it back, and on Apple platforms offers that same work to the other devices the person is signed in to.The substrate was already here and unused:
com.codename1.routerkeeps a stack of deep-link paths, which is exactly a serializable, portable "where the user is".Navigation.restoreStackrebuilds it without animating through every screen on the way.Two packages, because they cost different things
@Routescreen stackStateRelayStateRelayStateRelayStateRelaycom.codename1.continuitybuys a native define and oneNSUserActivityTypesentry, and no entitlement.com.codename1.continuity.syncbuys the iCloud key-value store, whose entitlement has to be granted on the App ID -- handing that to an app that only wanted to pass work to the tablet in the user's other hand would fail its codesigning for a capability it never asked for. Same split, same reason, asusesSmartHome/usesHomeAccessoryData.Three decisions worth recording
stop()returns, so an app that saved there would pay for it on every suspend.start()is unchanged for every existing app, andrestore()is never called for anyone -- where restoration belongs in a launch is a decision only the app can make.StateRelayagainst the app's own endpoint, because deciding which saved states belong to the same person is the app's account system's question.The load-bearing detail
The iOS delegate matches continuity before intents. The intents block ends in a general branch that hands any remaining activity to Java and returns Java's answer, and
Intents.dispatchUserActivitycorrectly declines a type it never declared -- so an app using both would have had its own continuation asked about by the wrong framework, told no, and dropped.NSUserActivityTypesstays a single key for the same reason a second one is worse than none: iOS reads a duplicated key unpredictably. The two contributors meet inuserActivityTypesKey.Android needs nothing injected -- no permission, no manifest entry, no dependency. The bridge exists there for one job: flushing the checkpoint from
onSaveInstanceState, the last callback guaranteed before a background process is reclaimed. Both cross-device capabilities report themselves unsupported rather than being emulated, because an app told "yes" by a bridge that then dropped the state is worse off than one told "no", which can fall back to a relay and reach an iPhone as easily as another Android.Verification
The load-bearing one: a real Xcode build of the generated project produced one
NSUserActivityTypesarray carrying both the sample's three App Intent ids and...hellocodenameone.continuity.BUILD SUCCEEDEDfor iOS and watchOS, and a deliberate probe inside the#ifdef CN1_USE_CONTINUITYblock failed the build, so that check is not vacuous. Dead-code elimination left both native callbacks with real bodies.Also green locally: 46 core unit tests, 10 plist-merge tests, 7 preflight tests, 5 shipped-hooks tests; SpotBugs 0 findings across
core-unittests,android,ios,codenameone-maven-pluginandbuild-hint-catalog; and the cast-semantics, control-character, copyright, build-hint, snippet and prose gates.Paired change
The builder half is mirrored in BuildDaemon; that PR has to land with this one.
🤖 Generated with Claude Code