Repository navigation
fix: recover connections during wallet restore - #1139
ben-kaufman wants to merge 12 commits into
Conversation
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: I found no actionable defects.
QA for 9481b50: iPhone 17 Pro simulator, iOS 26.5, unfunded regtest wallet, authenticated staging VSS and staging Electrum/RGS. Device cases used the Debug fault controls below.
Test 1: J1, required restore connection recovery ✅ Independent corrections survived failed probing, retry and restart; required recovery continued to gate wallet access.
Electrum and RGS saved independently without starting restoration. A failed RGS probe preserved the acknowledged revision. The complete pass verified selective replay over the old SETTINGS backup, including restoration of unrelated preferences, in the device log. Separate WALLET and USDT failures kept access restricted. After both cleared, Home opened while optional profile initialization remained held.
J1-focused-2x.2x.mp4
Focused repeat at 2×; endpoint values, revision retention and restore-marker transitions were also checked in the app log.
Test 2: manual 1, cancellation and wipe ✅ An admitted retry completed after Back; wipe drained a held probe before a replacement wallet became available.
The full pass held a required import, left the editor and released it: restoration and node startup completed. The subsequent held RGS probe was canceled by leaving the editor, then a wipe waited until release. A newly generated wallet reached Home with default endpoints and no stale correction. The focused repeat below covers probe draining and replacement; the retry-after-Back result was checked in the original recording and app log.
manual-1-focused-2x.2x.mp4
Probe/wipe repeat at 2×; drain order and final settings were checked in the app log.
Test 3: manual 2, local persistence failures ✅ Save and Retry reported failures while preserving required recovery state.
Denied correction-record writes left preferences unchanged. Denied preference promotion retained the durable correction. Retry remained gated. An unreadable correction record showed an error and disabled saving; direct state inspection confirmed the same correction and required WALLET marker remained stored.
manual-2-2x.2x.mp4
Storage-fault cases at 2×; durable-record and marker retention were checked in the app log.
The unmodified head built successfully, and ConnectionRecoveryServiceTests and UsdtRecoveryBackupTests passed with no failures or skips. Changed Swift files passed SwiftFormat 0.64.0; translation validation and git diff --check passed.
Coverage: full ff999a50..9481b50, all changed files and reached restore/startup/wipe callers. Device coverage used an unfunded wallet with no stored USDT snapshot; nonempty USDT imports, payment replay and RN migration were not driven. Optional profile unavailability used a held initialization, not a remote outage or terminal identity failure.
Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes matches the restricted editor, durable replay and drain contract. iOS restores required backup state before node startup; Android can need a running node during restoration.
Debug fault controls and deciding log excerpts
These unpushed hooks held or rejected import/probe/storage boundaries and logged state. Backup encryption, authentication, restore operations and node startup used the app's real services.
ACK complete WALLET bytes=934 SETTINGS bytes=306 result=completed
STATE node=initializing walletPending=true usdtPending=false correction=9CC80216-DA07-4EBD-9328-734B325E897F electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot/ hideBalance=true busy=false
STATE node=stopped walletPending=true usdtPending=true correction=9CC80216-DA07-4EBD-9328-734B325E897F electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot/ hideBalance=true busy=false
HOLD hold-profile
STATE node=running walletPending=false usdtPending=false correction=nil electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot/ hideBalance=true busy=false
diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift
index 9b06e9fd..8593ed45 100644
--- a/Bitkit/AppScene.swift
+++ b/Bitkit/AppScene.swift
@@ -512,6 +512,9 @@ struct AppScene: View {
config in AppUpdateSheet(config: config)
}
.task(priority: .userInitiated, setupTask)
+ #if DEBUG // test hook
+ .task { await qaCommands() } // test hook
+ #endif // test hook
.task(id: [scenePhase == .active, wallet.walletExists == true, isWalletBackupRestoreRunning, network.isConnected]) {
guard scenePhase == .active, wallet.walletExists == true,
!isWalletBackupRestoreRunning, !BackupService.shared.hasPendingWalletRestore()
@@ -956,6 +959,55 @@ struct AppScene: View {
// MARK: - Event Handlers
+ #if DEBUG // test hook
+ @MainActor private func qaCommands() async { // test hook
+ for _ in 0..<14400 { // test hook
+ guard !Task.isCancelled else { return } // test hook
+ let file = RecoveryTestHook.directory.appendingPathComponent("qa-command") // test hook
+ if let text = try? String(contentsOf: file, encoding: .utf8) { // test hook
+ try? FileManager.default.removeItem(at: file) // test hook
+ Task { @MainActor in // test hook
+ do { // test hook
+ let command = text.trimmingCharacters(in: .whitespacesAndNewlines) // test hook
+ RecoveryTestHook.log("COMMAND " + command) // test hook
+ switch command { // test hook
+ case "backup": // test hook
+ await BackupService.shared.stopObservingBackups() // test hook
+ ElectrumConfigService().saveServerConfig(ElectrumServer(host: "127.0.0.1", port: 1, protocolType: .tcp)) // test hook
+ UserDefaults.standard.set(true, forKey: "hideBalance") // test hook
+ try await BackupService.shared.persistWalletBackup() // test hook
+ let result = await BackupService.shared.triggerBackup(category: .settings) // test hook
+ guard let walletData = try await VssBackupClient.shared.getObject(key: "WALLET")?.value, // test hook
+ let settings = try await VssBackupClient.shared.getObject(key: "SETTINGS")?.value else { throw AppError(message: "Missing fixture backup", debugMessage: nil) } // test hook
+ try walletData.write(to: RecoveryTestHook.directory.appendingPathComponent("qa-wallet-backup")) // test hook
+ try settings.write(to: RecoveryTestHook.directory.appendingPathComponent("qa-settings-backup")) // test hook
+ RecoveryTestHook.log("ACK complete WALLET bytes=\(walletData.count) SETTINGS bytes=\(settings.count) result=\(result)") // test hook
+ case "wipe": // test hook
+ if let seed = try Keychain.loadString(key: .bip39Mnemonic(index: 0)) { try seed.write(to: RecoveryTestHook.directory.appendingPathComponent("qa-seed"), atomically: true, encoding: .utf8) } // test hook
+ try await AppReset.wipe(app: app, wallet: wallet, session: session) // test hook
+ RecoveryTestHook.log("WIPE completed") // test hook
+ case "restore": // test hook
+ let seed = try String(contentsOf: RecoveryTestHook.directory.appendingPathComponent("qa-seed"), encoding: .utf8) // test hook
+ wallet.nodeLifecycleState = .initializing // test hook
+ wallet.isRestoringWallet = true // test hook
+ BackupService.shared.setRestoring(true) // test hook
+ try StartupHandler.restoreWallet(mnemonic: seed, bip39Passphrase: nil) // test hook
+ try InstallationMarker.create() // test hook
+ try wallet.setWalletExistsState() // test hook
+ case "retry": await retryWalletStart() // test hook
+ case "snapshot": break // test hook
+ default: break // test hook
+ } // test hook
+ RecoveryTestHook.snapshot(wallet) // test hook
+ RecoveryTestHook.log("DONE " + command) // test hook
+ } catch { RecoveryTestHook.log("ERROR \(error)") } // test hook
+ } // test hook
+ } // test hook
+ try? await Task.sleep(for: .milliseconds(500)) // test hook
+ } // test hook
+ } // test hook
+ #endif // test hook
+
private func handleCurrencyStaleData(_: Bool) {
if currency.hasStaleData {
app.toast(type: .error, title: "Rates currently unavailable", description: "An error has occurred. Please try again later.")
diff --git a/Bitkit/Managers/PubkyProfileManager.swift b/Bitkit/Managers/PubkyProfileManager.swift
index d29dd6b1..34270bae 100644
--- a/Bitkit/Managers/PubkyProfileManager.swift
+++ b/Bitkit/Managers/PubkyProfileManager.swift
@@ -262,6 +262,9 @@ class PubkyProfileManager: ObservableObject {
isRestoringSession = false
}
guard revision == Self.sessionRevision, Self.sessionMutationCount == 0, !Self.isProfileDeletionInFlight else { return }
+ #if DEBUG // test hook
+ await RecoveryTestHook.hold("hold-profile") // test hook
+ #endif // test hook
await initializeSessionState(mode: mode, initializeSession: initializeSession)
if case .automaticRecovery = mode, revision == Self.sessionRevision {
completedRecoveryVersion += 1
diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift
index add0ff5f..fd4f838d 100644
--- a/Bitkit/Services/BackupService.swift
+++ b/Bitkit/Services/BackupService.swift
@@ -531,6 +531,10 @@ class BackupService {
private func restoreUsdtRecovery(legacyWallet: Data?) async throws {
let legacy = try legacyWallet.map { try JSONDecoder().decode(WalletBackupV1.self, from: $0) }
+ #if DEBUG // test hook
+ await RecoveryTestHook.hold("hold-usdt") // test hook
+ try RecoveryTestHook.reject("reject-usdt") // test hook
+ #endif // test hook
try await usdtRecoveryBackup.restore(
legacyWallet: legacy?.usdtWallet,
legacyPaymentState: legacy?.paykitPaymentState?.usdt,
@@ -1243,6 +1247,12 @@ class BackupService {
}
if let dataBytes {
+ #if DEBUG // test hook
+ if category == .wallet { // test hook
+ await RecoveryTestHook.hold("hold-wallet") // test hook
+ try RecoveryTestHook.reject("reject-wallet") // test hook
+ } // test hook
+ #endif // test hook
try await restoreAction(dataBytes)
Logger.info("Restore success for: '\(category.rawValue)'", context: "BackupService")
} else {
diff --git a/Bitkit/Services/ConnectionRecoveryService.swift b/Bitkit/Services/ConnectionRecoveryService.swift
index b6c8961a..5a76f3c1 100644
--- a/Bitkit/Services/ConnectionRecoveryService.swift
+++ b/Bitkit/Services/ConnectionRecoveryService.swift
@@ -58,8 +58,18 @@ final class ConnectionRecoveryService: ObservableObject {
return Owner(walletIndex: 0, network: Env.networkName,
binding: PaykitPaymentStateBackup.ActiveOnchainAttempt.Wallet.binding(storeId: namespace))
},
- load: { try Keychain.load(key: .connectionRecovery) },
- store: { try Keychain.upsert(key: .connectionRecovery, data: $0) },
+ load: {
+ #if DEBUG // test hook
+ try RecoveryTestHook.reject("deny-read") // test hook
+ #endif // test hook
+ return try Keychain.load(key: .connectionRecovery)
+ },
+ store: {
+ #if DEBUG // test hook
+ try RecoveryTestHook.reject("deny-store") // test hook
+ #endif // test hook
+ try Keychain.upsert(key: .connectionRecovery, data: $0)
+ },
clear: { try Keychain.delete(key: .connectionRecovery) }
)
@@ -307,6 +317,9 @@ final class ConnectionRecoveryService: ObservableObject {
private func apply(_ record: Record) throws {
_ = try currentAccess()
+ #if DEBUG // test hook
+ try RecoveryTestHook.reject("deny-preferences") // test hook
+ #endif // test hook
if let electrum = record.electrum {
let data = try JSONEncoder().encode(electrum)
defaults.set(data, forKey: "electrumServer")
diff --git a/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift b/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
index f826e851..5112dbf9 100644
--- a/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
+++ b/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
@@ -91,6 +91,9 @@ extension SettingsViewModel {
/// and returns a 2xx for a valid endpoint, so a HEAD request confirms reachability without
/// downloading the body. Mirrors the Android RGS connect check.
nonisolated func isRgsEndpointReachable(_ url: String) async -> Bool {
+ #if DEBUG // test hook
+ await RecoveryTestHook.hold("hold-probe") // test hook
+ #endif // test hook
let base = url.hasSuffix("/") ? String(url.dropLast()) : url
guard let testUrl = URL(string: "\(base)/0") else {
return false
diff --git a/Bitkit/Utilities/RecoveryTestHook.swift b/Bitkit/Utilities/RecoveryTestHook.swift
new file mode 100644
index 00000000..c6c8a173
--- /dev/null
+++ b/Bitkit/Utilities/RecoveryTestHook.swift
@@ -0,0 +1,33 @@
+#if DEBUG // test hook
+import Foundation // test hook
+ // test hook
+enum RecoveryTestHook { // test hook
+ static var directory: URL { FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] } // test hook
+ static func flag(_ name: String) -> Bool { FileManager.default.fileExists(atPath: directory.appendingPathComponent("qa-" + name).path) } // test hook
+ static func log(_ value: String) { // test hook
+ let line = "\(Date().timeIntervalSince1970) \(value)\n" // test hook
+ let target = directory.appendingPathComponent("qa-events.log") // test hook
+ if !FileManager.default.fileExists(atPath: target.path) { _ = FileManager.default.createFile(atPath: target.path, contents: nil) } // test hook
+ if let handle = try? FileHandle(forWritingTo: target) { defer { try? handle.close() }; _ = try? handle.seekToEnd(); try? handle.write(contentsOf: Data(line.utf8)) } // test hook
+ Logger.info(value, context: "RecoveryTestHook") // test hook
+ } // test hook
+ static func reject(_ name: String) throws { // test hook
+ if flag(name) { log("REJECT " + name); throw AppError(message: "Controlled " + name + " failure", debugMessage: nil) } // test hook
+ } // test hook
+ static func hold(_ name: String) async { // test hook
+ guard flag(name) else { return } // test hook
+ log("HOLD " + name) // test hook
+ for _ in 0..<3600 { // test hook
+ if !flag(name) { log("RELEASE " + name); return } // test hook
+ await Task.detached { try? await Task.sleep(for: .milliseconds(500)) }.value // test hook
+ } // test hook
+ log("TIMEOUT " + name) // test hook
+ } // test hook
+ @MainActor static func snapshot(_ wallet: WalletViewModel) { // test hook
+ let correction = (try? Keychain.load(key: .connectionRecovery)).flatMap { try? JSONDecoder().decode(ConnectionRecoveryService.Record.self, from: $0) } // test hook
+ let pendingWallet = (try? Keychain.load(key: .paykitPendingBackupRestore)) != nil // test hook
+ let pendingUsdt = (try? Keychain.load(key: .usdtPendingBackupRestore)) != nil // test hook
+ log("STATE node=\(wallet.nodeLifecycleState) walletPending=\(pendingWallet) usdtPending=\(pendingUsdt) correction=\(correction?.revision.uuidString ?? "nil") electrum=\(ElectrumConfigService().getCurrentServer().fullUrl) rgs=\(RgsConfigService().getCurrentServerUrl()) hideBalance=\(UserDefaults.standard.bool(forKey: "hideBalance")) busy=\(ConnectionRecoveryService.shared.isBusy)") // test hook
+ } // test hook
+} // test hook
+#endif // test hookReviewer model: gpt-6.1-sol
Reasoning effort: high
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
|
The Manual 1 video is accessible and shows the replacement wallet reaching Home. To close the remaining acceptance receipt, please attach a redacted excerpt from the existing Manual 1 |
There was a problem hiding this comment.
Verdict:
The PR moved to f3222bf after this run; these results are for 93181d5.
Findings: I found no actionable defect in the recovery delta and sampled merged paths.
QA for 93181d5: The build did not complete before the head moved to f3222bf. The author reports authenticated J1 and local persistence and marker fault cases in the PR description; I have not independently reproduced those cases on this head. The held import and Back slice is also reported there. The replacement-wallet slice below was recorded at the earlier head.
Test 1: J1, required restore connection recovery ⏭️ Not independently rerun before the head moved.
The author reports the authenticated restore and optional-profile cases at this head.
Test 2: Manual 1, cancellation and replacement ⏭️ Editor and replacement sequence was not independently rerun before the head moved.
The earlier review recorded the held probe, drain, and replacement on 9481b50 with the hooks published in that review. The deny-read control was turned off before the sequence and remained off for the final snapshot. The timestamped redacted app log excerpt below answers the evidence request; times are CET on 27 October 2026.
13:10:21.192 HOLD hold-probe
13:10:25.413 COMMAND wipe
13:10:52.223 RELEASE hold-probe
13:10:59.560 WIPE completed
13:11:25.380 STATE node=running walletPending=false usdtPending=false correction=nil electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot hideBalance=false busy=false
Test 3: Manual 2, local persistence and marker failures ⏭️ Not independently rerun before the head moved.
The author reports correction, preference and required-marker fault cases at this head.
Coverage: delta 9481b50..93181d5, focused on recovery and sampled merged payment and transaction paths. Merged payment and layout changes were not driven in this review.
Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes matches the restricted editor and durable correction contract; startup ordering differs by platform.
Reviewer model: Unknown
Reasoning effort: Not exposed
Model and effort metadata were not available in the review session records.
Reviewed by gpt-6.1-terra-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
1 finding (1 P1). Adds a guarded connection-settings editor to the failed-restore screen, a keychain-backed correction record replayed across restore and startup, and lifecycle/wipe admission around wallet start. Reviewed f3222bf, full tier, statically: iOS does not build here, so Run Tests is the gate (pending at review time; SwiftFormat green).
What I checked, and 5 candidates I ruled out
Read in full: ConnectionRecoveryService.swift, WalletConnectionRecoveryView.swift, the AppScene.swift startup/restore/retry paths (handleWalletExistsChange through restoreVssBackup), InitializingWalletView.swift, the BackupService restore hunks, LightningService.stop, AppReset, the Keychain fault hooks.
Call sites traced: startWallet (scene setup, NodeRestarter foreground and network-restored paths), retryWalletStart, restoreWalletBackupAndStart, CoreService.syncLdkNodePayments, WalletViewModel.start / stopLightningNode / wipe, vssDeriveStoreId.
CI: SwiftFormat green, Run Tests pending.
Ruled out
- Nested
performdeadlocking as busy:restoreAdmittedWalletBackupAndStartcallsstartWalletinside an admitted operation, but the@TaskLocalaccess is inherited by the unstructuredTaskinperform, so the nested call takes theaccess.service === selfbranch, notAdmissionError.busy. - Foreground restart racing a retry:
NodeRestarter.restartgoes throughstartWallet, which returns on.busywhile a retry holds admission. - Correction surviving a wipe:
wipeEntireKeychaindeletesconnection_recoverywith the rest, andbeginWiperotates the generation and drains the active task first, so a stale editor or late callback cannot write. - Older SETTINGS backup overwriting a saved correction:
mergeRestoredSettingsoverlays the record beforerestoreSettingsDictionary, andreplayre-applies it afterwards. - Owner binding: derived from the VSS store id of the wallet-0 mnemonic and passphrase, so a correction saved for one seed throws instead of applying to another.
Merge confidence: 3/5, one P1 on the post-start activity sync path.
Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: Not exposed (the run metadata available to this reviewer does not carry it)
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
Re-review of 911f24c after f3222bf. 1 earlier finding: 1 fixed. 0 new findings. Delta is 3 files: AppScene.swift, ConnectionRecoveryService.swift, ConnectionRecoveryServiceTests.swift. Reviewed statically, because iOS does not build here, so Run Tests is the gate. It was pending at review time. SwiftFormat is green.
What I checked
Earlier P1 (an activity sync error after a successful start reopens the restore-failed screen): fixed. The catch in startAdmittedWallet now sets didWalletBackupRestoreFail only when canReportStartupFailure() holds, which is canReportOperationFailure() && readyGeneration != generation. complete(...) sets readyGeneration = generation right before onReady(), so a throw from syncLdkNodePayments after that point logs the error and plays the haptic but leaves the flag false. That matches the base behaviour.
Paths that must still fail closed: replay, stopLightningNode, wallet.start, checkCurrentOperation, and a complete that returns early (pending restore, node not running, revision mismatch) or throws on clear all happen before readyGeneration is set, so they still report. perform resets readyGeneration to nil on admission and beginWipe rotates the generation, so a new start or a wipe cannot inherit an earlier success (testAnotherStartDoesNotInheritEarlierSuccessfulRecovery). A nested perform takes the already-admitted branch and does not reset the flag mid-operation.
canFinishInitialization: still keyed on readyGeneration == lifecycle && !isBusy, so after a sync error in an acknowledged start, initialization can finish once the operation releases. testActivitySyncFailureAfterAcknowledgedStartDoesNotReopenRecovery asserts this.
Test shape: the three tests rebuild the catch predicate inside perform rather than calling AppScene.startAdmittedWallet. That is enough here, because the AppScene change is the single predicate the tests exercise.
Earlier cut candidates: none were cut for the cap last time. The 5 ruled out in the first review still hold, since the delta does not touch admission, wipe, settings merge or owner binding.
Merge confidence: 4/5. No open findings, but Run Tests had not finished on this head when I reviewed it.
Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: Not exposed (the run metadata available to this reviewer does not carry it)
There was a problem hiding this comment.
Verdict:
Findings: I found no actionable defects in the reviewed delta.
QA for 911f24c: iPhone 17 Pro simulator, iOS 26.5, disposable regtest wallet. The restore checks used local fault and hold controls against the app's actual backup and startup services.
Test 1: required restore connection recovery ✅ Saved corrections survived required restore failures and restart, and Retry opened Home after the required markers cleared.
The recording shows the editor and retry sequence. App state logs also confirmed the retained correction, separate WALLET and USDT pending markers, and their clearance before Home. Profile initialization was held during the final startup.
J1-repeat-2x.2x.mp4
Test 2: cancellation and wallet replacement ✅ An admitted import completed after Back, and wipe waited for a held probe before a replacement wallet opened.
App state logs confirmed the probe was held when wipe was requested, wipe completed after release, and the replacement had default endpoints and no correction or required markers.
M1-2x.2x.mp4
M1-probe-2x.2x.mp4
Test 3: correction persistence failures ✅ Rejected writes and preference promotion kept restoration gated.
The app reported both failures; state logs confirmed that the failed write left the wallet marker and the failed preference promotion retained the correction. Retry with an unreadable correction also remained gated.
M2-focus-2x.2x.mp4
Test 4: optional profile failure and retry ⏭️ A real profile load failure and retry were not driven.
Home opened while profile initialization was held; the disposable wallet had no created profile. The PR description reports a separate genuine-profile failure and retry.
Test 5: WALLET and USDT marker read, write and delete failures ⏭️ The six-boundary matrix remains unverified in this run.
The marker-write attempt was restarted before its injected failure could be confirmed in the app log; the other five boundaries were not driven. The PR description reports separate installed-app coverage of these faults.
The exact head built successfully. The accelerated recordings retain all changed frames; the first two recordings contain visible stutters. The prior review covers an earlier installed-app recovery run. The hosted tests passed on the current head, including the new service-boundary startup regression; an installed-app corrupt-payment-row reproduction was not driven.
Coverage: delta 93181d5..911f24c, including the merged backup lifecycle and USDT changes. The merged USDT balance and receive changes were not driven in this review.
Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes matches the restricted editor and durable correction contract; startup ordering differs by platform.
Reviewed by gpt-6.1-terra-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan: no HIGH or MEDIUM. One LOW inline: the initializing loader restarts on every wallet create, adding about 1.3 s.
The two leads from synonymdev/bitkit-android#1685 don't apply here:
- The node isn't rebuilt on every foreground or on a backup-only Try Again.
blocksWalletStartkeeps the node down while a marker exists, stopping a nil node does nothing, and the record clears on the first acknowledged start. iOS has no retry counter. - The connection editor doesn't survive a Forgot-PIN wipe:
beginWiperotates the generation.
The two new strings are in all 15 locales.
Device gate (partial): not run yet. Pending: backup-restore/connection-recovery.xml. The Electrum, VSS and old-SETTINGS fault steps are manual, since the Capabilities table doesn't list them.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full review of the complete bitkit-ios#1139 diff against merge base e12df99, at 911f24cd.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
An offline skipped start is now classified as a failed restore whenever initialization is already showing, including first-time wallet creation. Post-start activity sync no longer reopens that screen: complete records the ready generation before syncLdkNodePayments, and the catch sets the failure flag only while canReportStartupFailure() is still true. The initialization-progress reset is the same mechanism already tracked in #1158; it stays deferred for 3.2.0.
Compared with bitkit-android#1685 at 2986fde for this flow only: both keep a restricted editor, a durable correction, and wallet actions gated until required restore finishes. iOS restores required backup state before starting a rebuilt node; Android can need a running node during restoration. Android’s offline start records a pending start and does not mark restore failed. bitkit-ios#1123 at e12df99 was checked only for the pending-restore gate this PR calls, not as a full review.
Validation: source and ConnectionRecoveryServiceTests were inspected, not executed. On this head, Run Tests and SwiftFormat passed. Device testing: not performed in this review.
Suggested additional test cases
- Platform: iOS simulator.
- Setup: airplane mode after the app has observed no network; no existing wallet.
- Action: create a new wallet from New Wallet, and again with a passphrase.
- Expected: initialization stays in progress, or shows an offline state, and does not show “Spending balance error” or Connection Settings. After reconnecting, startup continues without a connection correction left behind.
Findings
- [MEDIUM] Offline wallet creation is shown as a failed restore — inline at
Bitkit/AppScene.swift:1061.
Reviewer model: grok-4.7
Reasoning effort: high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed to 753d6d1. No new findings.
e7d5595 only removes the didWalletBackupRestoreFail = true lines in the offline guard (AppScene.swift:1058-1067), which restores master's behaviour:
- Creating a wallet offline stays on the loader.
- Reconnecting resumes through the existing
nodeRestarter.restarton.initializing. - The seed is generated once, and there is no Back path while creation is deferred.
That fixes piotr-iohk's MEDIUM. My loader LOW at :942 is deferred to #1158, as agreed on its thread. The new offline-create action in connection-recovery.xml isn't on the Android twin yet (synonymdev/bitkit-android#1685).
Device gate (partial): 753d6d1, not run yet. Pending: connection-recovery.xml, including offline create. Network loss isn't in the Capabilities table, so it runs as a manual step.
There was a problem hiding this comment.
Verdict:
Findings: I found no actionable defects in the reviewed delta.
QA for 753d6d1: iPhone simulators, disposable regtest wallets and authenticated staging backup service. A temporary test-only network monitor controlled the offline branch; recovery fault selectors throw at the app's real storage and profile boundaries.
Test 1: controlled offline New Wallet ✅ Creation stayed in initialization without the restore error and reached Home after the monitor was released.
offline-2x.2x.mp4
Test 2: controlled offline passphrase creation ✅ Creation stayed in initialization without the restore error and reached Home after the monitor was released.
passphrase-2x.2x.mp4
Test 3: optional profile failure after required restore ✅ Home remained available and retry reloaded the restored profile after the profile-load fault was removed.
The app log confirmed both required markers cleared and the node started before the profile failure. The second recording repeats the profile failure and retry.
markers-2x.2x.mp4
profile-repeat-2x.2x.mp4
Test 4: WALLET and USDT marker storage faults ✅ Read, write and delete failures at both marker boundaries kept recovery gated and each restore completed after its fault was removed.
Each fault was exercised on a fresh restoration. A saved endpoint correction was applied before the final node start. App logs confirmed each injected boundary, both marker deletions and node startup. The final recording repeats all six faults in sequence; earlier recordings show their individual retries.
markers-2x.2x.mp4
usdt-write-2x.2x.mp4
wallet-read-2x.2x.mp4
usdt-read-2x.2x.mp4
usdt-delete-2x.2x.mp4
wallet-delete-2x.2x.mp4
marker-repeat-2x.2x.mp4
Test 5: literal transport loss and reconnect ⏭️ Not driven: the simulator had no device-scoped transport switch.
The controlled monitor tests above establish the application branch, not a physical transport disconnect or reconnect.
The recordings have visible stutters; the accelerated copies retain all changed frames. The earlier review covers the required restore, cancellation and correction-persistence cases at 911f24c; this delta leaves those paths intact. The separate delta review also checked the offline guard and reconnect path.
Coverage: delta 911f24c..753d6d1, plus the offline-creation finding and recovery cases. The documentation edit has no device behavior. The USDT fixture had no nonempty USDT snapshot; spending and full-chain synchronization were not driven.
Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes shares the restricted editor and durable correction contract; startup order differs, and its journey does not include the new offline-create step.
Reviewer model: gpt-6.1-terra
Reasoning effort: high
Reviewed by gpt-6.1-terra-high
There was a problem hiding this comment.
Verdict:
Findings: I found no actionable defects in the reviewed delta.
QA for be804c7: iPhone 17 Pro simulators, iOS 26.5, disposable regtest wallets and authenticated staging backup service. The installed build adds unpushed controls that reject or hold real recovery boundaries and log state.
Test 1: J1, recovery across the merged startup ✅ Saved corrections survived failed recovery and cold restart; Retry opened Home after required restoration and node startup completed.
A rejected RGS probe preserved the saved revision. Older SETTINGS restored the unrelated balance-visibility preference while retaining the corrected Electrum endpoint. App logs confirmed separate USDT and WALLET import rejections, both required markers clearing before node startup, and correction cleanup before Home.
recovery-repeat-2x.2x.mp4
Focused repeat at 2×: RGS rejection, WALLET rejection, cold restart, retained endpoints and successful completion. The earlier USDT rejection was checked in the original recording and app log. The repeat contains visible stutters; its accelerated copy retains every changed frame.
Test 2: Manual 3, literal transport loss and reconnect ⏭️ Not driven: the simulator has no device-scoped transport switch.
Endpoint failure does not make the path monitor report an offline device. The previously proved controlled-monitor tests remain applicable to the unchanged offline guard, but do not establish literal transport loss or reconnect.
Check 1: unmodified build and focused regressions ✅ The head built, and the selected recovery, USDT backup, initialization, Lightning configuration and metadata-cache tests passed.
This includes startup acknowledgement, stale callbacks, wipe admission, correction persistence, required markers, idle-peer retention and decoding removed cache fields. These component tests are separate from the installed recovery recording.
Check 2: earlier device evidence ✅ Prior passing cases remain applicable where the delta leaves their paths unchanged.
The 911f24c review covers cancellation, replacement and correction-persistence failures. The 753d6d1 review covers controlled offline creation through both entry points, optional-profile failure/retry and all six required-marker storage faults. Those boundaries and guards are unchanged; the merged startup was exercised again above.
Coverage: delta 753d6d1..be804c7, including the additive base integration and reached restore/startup paths. Merged Blocktank sunset, receive, payment and notification UI changes were source-reviewed but not device-driven. No nonempty USDT import, funded spending or full-chain synchronization was driven.
Initialization progress remains deferred to bitkit-ios#1158 Preserve initialization progress across connection-recovery completion.
Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes, checked at c747cc0b, retains the restricted editor and durable correction contract. iOS restores required backup state before starting a rebuilt node; Android can need a running node during restoration.
Unpushed test controls and deciding state
The controls stage a genuine backup/restore, reject selected import boundaries and read back lifecycle state. Authentication, encryption, import and startup use the app's real services. No wipe was run in this review.
diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift
index e06480ff..8c2aa084 100644
--- a/Bitkit/AppScene.swift
+++ b/Bitkit/AppScene.swift
@@ -502,6 +502,9 @@ struct AppScene: View {
config in AppUpdateSheet(config: config)
}
.task(priority: .userInitiated, setupTask)
+ #if DEBUG // test hook
+ .task { await qaCommands() } // test hook
+ #endif // test hook
.task(id: [scenePhase == .active, wallet.walletExists == true, isWalletBackupRestoreRunning, network.isConnected]) {
guard scenePhase == .active, wallet.walletExists == true,
!isWalletBackupRestoreRunning, !BackupService.shared.hasPendingWalletRestore()
@@ -953,6 +956,56 @@ struct AppScene: View {
// MARK: - Event Handlers
+ #if DEBUG // test hook
+ @MainActor private func qaCommands() async { // test hook
+ for _ in 0..<14400 { // test hook
+ guard !Task.isCancelled else { return } // test hook
+ let file = RecoveryTestHook.directory.appendingPathComponent("qa-command") // test hook
+ if let text = try? String(contentsOf: file, encoding: .utf8) { // test hook
+ try? FileManager.default.removeItem(at: file) // test hook
+ Task { @MainActor in // test hook
+ do { // test hook
+ let command = text.trimmingCharacters(in: .whitespacesAndNewlines) // test hook
+ RecoveryTestHook.log("COMMAND " + command) // test hook
+ switch command { // test hook
+ case "backup": // test hook
+ await BackupService.shared.stopObservingBackups() // test hook
+ ElectrumConfigService().saveServerConfig(ElectrumServer(host: "127.0.0.1", port: 1, protocolType: .tcp)) // test hook
+ UserDefaults.standard.set(true, forKey: "hideBalance") // test hook
+ try await BackupService.shared.persistWalletBackup() // test hook
+ let result = await BackupService.shared.triggerBackup(category: .settings) // test hook
+ guard let walletData = try await VssBackupClient.shared.getObject(key: "WALLET")?.value, // test hook
+ let settings = try await VssBackupClient.shared.getObject(key: "SETTINGS")?.value else { throw AppError(message: "Missing fixture backup", debugMessage: nil) } // test hook
+ if let seed = try Keychain.loadString(key: .bip39Mnemonic(index: 0)) { try seed.write(to: RecoveryTestHook.directory.appendingPathComponent("qa-seed"), atomically: true, encoding: .utf8) } // test hook
+ try walletData.write(to: RecoveryTestHook.directory.appendingPathComponent("qa-wallet-backup")) // test hook
+ try settings.write(to: RecoveryTestHook.directory.appendingPathComponent("qa-settings-backup")) // test hook
+ RecoveryTestHook.log("ACK complete WALLET bytes=\(walletData.count) SETTINGS bytes=\(settings.count) result=\(result)") // test hook
+ case "wipe": // test hook
+ if let seed = try Keychain.loadString(key: .bip39Mnemonic(index: 0)) { try seed.write(to: RecoveryTestHook.directory.appendingPathComponent("qa-seed"), atomically: true, encoding: .utf8) } // test hook
+ try await AppReset.wipe(app: app, wallet: wallet, session: session) // test hook
+ RecoveryTestHook.log("WIPE completed") // test hook
+ case "restore": // test hook
+ let seed = try String(contentsOf: RecoveryTestHook.directory.appendingPathComponent("qa-seed"), encoding: .utf8) // test hook
+ wallet.nodeLifecycleState = .initializing // test hook
+ wallet.isRestoringWallet = true // test hook
+ BackupService.shared.setRestoring(true) // test hook
+ try StartupHandler.restoreWallet(mnemonic: seed, bip39Passphrase: nil) // test hook
+ try InstallationMarker.create() // test hook
+ try wallet.setWalletExistsState() // test hook
+ case "retry": await retryWalletStart() // test hook
+ case "snapshot": break // test hook
+ default: break // test hook
+ } // test hook
+ RecoveryTestHook.snapshot(wallet) // test hook
+ RecoveryTestHook.log("DONE " + command) // test hook
+ } catch { RecoveryTestHook.log("ERROR \(error)") } // test hook
+ } // test hook
+ } // test hook
+ try? await Task.sleep(for: .milliseconds(500)) // test hook
+ } // test hook
+ } // test hook
+ #endif // test hook
+
private func handleCurrencyStaleData(_: Bool) {
if currency.hasStaleData {
app.toast(type: .error, title: "Rates currently unavailable", description: "An error has occurred. Please try again later.")
diff --git a/Bitkit/Managers/PubkyProfileManager.swift b/Bitkit/Managers/PubkyProfileManager.swift
index d025403c..64d7f752 100644
--- a/Bitkit/Managers/PubkyProfileManager.swift
+++ b/Bitkit/Managers/PubkyProfileManager.swift
@@ -262,6 +262,9 @@ class PubkyProfileManager: ObservableObject {
isRestoringSession = false
}
guard revision == Self.sessionRevision, Self.sessionMutationCount == 0, !Self.isProfileDeletionInFlight else { return }
+ #if DEBUG // test hook
+ await RecoveryTestHook.hold("hold-profile") // test hook
+ #endif // test hook
await initializeSessionState(mode: mode, initializeSession: initializeSession)
if case .automaticRecovery = mode, revision == Self.sessionRevision {
completedRecoveryVersion += 1
diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift
index 7563e474..0674f6d2 100644
--- a/Bitkit/Services/BackupService.swift
+++ b/Bitkit/Services/BackupService.swift
@@ -624,6 +624,10 @@ class BackupService {
private func restoreUsdtRecovery(legacyWallet: Data?) async throws {
let legacy = try legacyWallet.map { try JSONDecoder().decode(WalletBackupV1.self, from: $0) }
+ #if DEBUG // test hook
+ await RecoveryTestHook.hold("hold-usdt") // test hook
+ try RecoveryTestHook.reject("reject-usdt") // test hook
+ #endif // test hook
try await usdtRecoveryBackup.restore(
legacyWallet: legacy?.usdtWallet,
legacyPaymentState: legacy?.paykitPaymentState?.usdt,
@@ -1345,6 +1349,12 @@ class BackupService {
}
if let dataBytes {
+ #if DEBUG // test hook
+ if category == .wallet { // test hook
+ await RecoveryTestHook.hold("hold-wallet") // test hook
+ try RecoveryTestHook.reject("reject-wallet") // test hook
+ } // test hook
+ #endif // test hook
try await restoreAction(dataBytes)
Logger.info("Restore success for: '\(category.rawValue)'", context: "BackupService")
} else {
diff --git a/Bitkit/Services/ConnectionRecoveryService.swift b/Bitkit/Services/ConnectionRecoveryService.swift
index 6ce06a85..60862066 100644
--- a/Bitkit/Services/ConnectionRecoveryService.swift
+++ b/Bitkit/Services/ConnectionRecoveryService.swift
@@ -58,8 +58,18 @@ final class ConnectionRecoveryService: ObservableObject {
return Owner(walletIndex: 0, network: Env.networkName,
binding: PaykitPaymentStateBackup.ActiveOnchainAttempt.Wallet.binding(storeId: namespace))
},
- load: { try Keychain.load(key: .connectionRecovery) },
- store: { try Keychain.upsert(key: .connectionRecovery, data: $0) },
+ load: {
+ #if DEBUG // test hook
+ try RecoveryTestHook.reject("deny-read") // test hook
+ #endif // test hook
+ return try Keychain.load(key: .connectionRecovery)
+ },
+ store: {
+ #if DEBUG // test hook
+ try RecoveryTestHook.reject("deny-store") // test hook
+ #endif // test hook
+ try Keychain.upsert(key: .connectionRecovery, data: $0)
+ },
clear: { try Keychain.delete(key: .connectionRecovery) }
)
@@ -311,6 +321,9 @@ final class ConnectionRecoveryService: ObservableObject {
private func apply(_ record: Record) throws {
_ = try currentAccess()
+ #if DEBUG // test hook
+ try RecoveryTestHook.reject("deny-preferences") // test hook
+ #endif // test hook
if let electrum = record.electrum {
let data = try JSONEncoder().encode(electrum)
defaults.set(data, forKey: "electrumServer")
diff --git a/Bitkit/Utilities/RecoveryTestHook.swift b/Bitkit/Utilities/RecoveryTestHook.swift
new file mode 100644
index 00000000..c6c8a173
--- /dev/null
+++ b/Bitkit/Utilities/RecoveryTestHook.swift
@@ -0,0 +1,33 @@
+#if DEBUG // test hook
+import Foundation // test hook
+ // test hook
+enum RecoveryTestHook { // test hook
+ static var directory: URL { FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0] } // test hook
+ static func flag(_ name: String) -> Bool { FileManager.default.fileExists(atPath: directory.appendingPathComponent("qa-" + name).path) } // test hook
+ static func log(_ value: String) { // test hook
+ let line = "\(Date().timeIntervalSince1970) \(value)\n" // test hook
+ let target = directory.appendingPathComponent("qa-events.log") // test hook
+ if !FileManager.default.fileExists(atPath: target.path) { _ = FileManager.default.createFile(atPath: target.path, contents: nil) } // test hook
+ if let handle = try? FileHandle(forWritingTo: target) { defer { try? handle.close() }; _ = try? handle.seekToEnd(); try? handle.write(contentsOf: Data(line.utf8)) } // test hook
+ Logger.info(value, context: "RecoveryTestHook") // test hook
+ } // test hook
+ static func reject(_ name: String) throws { // test hook
+ if flag(name) { log("REJECT " + name); throw AppError(message: "Controlled " + name + " failure", debugMessage: nil) } // test hook
+ } // test hook
+ static func hold(_ name: String) async { // test hook
+ guard flag(name) else { return } // test hook
+ log("HOLD " + name) // test hook
+ for _ in 0..<3600 { // test hook
+ if !flag(name) { log("RELEASE " + name); return } // test hook
+ await Task.detached { try? await Task.sleep(for: .milliseconds(500)) }.value // test hook
+ } // test hook
+ log("TIMEOUT " + name) // test hook
+ } // test hook
+ @MainActor static func snapshot(_ wallet: WalletViewModel) { // test hook
+ let correction = (try? Keychain.load(key: .connectionRecovery)).flatMap { try? JSONDecoder().decode(ConnectionRecoveryService.Record.self, from: $0) } // test hook
+ let pendingWallet = (try? Keychain.load(key: .paykitPendingBackupRestore)) != nil // test hook
+ let pendingUsdt = (try? Keychain.load(key: .usdtPendingBackupRestore)) != nil // test hook
+ log("STATE node=\(wallet.nodeLifecycleState) walletPending=\(pendingWallet) usdtPending=\(pendingUsdt) correction=\(correction?.revision.uuidString ?? "nil") electrum=\(ElectrumConfigService().getCurrentServer().fullUrl) rgs=\(RgsConfigService().getCurrentServerUrl()) hideBalance=\(UserDefaults.standard.bool(forKey: "hideBalance")) busy=\(ConnectionRecoveryService.shared.isBusy)") // test hook
+ } // test hook
+} // test hook
+#endif // test hook
diff --git a/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift b/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
index f826e851..5112dbf9 100644
--- a/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
+++ b/Bitkit/ViewModels/Extensions/SettingsViewModel+Rgs.swift
@@ -91,6 +91,9 @@ extension SettingsViewModel {
/// and returns a 2xx for a valid endpoint, so a HEAD request confirms reachability without
/// downloading the body. Mirrors the Android RGS connect check.
nonisolated func isRgsEndpointReachable(_ url: String) async -> Bool {
+ #if DEBUG // test hook
+ await RecoveryTestHook.hold("hold-probe") // test hook
+ #endif // test hook
let base = url.hasSuffix("/") ? String(url.dropLast()) : url
guard let testUrl = URL(string: "\(base)/0") else {
return falseThe backup was acknowledged before restoration. Selected app-state receipts (28 October 2026):
19:06:41.770 CET REJECT reject-usdt
19:08:30.007 CET STATE node=initializing walletPending=true usdtPending=true correction=920D1437-33FF-40E3-8DB4-75D5B7B5918E electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot hideBalance=false busy=false
19:08:41.570 CET STATE node=stopped walletPending=true usdtPending=false correction=920D1437-33FF-40E3-8DB4-75D5B7B5918E electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot hideBalance=true busy=false
19:10:29.007 CET STATE node=starting walletPending=false usdtPending=false correction=DD7A03E5-34ED-4B92-87D4-B7DDCE6DDAF5 electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot hideBalance=true busy=true
19:10:44.640 CET STATE node=running walletPending=false usdtPending=false correction=nil electrum=ssl://electrs.bitkit.stag0.blocktank.to:9999 rgs=https://bitkit.stag0.blocktank.to/rgs/snapshot hideBalance=true busy=false
Reviewed by gpt-6.1-sol-high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving at be804c7. Since 753d6d1 the PR has only gained a merge of its base branch, and its own diff against fix/backup-observer-lifecycle is byte-identical.
Device gate: 753d6d1 on the iPhone 17 simulator (iOS 26.5), against staging VSS restore and electrs. A local Electrum proxy stood in for the failing and replacement endpoints.
- ✅
connection-recovery.xmlactions 1–10:- A failed restore offers Connection Settings and no ordinary actions.
- Saving shows Saved and doesn't retry by itself.
- A failed RGS probe keeps the last correction.
- Try Again resumes on the saved endpoint and deletes the recovery record.
- The correction and the gates survive a restart and three background/foreground cycles.
- From an old SETTINGS backup, only the corrected endpoint overrides; the other preferences restore.
- Ordinary actions wait for the WALLET and USDT acknowledgements, including with the
usdt_pending:deletefault selector. - When the profile is unavailable (
profile:loadselector), restore still completes and Profile offers a retry.
- ✅ Offline New Wallet: it stays on the loader with no Back, one
Saved bip39_mnemonic_0is logged, and it resumes on reconnect. With e7d5595's AppScene change reverted, the same run shows the Spending balance error.⚠️ Offline was forced with a gate-only patch that setsNetworkMonitor.isConnected = false, not by a real network loss.
On reconnect the loader restarts from about 29%, which is the progress reset already tracked in #1158.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of bitkit-ios#1139 at be804c7c, covering the complete diff from merge base 566b2ff8. No earlier review baseline was supplied. bitkit-ios#1123 was checked only as that parent for restore integration, and bitkit-android#1685 at c747cc0b only for the shared recovery contract.
No new actionable code findings.
A saved Electrum or RGS correction is written to the local recovery record before UserDefaults change, replayed around required WALLET and USDT restore, and removed only after the node is running with no pending restore. An activity-sync error after that acknowledgement leaves startup closed. Offline wallet creation returns before node startup and leaves initialization in place. An optional profile load failure stays in profile handling. A stop that reports NodeError.NotRunning drops the failed node so retry can build another one. iOS restores the required backup before that rebuilt node starts; the Android journey allows restoration to need a running node. iOS Electrum Save checks syntax, and startup proves connectivity.
Validation: unit-tests and SwiftFormat succeeded on this head. This review inspected the recovery tests and startup, restore, and wipe callers, and did not execute them because the Swift toolchain is not available here. Device testing was not performed. Author journey and manual results were not re-run.
Earlier findings reconciled at this revision: 1 still present, 1 verified fixed.
Existing concerns
- [LOW] Initializing loader restarts from 0% when connection recovery becomes idle — still unresolved; already tracked.
Reviewer model: grok-4.7
Reasoning effort: high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: full reassessment of 1f8e07eb against d33547cb, also the verified merge base, because the comparison base changed. No coverage was inherited. Android c747cc0b and dependency #1123 were checked only for affected recovery and backup-lifecycle contracts.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
Validation: local SwiftFormat 0.64.0 passed for all 11 changed Swift files; translation validation passed with zero errors and 15 warnings. Inspected hosted native-test logs for the exact head/base merge: the 28 connection-recovery and eight USDT-recovery tests report zero failures. Those assertions do not cover abandoned native-node lifetime.
Suggested additional test cases
- iOS, disposable wallet with an unreachable Electrum endpoint and no queued LDK events: instrument native node release, retry startup repeatedly, then save a reachable endpoint and retry. Expect prior nodes/runtimes to be released and one active listener for the recovered node.
Device testing: not performed in this review. Local native builds/tests were unavailable without Swift/Xcode; visual behavior and authenticated recovery journeys remain unexecuted.
Earlier findings reconciled at this revision: 1 still present, 1 verified fixed.
Findings
- [MEDIUM] Release failed-node event waiters before rebuilding on Retry — inline at
Bitkit/AppScene.swift:1293.
Existing concerns
- [LOW] Initializing loader restarts from 0% when connection recovery becomes idle — still unresolved; already tracked.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
There was a problem hiding this comment.
Verdict:
Findings: I found no new actionable defects. The failed-node listener finding is fixed: failed native starts create no consumer, and stop drains the captured node’s consumer before replacement.
QA for a4f5ca6: iPhone 17 Pro simulators, iOS 26.5, disposable regtest wallet, authenticated staging backup service and Electrum. The installed build adds unpushed boundary controls and weak-reference diagnostics; authentication, encrypted backups, imports and native node startup use the real services.
Failed node retries release previous nodes and recover ✅ Three failed starts retained only the current node wrapper; correcting the endpoint and retrying opened Home.
App-state readback showed one live wrapper and zero consumers after each failed allocation, then one live wrapper and one consumer after successful recovery. The saved correction cleared. This checks Swift reference lifetime, not native heap usage. Separate component tests cover acknowledged-callback draining and failed-stop resumption.
listener-retry2x.2x.mp4
Three failed starts, connection correction and successful Retry at 2×; reference and consumer counts were checked in the app log. The recording has visible stutters.
Saved connections survive required restoration ✅ The correction survived rejected imports and cold restart, and recovery opened Home after both required markers cleared.
The source backup was acknowledged before restoration. Separate USDT and WALLET import rejections kept access restricted. App logs confirmed the same correction revision across restart, selective replay over the old SETTINGS endpoint, restoration of the unrelated balance-visibility preference, and final running, both markers absent and correction absent.
restore2x.2x.mp4
Required-import failures, saved connection, cold restart and completion at 2×. State transitions were also checked in the app log. The recording has visible stutters; its accelerated copy retains every changed frame.
Optional profile reload recovers after foregrounding ✅ Profile failure preserved wallet access and retry controls; removing the fault and foregrounding reloaded the same profile.
The profile was created through normal signup before backup. The real profile:load fault occurred after required restoration and node startup. Profile showed its retry controls; after removing the fault, a background/foreground cycle loaded the original identity without another profile retry tap. The profile key was compared through the UI snapshot.
profile2x.2x.mp4
Optional profile error, retry controls and automatic reload at 2×.
New wallet waits through transport loss and reconnect ⏭️ The simulator provides no device-scoped transport switch.
I checked simulator Settings and the available network-control routes. Endpoint failure does not make the path monitor report an offline device. Earlier controlled-monitor results establish the application branch, not literal transport loss or reconnect.
Check: build and focused regressions ✅ The head built; selected listener, recovery, backup, allowance, profile, payment-lifecycle and contact tests passed.
I ran xcodebuild build and the focused xcodebuild test selections. The default Xcode 27 test build rejected a newer weak-reference mutability warning in NodeEventListenerTests. With that diagnostic demoted using -Wwarning VariableNeverMutated and dependency warning suppression disabled, the unchanged source passed the selected tests. swiftformat --lint with 0.64.0 passed on the changed Swift files. node scripts/validate-translations.js completed with warnings, and node --test .github/scripts/retarget-tests.test.cjs passed.
Earlier passing evidence remains applicable to unchanged boundaries:
- Required recovery, cancellation, wallet replacement and correction-storage failures at 911f24c.
- Controlled offline creation and all six required-marker storage faults at 753d6d1.
Coverage: contiguous delta be804c7..a4f5ca6, prior open findings and reached lifecycle/restore callers. Merged allowance, send/PIN, contact, toast, artwork and CI changes were source-reviewed; their full journeys were not driven. No nonempty USDT import, funded spending or full-chain synchronization was driven. Initialization progress remains deferred to bitkit-ios#1158 Preserve initialization progress across connection-recovery completion.
Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes, checked at c747cc0b, shares the restricted editor and durable correction contract. iOS restores required backup state before starting a rebuilt node; Android can need a running node during restoration.
Reviewed by gpt-6.1-sol-high
|
The Swift 6.4 compile issue from review 5702671374 is fixed in f38c33e, which also incorporates the current #1123 parent (8947f49). Constant weak-capture closures preserve both node-deallocation assertions without demoting warnings. The actual Xcode 27.1 final combined-tree build passed with warnings-as-errors: 2,912 unit tests and 39 physical UI executions, including all four listener tests; four prerequisite skips remain explicit. Full formatting/translation checks and complete nested diagnostics were inspected, and every one of the 12 live-base commits is verified locally and on GitHub. New-head hosted CI is pending. Your runtime observations retain their original scope; this does not claim literal transport-loss/reconnect or nonempty USDT restoration. Existing pending review requests are preserved. |
|
@ben-kaufman Moving this PR to 3.2.0 per the 3.1.0 cut line (https://synonymworkspace.slack.com/archives/C07BJ7DNPCG/p1793307568629859): the restore and profile-deletion stacks harden behaviour that already shipped in 3.0, so they keep landing on master and ship in 3.2. |
There was a problem hiding this comment.
Verdict:
Findings: I found no new actionable defects. The weak-capture closures preserve both node-release assertions and fix the Xcode 27 test-build warning reported in the previous review, without demoting diagnostics.
QA for f38c33e: Xcode 27.0, SDK 27.0, iPhone 17 Pro simulator on iOS 26.5.
Check: build and listener, recovery and parent regressions ✅ The unmodified head built with warnings-as-errors, and the selected component tests passed without skips.
I ran xcodebuild build, focused xcodebuild test selections and a supplemental payment-request service selection with test-without-building. The results cover node release, startup event delivery, acknowledged-callback draining, failed-stop resumption, connection recovery, foreground backup recovery, USDT recovery, SDK admission, deferred private contact work, recipient eligibility and connection-close navigation. Both result trees were inspected. SwiftFormat 0.64.0 passed on the changed Swift files, and git diff --check passed.
Check: retarget execution snapshots and reporting ✅ Both committed Node regression suites passed without failures or skips.
I ran node --test .github/scripts/retarget-tests.test.cjs .github/scripts/retarget-execution.test.cjs. These controls cover execution identity and signature checks, immutable snapshots after source movement, lost-response reconciliation, recovery dispatch and cleanup ownership. They do not dispatch a hosted workflow.
New wallet waits through transport loss and reconnect ⏭️ A device-scoped transport switch was unavailable on iOS Simulator.
Settings exposed no Wi-Fi or airplane-mode control, and both direct settings routes failed. The available simulator commands provide no transport switch. Endpoint failure and the earlier controlled network-monitor cases do not establish literal path loss or reconnect, so Manual 3 remains unverified.
Earlier passing device evidence remains applicable to the unchanged recovery, import, storage and cancellation boundaries:
- Required restoration, failed-node retry and optional-profile recovery at a4f5ca6.
- Cancellation, wallet replacement and correction-storage failures at 911f24c.
- Controlled offline creation and all six required-marker storage faults at 753d6d1.
Coverage: contiguous delta a4f5ca6..f38c33e, prior discussions and reached recovery/startup callers. Merged contact-publication, invoice-admission, connection-close navigation and CI changes were source-reviewed and component-tested; their complete app journeys were not driven. No new authenticated restoration, nonempty USDT import, funded spending or full-chain synchronization is claimed. Initialization progress remains deferred to bitkit-ios#1158 Preserve initialization progress across connection-recovery completion.
Twin: bitkit-android#1685 fix: guard wallet operations until required restore completes, still at c747cc0b, retains the shared restricted-editor and durable-correction contract. iOS restores required backup state before starting a rebuilt node; Android can need a running node during restoration.
Reviewed by gpt-6.1-sol-high
Current Xcode 27 repair and parent composition — October 29
Signed
f38c33e2317552a5716dd02ab4c6d653ed029daeincorporates published #1123/8947f493a681f57cb76486d96bbc36da3f9999a6; tree91ab0589b4a7ca1b3dd37ba172ecc8d6bcc58af6. Two weak node-release checks now use constant weak-capture closures, preserving their deallocation assertions while compiling under Swift 6.4. No warning demotion, timeout change or product behavior change is introduced by this test repair. Controlled compiler before/after and the affected independent reviews are clean; the other 32 child files and their edit sequences are preserved.The final combined tree passed 2,912 unit tests and 39 physical UI executions on Xcode 27.1/27A9275, SDK 27.1, iOS 26.2/23C54, with warnings-as-errors intact. All four listener lifecycle tests pass. Four prerequisite tests were skipped; the expired fork-payment method and four workflow integration classes were explicitly excluded, not passed. All 1,675 frozen inputs match. Full logs, all 2,955 nested summaries, eight diagnostic logs and attachments were inspected; the owner rehashed all 14,067 sealed evidence rows. SwiftFormat 0.64.0 reports 0/829; translation validation has zero errors and 15 warnings verified against the actual parent. Two AppIntents notices and 22 stdout QoS traces versus 21 nested entries remain disclosed; no runtime-causality claim is made.
All 12 live-base commits verify locally and on GitHub (
verified=true,reason=valid). Ready status remainsdraft=false. The original attempt of the current-head hosted native run failed atUsdtWalletUITests.swift:288: the camera permission alert remained visible through the original five-second wait after a centered Allow tap. All 2,912 units and 38 physical UI executions passed, one UI execution failed and five prerequisite cases skipped. All four listener tests passed. Complete logs, 2,956 nested summaries, 77 attachment payloads and 3,869,953 simulator system-log events were inspected; the owner verified 13,563 sealed evidence rows. TCC completed the original request only during the following test. Why the first tap did not dismiss the prompt remains unresolved; no harmless-flake or source-fix claim is made. The test did not reach its later invalid-recipient/payment assertions. Hosted execution was Xcode16.4/16F6, SDK18.5, iOS26.2/23C54, separate from the local Xcode27.1 pass. Hosted formatting passed 0/829; nine migration jobs skipped. All 286 dependency linker warnings match verified prior warning metadata and actual-parent producers. The green native checks below belong to prior heads. Upstream #1120's separate launch-assertion failure remains preserved; its one bounded native retry passed without a product-source repair claim. Existing J1/Manual acceptance retains its recorded revision and scope below. Literal transport loss/reconnect and nonempty USDT import are not newly proved by this suite.A separate follow-up correlated the original touch with SpringBoard's alert window and transport acknowledgement. The touch overlapped that same alert's Appearing → Appeared transition while SpringBoard logged
ignoreInteractionEvents=1; no second permission presentation or independent app foreground cycle was observed. This supports a presentation/input-suppression race, while exact Allow-action consumption remains unproved. All 21 follow-up evidence rows are verified. On that new service-boundary evidence, exactly one bounded native-job retry was requested; attempt 2 completed successfully. The original five-second assertion, failure and complete diagnostic record remain unchanged. No further unchanged retry is authorized.The single authorized attempt 2, job
123743388511, is now completely inspected: 2,912 units and 39 physical UI executions passed, five explicit skips, zero failures (2,922 logical passes). The original camera case passed its unchanged five-second alert-disappearance assertion in 1.201 seconds and executed the later invalid-recipient checks. All four listener tests passed. Checkoutc549a66dhas the exact published tree and actual8947f493parent. Complete logs, all 2,956 nested summaries, eight diagnostic files, 55 payloads and the checksum/CRC-verified archive were inspected; the owner verified 12,134 sealed rows. The 286 known linker messages, two AppIntents notices and 48 CoreGraphics NaN observations remain disclosed. No attempt-2 system archive was supplied, so no TCC/HID cause comparison is claimed. This passing repeatability check does not prove a source fix or erase attempt 1. Fixture skips and four excluded integration classes remain outside the pass count. Existing formatting and revision-scoped app acceptance remain unchanged.Prior node listener repair — October 29
Signed
a4f5ca6034f1025baa857a538d5e250c1529194afixes the failed-start and Retry listener lifetime finding on actual parent #1123/d33547cb; tree7c1411217be0e26324d475d246475a0d4bc9383e. One consumer captures one node and starts only after native startup succeeds. Stop drains an already acknowledged callback before stopping or replacing that node. Unexpected stop errors retain the node and resume its consumer. MainActor delivery is awaited; existing downstream payment/proof tasks remain unchanged.The pinned UniFFI asynchronous event wait ignores Swift cancellation. Cancellable nonblocking polling avoids retaining that wait, with up to 250 ms idle delivery latency and four idle wakeups per second. Four controlled fake-native lifecycle tests pass; omitting the drain produces the expected failure. This is not a native heap measurement or a new authenticated journey.
Final combined validation passed 2,897 selected unit and 39 UI executions, zero failures/retries, with five explicit prerequisite skips. All 1,668 frozen inputs match. Complete 22,918-line compile/test output, all 2,941 nested summaries, exported diagnostics and attachments were inspected; 14,047 evidence hashes verified. Two AppIntents notices, 12 nested QoS entries/13 canonical backtraces and 48 CoreGraphics NaN messages remain disclosed. Actual-parent producer comparison found no introduced compiler/format/translation violation; differing runtime incidence has no asserted cause. SwiftFormat 0.64.0: 0/828, 99 exclusions; translation validation: zero errors, 15 actual-parent inherited warnings. Three dependency checkouts have unavailable historical Git alternates for cleanliness comparison; all six pinned HEADs match and executed products are separately hashed. Four integration classes remain excluded.
Both affected independent reviews are clean. All 11 live-base commits verify locally and on GitHub (
verified=true,reason=valid). The immediate post-push REST response was stale; read-only reconciliation confirmed the published head without another push. Ready status is verified (draft=false). Current-head native tests and SwiftFormat now pass on synthetic2b43d8b7, whose tree equals this head: 2,897 unit + 39 UI physical passes, five explicit skips, zero failures/retries; 2,907 logical passes after launch-variant reconciliation. All four listener tests pass. Formatting is 0/828; nine migration jobs are skipped, not passes. Complete logs, every nested summary, exported diagnostics and all archive/attachment hashes were inspected; the owner verified 9,158 sealed evidence rows. All 286 distinct linker warnings match the actual parent. One keyboard timeout during a passing unchanged route test, two AppIntents notices and 48 NaN emissions remain disclosed; their cause/introduction is unproved. No CI rerun or broader authenticated acceptance is inferred. Existing app-journey evidence below retains its exact tested revision and scope; these local suites do not replace it or claim a new authenticated restore, wallet wipe, nonempty USDT import or spending run.Previous parent composition — October 29
Signed additive merge
1f8e07eb44005255207c67993d57403f04f22c63incorporates #1123 atd33547cba681f8cf826a715de5f51a2ba89c28ef; final treed764759c19b400f4bfbbeb5403c9b65202feebaa. The independent affected composition review is clean. Required restore markers, original-owner correction persistence, restricted routing, wipe drains and delayed-callback guards are retained.Final selected native validation passed 2,893 unit and 39 UI executions, zero failures, with five fixture skips. All 1,666 frozen inputs and all 2,937 nested physical summaries reconcile; the selected recovery suites include 28 ConnectionRecoveryService and eight UsdtRecoveryBackup passes. SwiftFormat 0.64.0 passed 0/826 (99 exclusions); translations passed zero errors and 15 actual-parent inherited warnings. Full logs and nested diagnostics were inspected: three AppIntents notices, 18 QoS observations across nine logical tests and 72 CoreGraphics NaN notices in passing USDT UI cases remain disclosed without causal claims. Four integration classes and missing/disabled fixture cases are not passes.
The complete 10-commit live-base range is locally and GitHub verified (
verified=true,reason=valid). Ready status is retained (draft=false). Hosted native/UI 40889840827 and SwiftFormat 40889840829 passed on the exact synthetic tree: 2,903 unique passes, zero failures/retries and five skips (2,937 physical summaries); formatting 0/826. Complete logs, nested summaries, diagnostics and checksum/CRC-verified artifacts were inspected; all 12,150 sealed evidence rows were verified. The 286 distinct linker warnings match the actual parent. One keyboard timeout remains in a passing route test, with unproven cause. Nine migration jobs were skipped, not passed. Piotr's subsequent node-event listener lifetime finding is addressed by the repair above; these green checks apply only to the earlier head. Existing app-journey evidence below retains its tested revision and scope; this composition does not claim a new authenticated restoration, actual wallet wipe, nonempty USDT import or spending journey.Twin: synonymdev/bitkit-android#1685
Dependency: #1123
A wallet whose required restoration failed can correct Electrum/RGS settings and retry through guarded startup. Corrections are acknowledged locally before settings change, survive restart and older SETTINGS restoration, and are cleared only after required imports and node startup complete. The editor keeps ordinary wallet actions gated. Exact typed “node not running” during stop permits rebuilding the failed node; other stop errors propagate. Late callbacks and stale editors cannot complete another lifecycle. Offline creation waits for connectivity without inventing a restore failure, and a later activity-sync error cannot reopen an already acknowledged startup.
Out of Scope
Current composition and validation
Signed additive merge
be804c7cincorporates #1123 at566b2ff8. The 29-file connection-recovery patch retains all 1,572 feature-line operations; the affected integration review is clean. Final composed-tree validation passed 2,858 unit and 39 UI executions, with three disabled Trezor skips and zero failures/retries. SwiftFormat: 0/824, 93 excluded; translations: zero errors and 15 warnings verified against the actual parent. Complete logs, all 2,900 nested execution summaries and 11,869 evidence hashes were inspected. Two AppIntents notices, eleven test-attached QoS observations and twelve raw process QoS notices are retained as distinct diagnostic counts; no timing-cause or warning-free runtime claim is made. All nine current-base commits are locally and GitHub verified. Current-head native tests and formatter now pass on the exact98f6e6e9tree: 2,858 unit + 39 UI passes, five explicit fixture/hardware skips, zero failures/retries, format 0/824. Complete logs, all 2,902 nested summaries and every original archive CRC/hash were inspected; the owner verified 8,945 sealed evidence files. The recurring 571 duplicate-symbol occurrences/286 symbols and one keyboard queue timeout during a passing 4.382-second lifecycle case remain disclosed; no causal repair or fresh exact-parent runtime baseline is claimed. Existing J1/Manual evidence below retains its recorded binary/fixture scope; this composition adds no fresh authenticated journey claim. The PR remains ready for review.QA
Setup
Use an isolated empty iPhone simulator and the pinned dependencies at
be804c7cf44352b7cd48933282437618c75e0957; do not reset an existing wallet. Build the simulator app withxcodebuild -project Bitkit.xcodeproj -scheme Bitkit -configuration Debug -destination 'platform=iOS Simulator,id=<OWNED_SIMULATOR_UUID>' E2E_BACKEND=network E2E_NETWORK=regtest 'SWIFT_ACTIVE_COMPILATION_CONDITIONS=$(inherited) E2E_BUILD' build. J1 requires an owned disposable LNURL/RS256 VSS + PostgreSQL backend, an acknowledged complete backup and older SETTINGS record, a failing Electrum endpoint and a reachable regtest replacement; these are separate controlled resources, not assumed available on public staging. The author's replacement wastcp://127.0.0.1:18682. Followjourneys/backup-restore/connection-recovery.xmlat that commit.For Manual 2, launch the built app with
-E2E_CONNECTION_RECOVERY_FAULTS. Select one of the twelve allowlisted boundaries fromBitkit/Utilities/Keychain.swiftin its own container'sLibrary/Application Support/connection-recovery-test-fault; remove only that selector before Retry. The selector injects errors at real boundaries and does not replace wallet/backup state. Manual 1's wipe slice remains attributed external evidence below; it does not authorize resetting a wallet. Manual 3 used a temporary offline monitor in two preserved test builds; it is absent from the published app, and literal simulator transport reconnect remains unclaimed.9481b503with its posted hooks: video, hook patch and the timestamped trace in review 5674650561 were inspected. The trace proves wipe requested while the probe was held, drain after release, completion and replacement defaults with no retained correction;deny-readwas off. Relevant paths remain compatible with the final tree. This is historical external evidence, not a fresh author-run wipe or permission to reset wallets.The ordinary fixture has a genuine empty independent USDT record listing. This is not a nonempty USDT import, full-chain synchronization or spending claim. Source cached height and stale RGS/network observations are retained in the evidence. The optional-profile selector produced five failed reads across startup, Profile entry and explicit Retry; the same genuine identity returned afterward. The final WALLET-delete fault is proven by runtime and blocked completion; its captured editor screenshot alone does not display that last error.
The DEBUG fault controls require simulator + E2E + regtest + explicit launch flag, exclude XCTest, and accept only twelve named boundaries in a fixed local file. They throw at the real local/profile boundary and never fabricate authenticated state, backup contents or success. Native authenticated network journeys use owned disposable resources; production/deployed-server equivalence is not claimed.
Validation
753d6d1f3dfa1b6ba2f39f608ab657251972897ais based on live #1123 /e12df99e2e91944308314d82bc8d71e9aaa24f66. The offline correction removes only the premature failure assignment; it does not clear an existing failure, complete a pending restore, erase a correction or change reconnect admission. The two creation entry points persist local entropy before startup, and the controlled installed-app before/after reproduces the reported branch. Earlier activity-sync regression tests remain unchanged.Final local validation passed 3,018 unit and 39 UI executions, with three explicitly disabled Trezor UI skips and no failures or retries. All 1,689 frozen inputs remained identical. Full SwiftFormat 0.64.0 lint passed (0/855). The sole complete diagnostic audit inspected 29,461 raw log lines, every nested test summary and exported issue: no Swift source warnings/errors; two AppIntents notices and 13 runtime priority-inversion observations remain. Their complete stacks match the prior local result; an actual-base runtime baseline was not run, so this is not a warning-free claim. One fresh combined affected-delta review is clean under the current risk-based review policy; prior whole-PR reviews remain applicable. All eight live-base commits verify locally and on GitHub.
The final documentation-only commit condenses the changelog to one sentence; all validated product, test and configuration inputs are byte-identical. Current-head hosted native/UI run 40675194474 passed 3,018 unit and 39 UI executions, with one native-fixture and four Trezor/fork UI skips, zero failures and no retries. Formatter 40675194463 passed 0/855. The logged synthetic merge
58228f6ahas the exact head tree2e1058ec. Complete 5,991-line logs, all 3,062 nested summaries, 572 mirrored issue objects, 1,180 activity messages and eight diagnostic logs were inspected; artifact digest and all 6,267 ZIP member CRC/SHA values were verified. The 286 dependency linker warnings match prior 911f24c messages and unchanged actual-parent pins/link configuration; an exact-parent build baseline is unavailable. No additional Swift or runtime checker diagnostics were found in this hosted Xcode16.4 export; local runtime observations remain separately preserved. Six unselected integration groups, five skipped tests and nine skipped migration jobs are not passes. The superseded e7d native run was automatically cancelled during compilation after the changelog-only push; it receives no test-pass credit.Applicable installed J1 and Manual 1–2 retain their exact recorded earlier build provenance and scoped source-composition verification. Their evidence is preserved in
work/ios1139-pending-e-evidence-review.md,work/ios1139-local-storage-app-f-evidence-review.md,work/ios1139-required-marker-app-g-evidence-review.md,work/ios1139-optional-profile-app-h-evidence-review.mdand the final validation directories. The new offline regression is inwork/validation/ios1139-offline-creation/. Prior failed commands, tests, initial attempts, runtime observations and narrower external evidence remain retained; no new wipe or literal transport-toggle pass is inferred.Readiness
This PR remains ready under the explicit readiness authorization. Applicable J1 and Manual 1–2 acceptance and the bounded Manual 3 regression are complete at the scope above. Full-wallet redesign and native contact CAS are separate work. Awaiting human approval does not require draft status; all review states and outstanding discussions remain visible. The final local native/UI and formatter checks above are verified; current-head hosted formatting and the single authorized native retry are fully inspected above, with original failures and remaining runtime limits retained.
Models used
Planning and implementation:
gpt-6-astra(reasoning:xhigh). Independent source review:gpt-6-astra(reasoning:xhigh) andgpt-6.1-sol(reasoning:high). Scoped final diagnostics and external-evidence review:gpt-6.1-sol(reasoning:high).