oficina / docs Public

Reviewer: Astra (GPT-6-Astra via Codex). Whole-branch review of feat/k2-connectivity at fd67d33b36e52896e1ef0200b413b3d3eb7acb19, against fetched origin/main (e0c2f2a). Binding spec: rastrillo/native spec/offline-kit at 6049830, §3.2–3.4 and the K2 slice in §4.10. I read the aggregate merge-base diff and checked its content against current main: K1's squash is byte-for-byte identical to this branch's parent, so the main-to-head content delta is the nine-file K2 change.

VERDICT: CHANGES-REQUIRED

1. P2 — a connectivity wake bypasses Retry-After. native/apple/Sources/DocsKit/DocsModel.swift:352. A 429 with Retry-After: 30 becomes a 30-second argument to wait, but foreground/path/reachability wakes end that wait, reset backoff and immediately start another stream attempt. Scratch reproduction against the real DocsModel.listen, Backoff and NativeConnectivity: send the 429, foreground after 100 ms, inspect after another 600 ms; stream count is 2, expected 1. Keep a monotonic server-imposed retry deadline (capped at five minutes) separately from exponential backoff. A wake can reset exponential delay and reassess connectivity but must not erase the remaining server floor. Preserve the path-down/no-timer rule. Add a regression test with a wake inside Retry-After.

1. P2 — connection setup and access refresh are counted as an open stream. native/apple/Sources/DocsKit/DocsModel.swift:332 and :350–351. opened is captured before the stream request, and elapsed time is read after the catch's refreshAccess. A slow refusal, timeout or access-refresh request therefore resets backoff even when no stream ever opened. Scratch reproduction: three quick 503s, followed by a fourth 503 delayed 5.2 seconds; a fifth stream attempt occurs within 700 ms instead of retaining the expected 2–4-second delay. Capture a successful opening via the transport's onOpen, measure until stream termination, and exclude connection setup and post-failure requests. Only an actually open stream lasting more than five seconds should get the stable-stream reset.

1. P2 — merge main into this stacked branch before its squash merge. This is a demonstrated integration blocker, not a recommendation based only on ancestry. git merge-tree --write-tree origin/main HEAD reports six content conflicts: native/apple/Package.resolved, native/apple/Package.swift, native/apple/Sources/DocsKit/DocsModel.swift, native/ios/project.yml, native/macos/project.yml, and native/smoke/document-window/Package.resolved. git diff --quiet e0c2f2a HEAD^ succeeds for the entire tree, confirming the parent and K1 squash have identical content. Merge origin/main into the branch, preserve the K2 content in these conflicts, verify the resulting main-to-branch diff contains only K2, then rerun the gate and request review of the corrected head. I did not perform a merge or move a checkout.

Validation: with $HOME/.cargo/bin first on PATH, native/scripts/check-native.sh exited 0. Rust tests, both directions of wire compatibility, rule-wire checks and all CRDT framework architecture builds passed; Swift ran 55 XCTest cases (3 existing fixture-dependent skips) plus 16 Swift Testing tests with no failures. Mutation check: replacing the backoff wait with a fixed two-second wait made testStreamBacksOffBetweenQuickFailures fail specifically with 1 attempt, expected at least 2. After restoring the source, both original StreamConnectivityTests passed again. The two failing probes above ran against the unmodified production loop; an additional probe confirmed closing a clean model while offline cancels the loop and foreground does not restart it.

Other checks: the immediate-EOF test exercises the actual listen implementation and does not spin without delay. The supplier/account-boundary tests also passed; the K1 supplier still prevents an old workspace borrowing another account's transport. ScriptedTransport drives the real code, but does not emit NativeHTTP outcome-observer notifications, so it is not end-to-end coverage of request-driven reachability. There are no server/migration changes in K2.

Spec departures: per-origin connectivity is accepted; deferring banner summary and durable-save wording until K3 is correct. App-catalog English copy is acceptable for K2, with the package catalog/twelve locales still required at K3. The English wording is approved. Re-pin to reviewed native main after native #7 lands, as already planned.

Review-only: no branch edits, commits, pushes or merges. Scratch mutations were restored; the scratch worktree and build outputs have been removed. Physical-device airplane mode, Wi-Fi toggles and Network Link Conditioner were not performed in this review.

Addressed Astra's review. The new head is c521f9d; hub CI passed.

P2, merge main first (f71d4e5). I merged origin/main e0c2f2a, the K1 squash. main's tree is byte-identical to this branch's parent 2c0382f, and I checked each of the six conflicting files against it before taking K2's side. git diff --stat origin/main HEAD: 9 files changed, 158 insertions(+), 9 deletions(-). That is K2 only: DocsModel, DocsWorkspace, the en string, the tests, Package.swift/Package.resolved, the two project.yml and the smoke Package.resolved.

P2, Retry-After crossed by wakes; P2, stable clock started before open. Both are fixed once in the kit, rastrillo/native fix/k2-retry-deadline (#8, 67cd5ef), with NativeReconnect. listen now calls reconnect.opened() in onOpen, reconnect.failed(error) in the catch, and await reconnect.pause() between attempts. Backoff is no longer used directly, so DocsKit's RastrilloNative dependency is dropped again. There is a test-only reconnectClock on DocsModel.

• Pinned to 67cd5efe420f7c813254022598b7328a555781ef in the five pin files. Re-pin to main when #8 merges.

Tests, shown failing first (StreamPacingTests):

• testRetryAfterSurvivesAWake: a 429 with Retry-After: 1, then a foreground wake at 0.2 s. There is no second stream attempt before 0.7 s, and the next attempt comes at or after 0.95 s.
• testSlowFailuresThatNeverOpenedKeepBackingOff: each stream attempt costs 10 s on an injected clock and never opens, and there are at most 3 attempts in 1.6 s.
• Red. Against the previous loop (clock injected, no other change), the wake test got a second attempt early ("A foreground wake crossed the server's Retry-After"), and the slow-failure test got 4 attempts ("reset the backoff").

Full local gate passes: 10-gate, 20-native (swift test ran 57 tests with 0 failures and the 3 usual skips; all four stream tests passed), the smoke resolve, build-mac.sh, build-iphone.sh simulator (xcodegen first) and the DocsMacStore xcodebuild. The tree is clean afterwards.

Re-pinned to rastrillo/native main 14ee567 (the #8 fix merged) in all five pin files (cd94124). Docs main then moved to 1fcb203 (estilo bump), so I merged it in as 050b972 with no conflicts. Head 050b9721cbe082eb7dda94678efc58cb15141bf5, hub CI passed. git diff --stat origin/main HEAD: 9 files, +158 −9, K2 only. The full local gate was rerun after the merge, one stage at a time, deleting each stage's outputs because the disk is tight: 10-gate, 20-native (57 tests, 0 failures, 3 usual skips; all 4 stream tests passed), smoke resolve, build-mac.sh, build-iphone.sh simulator (xcodegen first) and DocsMacStore. All pass, and the tree is clean. Ready for -squash -expect 050b9721cbe082eb7dda94678efc58cb15141bf5.