Skip to content

fix(swift-ios): ignore stale device reads after revocation - #10748

Closed
saphid wants to merge 1 commit into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-device-revocation-20260908
Closed

saphid wants to merge 1 commit into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-device-revocation-20260908

Conversation

@saphid

@saphid saphid commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

A delayed device-session reload can restore a revoked session or overwrite newer action feedback. Give reload and revocation operations one generation owner and reject stale completions.

Verification: all current-head GitHub checks pass, including contract fixtures and native tests. Skipped checks are not claimed as executed proof.

Delivery: direct.

Base: Theo’s t3code/rebuild-mobile-app-swift, 93ca26695eb1c6ac6ef2d44bb76fe371ac0bb385.

The claimed behavior is covered at the native protocol, persistence, or client boundary. No visible layout change or measured phone responsiveness improvement is claimed.

Independent cross-provider review was unavailable: claude auth status returned exit 1 with loggedIn: false. No Claude review or maintainer approval is claimed.

Model and harness: GPT-6 / Codex.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 8, 2026
@saphid
saphid marked this pull request as ready for review September 8, 2026 21:44
@cursor

cursor Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a focused concurrency fix that prevents stale device-session reads from restoring revoked entries or overwriting newer revocation feedback. Because it changes production device access and session-revocation behavior, human review is warranted.

You can add or adjust custom eligibility rules. Learn more.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 8, 2026
@saphid

saphid commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@t3dotgg Could you review this SwiftUI reliability/performance fix? It will reject delayed device reloads after a newer revocation operation. It does not change the visible interface. This can be reviewed independently against your SwiftUI branch.

A device list reload that finishes after a revoke could put the removed
device back on screen. Each reload and revoke now takes a generation, and
only the newest operation may write results or clear its spinner.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@saphid
saphid force-pushed the pr/swiftui-device-revocation-20260908 branch from 13150e9 to 1bf7c9a Compare September 11, 2026 07:33
@macroscopeapp
macroscopeapp Bot dismissed their stale review September 11, 2026 07:34

Dismissing prior approval to re-evaluate 1bf7c9a

@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Sep 11, 2026

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

The supplied verification does not exercise the changed DevicesView race. Native CI passes, but its device tests cover sorting, metadata and loading; the PR gives no reproduction or observed result for a delayed reload completing after revocation. Closing under the verification rule. Add a focused test or manual check that holds a reload open, revokes a device, then releases the stale result and verifies that the device stays removed and newer feedback survives. Request reconsideration with that evidence.

@saphid

saphid commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Requesting reconsideration with the verification asked for in the closure (verification).

GitHub does not let me reopen this PR, so the corrected change is in the replacement #14683 (head f633e6790f, replayed onto the rewritten t3code/rebuild-mobile-app-swift at cf7902896b). The change is limited to DevicesView.swift and DeviceManagementTests.swift. The description explains the defect, why it qualifies as a small obvious-bug fix, and how the parts relate to that one problem.

With [current, laptop] loaded, the new tests hold a reload open with a test double, revoke laptop, then release the stale result. They assert that the device stays removed, that a newer revocation error survives, and that a stale reload error does not replace a successful revocation. A fourth test holds the first revocation open and checks that an overlapping second removal is refused, so the first one's failure is not cleared.

On the iOS Simulator (iPhone 17, iOS 26.5), -only-testing:T3CodeTests/DeviceManagementTests passes 9/9 with the fix. The tested revision had the same diff on the previous base. For the comparison, I used a local overlay with the model's operations replaced by the previous DevicesView code and the tests unchanged; it is not an upstream revision. There it fails 4/9: the revoked device reappears, newer feedback is lost or replaced, and the overlapping removal is sent. Output and overlay diff: https://gist.github.com/saphid/e41be48dd5edd953b80276a74d904fb1

Not checked: an end-to-end run against a real server, and media of the remove actions being disabled during a removal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants