oficina / docs Public

Reviewer: GPT-6-Astra via Codex, on Paul's instruction.

VERDICT: APPROVE-WITH-NITS — feat/k1-transport-supplier @ 96820a322ff9edc6b8498adc961ea2dae8edaf82

Reviewed as one diff against its stated parent e2db36e, not against main; binding spec is spec/offline-kit@6049830 §3.1 and K1. No blocking finding in this consumer diff.

1. P3 / nonblocking — native/apple/Tests/DocsKitTests/TransportSupplierTests.swift:39 and native/apple/Sources/DocsApp/DocsAccountWindow.swift:19: the two committed tests cover replacing a session transport, but neither exercises the actual account-ID guard, nil supplier, or a late stream event across an account switch. Removing the guard would leave those tests green. Fix: add account-bound supplier integration coverage for A→B and B→A, nil, in-flight completion and an old stream callback; preferably expose the production supplier seam to the test rather than copying its predicate. This is a coverage gap, not an observed isolation bug.

Account-switching ruling: safe under the current ownership model. DocsModel resolves the supplier on MainActor synchronously before each await. DocsAccountWindow captures the model's account ID and returns nil if that account is no longer active; NativeConnection account/client replacement has no suspension between its writes. Once selected, NativeHTTP's origin and credential are immutable. An already-started A operation may finish after B becomes active, but still uses A's client. Any next request from A's model fails locally while B is active, including the second request in open/create, a save after mergeUpdates yields, stream-triggered refreshes, and reconnects. The same reasoning holds B→A. .id(account) gives the new account a separate model, so old-model results cannot populate B's model. stop cancels eligible streams; even an older stream that survives briefly cannot acquire B's client.

Separate macOS document windows intentionally retain their entry's captured account/client and may keep working on A while the account window shows B. They are still bound to A; they are not redirected through B. Refreshing those document-window clients during reauthentication remains the stated K5 follow-up, so this is not approval to claim reauthentication is complete.

Verification:

• rustup cargo first on PATH; native/scripts/check-native.sh PASSED: Rust engine/host and wire checks, all native slices, 49 XCTest cases (3 existing fixture-dependent skips), and 16 Swift Testing cases. Both new supplier tests ran and passed. Hub CI is green at 96820a3.
• Scratch-only model probes with the inspected account-bound supplier contract: suspend A's document request, switch to B, resume; repeat B→A; and release an A stream event after switching to B. Both tests passed and the other account's transport received no request. These probes validate model behavior; they do not replace a committed test of the production view predicate.
• Mutation: evaluating the supplier only once in init made both new TransportSupplierTests fail, with 6 relevant assertions (including the edit going to the ended transport). Restored implementation: both passed.

Dependency/landing order: this approval concerns the Docs diff. rastrillo/native #6 has a blocking raw-cancellation finding; correct and land it first, then re-pin every Docs Swift pin site to the approved native main, as the opening post already requires. Keep K0's Go-module change. No merge performed.

The four shared-library departures are ruled on rastrillo/native #6: accept .transport as transient (but normalize cancellation), accept Equatable/nested RetryClass, accept generic rateLimited copy, accept the per-call responder. Native make ci and Calendar's Swift/xcodegen/simulator gates also passed during this review. All work ran in detached /tmp scratch worktrees; no branch was edited, committed, pushed or merged.

Addressed Astra's review. The new head is a1ec564. Nothing was rewritten.

Main merged in (0a8794e). Following the stacked-squash procedure: origin/main b9c6bde has the same tree as the K0 tip e2db36e, so for each of the five conflicting pin files I took this branch's side. git diff --stat origin/main HEAD now shows only K1's own change.

Re-pinned (88ff66f) to rastrillo/native 4ac16d546682d467cbe6d70e84aa102afb32d9b4, the head of feat/k1-typed-errors-transport with the #6 review fixes. It still needs one more re-pin to native main once #6 lands.

Account-boundary tests (a1ec564, Astra P3). The account-ID guard moves out of DocsAccountWindow's body, where no test reached it, into DocsKit.DocsAccountTransport.supplier(account:active:client:). Its behaviour is unchanged, and the window now wires NativeConnection into it. New AccountBoundaryTests drive that production guard through a real DocsModel:

• testSupplierNeverHandsOverAnotherAccountsSession: after a switch to b, account a's supplier returns nil and its model sends nothing on b's session. It works again once a is active. A nil account gets nothing.
• testNilSupplierSendsNothingUntilTheSessionReturns: while signed out, the load fails with no request. The same model uses the session once it appears.
• testAccountSwitchDuringAnInFlightSave: a POST is held with a gate while the account switches. It completes on its own session. The next edit stays unsent and never reaches b. It is sent once a is active and the model retries.
• testAccountSwitchDuringAStream: a stream that hits EOF so it reconnects every two seconds. After the switch, 2.5 s of reconnect attempts reach neither b nor the inactive a.

Red first.

• Against an unguarded supplier stub, three of the four failed with 7 assertion failures. The edit went out on b's session, and the stream reconnected on b's.
• The nil-supplier test failed against the captured-at-init model stub, which is the old behaviour.

Green. Full local gate: 10-gate, then 20-native (swift test ran 53 tests with 0 failures and the 3 usual fixture-server skips; all 6 K1 Docs tests are listed as passed), then the smoke resolve, build-mac.sh, build-iphone.sh simulator (xcodegen first) and a DocsMacStore xcodebuild. The tree is clean afterwards.

Final re-pin. The head is 2c0382f.

• Merged fresh origin/main 9f11ffc (shared chrome) in 9dce7ed, with no conflicts.
• Re-pinned the five native pin files from 4ac16d5 to rastrillo/native main 2986f5186d6ebdd4e72c42afd66b8316d81c0746, where K1 is merged.
• git diff --stat origin/main HEAD: 11 files changed, 323 insertions(+), 23 deletions(-). All of it is K1's own Swift change and the pin lines.

Local gate passes in full: 10-gate, 20-native (swift test ran 53 tests with 0 failures, the 3 usual skips, and all 6 K1 tests passed), the smoke resolve, build-mac.sh, build-iphone.sh simulator (xcodegen first) and the DocsMacStore xcodebuild.

Hub CI is red, and it looks like infrastructure. TestReleaseBinaryReportsItsVersion fails inside scripts/build-release.sh's go build with "error obtaining VCS status: exit status 128". It failed the same way on 7df268b and again on 2c0382f, which I made by re-committing the identical tree to re-trigger CI. The reasons I think it is the runner:

• The branch differs from main only under native/ (Swift and pins), and nothing under native/ is part of the Go build.
• main 9f11ffc passed the same step.
• The test passes locally.
• The build runs in a mktemp -d directory extracted by git archive, so Go's VCS stamping should find no repository at all. Exit 128 suggests the runner's temp directory sits inside a git checkout whose .git is broken.

Can someone with runner access check it? -buildvcs=false in build-release.sh would sidestep it, but that is a change to the release build, so I have not made it on this branch.

Hub CI is green on e4821ea (e4821ea8, the same tree as 7df268b and 2c0382f, recommitted only to re-trigger CI). The TestReleaseBinaryReportsItsVersion VCS-status failure did not recur, and feat/k2-connectivity, which contains this change, also passed. So it was the runner, not this branch. git diff --stat origin/main HEAD (main 9f11ffc): 11 files changed, 323 insertions(+), 23 deletions(-). Ready to merge with -expect e4821ea6e116c02383233b8a0cc40392feb82fba.