fix(mobile): resolve document providers once per selection

[jabrailkhalil]

What this PR does

Resolve each document-provider authority once while validating a native file-picker result. Keep the cache local to one result and still check the content scheme, authority, provider ownership and read permission for every selected URI.

Why it's needed

The follow-up review of #12126 identified repeated provider-resolution Binder calls for documents from the same authority. Before this change, a selection of 100 documents from one provider performs 100 resolutions. The new code performs one resolution while retaining all 100 permission checks. This addresses the nonblocking provider-cache suggestion separately from the already merged file-selection feature. The Phase 1 design also gains synchronized English/Chinese notices linking to the implemented file-selection design, addressing the stale-documentation finding in the same parent review.

Reviewer Test Plan

How to verify

1. From packages/mobile-shell, run ./gradlew :app:assembleDebug :app:assembleDebugAndroidTest :app:testDebugUnitTest :app:lintDebug.
2. With an Android emulator/device attached, run ./gradlew :app:connectedDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=com.qwen.mobileshell.FilePickerDeviceTest.
3. The new same-provider test accepts 100 granted documents twice on the same picker: each result must resolve its provider again and check every URI grant. The per-document-grant test must reject a selection whose second document lacks a grant despite sharing the first document's provider. The mixed-authority test must reject the unsafe result and observe a separate package-manager access for the fixture provider and the settings provider. Existing unsafe-provider, URI, limit, callback and lifecycle tests remain unchanged.

Evidence (Before & After)

The actual production controller was also compiled and executed in a local JVM harness with controlled Android API stubs. Both cache-count assertions fail on upstream 0a5f518b4f73 and pass with this change; all nine behavioral/security controls pass before and after.

Selection	Before: provider resolutions	After: provider resolutions	URI permission checks
100 documents, one authority	100	1	100
100 documents, two authorities	100	2	100

The harness also checks provider ownership/availability changes between requests, denied URI grants, unsafe/unknown/same-UID providers, deduplication and the selection limit. These are controller call-count observations, not Android UI timing measurements. The checked-in Android tests count accesses to the package manager at the controller's provider-resolution boundary and delegate real permission checks to the target context. To verify R1-1, the three actual checked-in test methods were also executed through JUnit on the JVM with controlled Android API stubs: production passes all three; both a one-slot cache without an authority key and a manager-hoisting/no-cache mutation pass the old two tests but fail the new mixed-authority test (expected 2 manager accesses, observed 1). This mutation execution is not Android instrumentation.

Tested on

OS	Status
macOS	Not run
Windows	APK and test APK assembled; 21 JVM unit tests passed; lint: 0 errors, 6 existing dependency-version warnings; local controller harness: 11/11 passed
Linux	Upstream Android CI on previous head 70256adaa9: API 26: 22 passed / 4 expected skips; API 36: 25 passed / 1 expected skip; all 19 file-picker tests passed on both; CI for the updated test head is pending

Environment

Windows, JDK 17, Gradle 8.2.1, Android SDK 34. The instrumentation tests were compiled and packaged, but have not been executed on an Android emulator or physical device locally.

Risk & Scope

• Main risk: a cache lasting across requests could hide provider changes; the map is local to one validation invocation and the repeated-selection test pins this lifetime.
• Not validated: local Android instrumentation execution, third-party provider behavior or UI latency. Android CI/device execution remains required.
• Breaking changes / migration: none. No permission checks or URI restrictions are relaxed.

Linked Issues

Follow-up to #12126, related to #11704 and #13111. Reviewer evidence: #12126 (comment).

中文说明

本 PR 的改动

验证原生文件选择器结果时，每个文档提供程序 authority 只解析一次。缓存仅存在于本次结果处理中；仍对每个选中的 URI 检查 content scheme、authority、提供程序所有者以及读取权限。

为什么需要

#12126 的后续评审指出，同一 authority 下的多个文档会重复触发提供程序解析的 Binder 调用。改动前，同一个提供程序的 100 个文档会触发 100 次解析；改动后只解析一次，同时保留全部 100 次权限检查。本 PR 单独处理这一非阻塞缓存建议，不扩大已合入文件选择功能的范围。Phase 1 设计文档同时添加同步的中英文说明，链接到已实现的文件选择设计，处理同一次原 PR 评审中的文档过时问题。

评审测试计划

如何验证

1. 在 packages/mobile-shell 运行 ./gradlew :app:assembleDebug :app:assembleDebugAndroidTest :app:testDebugUnitTest :app:lintDebug。
2. 连接 Android 模拟器或设备后，运行 ./gradlew :app:connectedDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=com.qwen.mobileshell.FilePickerDeviceTest。
3. 新增的同一提供程序测试在同一个 picker 上连续接受两次各 100 个已授权文档；每次结果必须重新解析提供程序，并检查每个 URI 的授权。逐文档授权测试必须拒绝第二个文档未授权的选择，即使两个文档来自同一个提供程序。混合 authority 测试必须拒绝危险结果，并分别访问 fixture 提供程序和 settings 提供程序的 package manager。现有的危险提供程序、URI、数量上限、回调和生命周期测试保持不变。

证据（改动前后）

另用受控 Android API 桩构建本地 JVM harness，编译并运行实际生产控制器。在 upstream 0a5f518b4f73 上，两项缓存调用次数断言均失败；改动后均通过。其余九项行为与安全控制在改动前后均通过。

选择	改动前提供程序解析次数	改动后提供程序解析次数	URI 权限检查次数
100 个文档，一个 authority	100	1	100
100 个文档，两个 authority	100	2	100

Harness 还检查请求之间提供程序所有者或可用性的变化、URI 授权拒绝、危险或未知或同 UID 的提供程序、去重以及数量上限。这些数据是控制器调用次数，不是 Android UI 耗时。提交的 Android 测试统计控制器解析提供程序时对 package manager 的访问，并把实际权限判断委托给目标 context。R1-1 的三个实际测试方法还通过 JUnit 在受控 Android API 桩的 JVM 上执行：生产实现全部通过；无 authority 键的单槽缓存和移出循环但不缓存解析的两种变异均通过旧的两项测试，却在新增混合 authority 测试上失败（应为 2 次访问，实际 1 次）。这一变异验证不是 Android instrumentation。

本地测试环境

操作系统	状态
macOS	未运行
Windows	应用 APK 和测试 APK 构建成功；21 项 JVM 单元测试通过；lint 为 0 错误、6 项已有依赖版本警告；本地控制器 harness 11/11 通过
Linux	前一提交 70256adaa9 的 upstream Android CI：API 26 为 22 通过 / 4 预期跳过；API 36 为 25 通过 / 1 预期跳过；两者均通过全部 19 项 file-picker 测试；新增测试提交的 CI 待运行

环境

Windows、JDK 17、Gradle 8.2.1、Android SDK 34。Instrumentation 测试已编译打包，但本地尚未在 Android 模拟器或真机上执行。

风险与范围

• 主要风险：跨请求缓存可能掩盖提供程序变化；本实现的 map 仅存在于一次验证调用中，连续选择测试固定这一生命周期。
• 未验证：本地 Android instrumentation 执行、第三方提供程序行为以及 UI 延迟；仍需 Android CI 或设备执行。
• 破坏性变更或迁移：无；没有放宽任何权限检查或 URI 限制。

关联

#12126 的后续工作，关联 #11704、#13111。评审证据：https://github.com/QwenLM/qwen-code/pull/12126#issuecomment-5848429760。

[qwen-code-ci-bot]

⚠️ Deferred approval withheld — 1 PR CI workflow run(s) on 70256ad did not finish green; see the updated table in the Stage 2 comment. Re-run @qwen-code /triage after fixes. finalize run

⚠️ 延迟审批已搁置 —— 70256ad 有 1 个 PR CI workflow 未以绿色完成，详见 Stage 2 评论中已更新的表格。修复后可重新运行 @qwen-code /triage。查看 finalize 运行

[qwen-code-ci-bot]

Thanks for the follow-up — this one is easy to place, because both halves trace straight back to findings in the round-2 maintainer verification of #12126.

Template looks good ✓ — every section present, and the Chinese version is a real translation rather than an abridged one.

Problem: observed and measured, not theoretical. Finding S4 in that review timed result() on a real device against N granted fixture URIs sharing one authority and reported a median 55–369 ms at 100 URIs, attributed to one Binder call per URI. The reviewer's own verdict was "a short main-thread stall, nowhere near an ANR. A follow-up is fine," and a candidate authority-cache diff was attached. Finding S5 confirmed the documentation staleness with exact line numbers and prescribed the fix: "a one-line supersession pointer, like the one #12121 added." So this PR implements two things a maintainer explicitly asked for. Being plain about the scale, though: the reviewer characterized S4 as a modest main-thread stall, not a user-facing defect. This is maintenance of code that already merged, not a repair of something broken — which is fine, it just shouldn't be read as more than it is.

Direction: aligned. packages/mobile-shell is an in-repo package with its own CI lane, and this is a scoped response to review on already-merged code rather than a speculative addition. The reference CHANGELOG has no analogue here — its file-picker entries are all about the TUI @-mention picker — so the maintainer's own review is the direction signal, and it points this way.

Size: not applicable, no core paths. packages/mobile-shell/app/src/main/java/... does not match any of the protected packages/*/src/{auth,providers,models,config,tools,services} shapes, and the change stays inside one package plus docs. For the record: 6 production lines (NativeFilePicker.kt, +5/−1), 41 test lines, 4 doc lines, 0 generated/schema. Nowhere near any threshold.

Approach: the scope feels right and the diff is genuinely minimal — no drive-by refactors, no formatting churn, no unrelated edits. Every line serves the stated goal. Two honest observations, neither blocking:

• The cache half (S4) and the docs half (S5) are separate findings and could have been two PRs. At two lines per language I would not bother splitting it, and the body explains that both come from the same parent review.
• The title says fix(mobile): but nothing was broken — perf(mobile): would describe the code half more accurately. Cosmetic; it changes no gate outcome either way.

Risk: no elevated risk signals — none of the changed files match the revert-correlated paths. The one thing worth a reviewer's attention is that this implementation differs slightly from the candidate attached to S4, and uses a Kotlin construct that reads ambiguously even though it is correct. Both points are covered in the code review below.

Moving on to code review. 🔍

中文说明

感谢这个后续 PR——定位很清楚，因为两部分改动都能直接追溯到 #12126 第二轮维护者验证 中的具体结论。

模板完整 ✓ ——各节齐全，中文版是完整翻译，不是缩略版。

问题： 已观测且已实测，不是理论性加固。该评审的 S4 在真机上对同一 authority 下 N 个已授权 fixture URI 计时 result()，100 个 URI 时中位数 55–369 ms，归因于每个 URI 一次 Binder 调用。评审人自己的结论是"最大选择量下这只是短暂的主线程停顿，远达不到 ANR，做后续 PR 是可以的"，并附上了一个 authority 缓存的候选 diff。S5 则用确切行号确认了文档过时问题，并给出修法："一行 supersession 指引，参照 #12121 的做法。" 所以本 PR 实现的是维护者明确提出的两件事。不过规模也要说清楚：评审人把 S4 定性为轻量的主线程停顿，而非用户可见缺陷。这是对已合入代码的维护，不是修复坏掉的东西——这没问题，只是不应被读成比实际更重要的改动。

方向： 对齐。packages/mobile-shell 是仓库内的包，有独立 CI，本 PR 是对已合入代码评审意见的定向回应，而不是投机性新增。参考 CHANGELOG 在此没有对应项——其中 file picker 相关条目都是指 TUI 的 @ 提及选择器——因此方向信号来自维护者自己的评审，而评审是支持这个方向的。

规模： 不适用，未触及核心路径。packages/mobile-shell/app/src/main/java/... 不匹配任何受保护的 packages/*/src/{auth,providers,models,config,tools,services} 形态，改动也只在单个包加文档内。记录一下：生产代码 6 行（NativeFilePicker.kt，+5/−1）、测试 41 行、文档 4 行、生成/schema 0 行。远低于任何阈值。

方案： 范围合理，diff 确实做到了最小——没有顺手重构，没有格式化噪音，没有无关改动，每一行都服务于既定目标。两点如实说明，都不阻塞：

• 缓存部分（S4）和文档部分（S5）是两个独立结论，本可以拆成两个 PR。但每种语言只有两行，我倾向不必拆，正文也说明了两者出自同一次父评审。
• 标题写的是 fix(mobile):，但并没有东西是坏的——perf(mobile): 更准确地描述代码部分。纯属措辞，不影响任何门禁结论。

风险： 无升级风险信号——改动文件都不匹配与回滚相关的路径。唯一值得评审人留意的是：本实现与 S4 附带的候选 diff 略有不同，并且用了一个读起来有歧义（但行为正确）的 Kotlin 写法。这两点都在下面的代码审查中说明。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 70256adaa903619aa6241cd65242c5515937c65d · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review

No blocking issues. I read the whole of NativeFilePicker.kt and FilePickerDeviceTest.kt at the base commit, not just the diff hunks.

My own proposal before reading the diff was: memoize resolveContentProvider per authority in a map scoped to one validateResult call, and leave the per-URI scheme / authority / ownership / permission checks exactly where they are. That is what this does, and it also converges with the candidate attached to finding S4 — so there is no simpler path I can offer.

What I checked, and why I am satisfied:

• Cache lifetime. providers is a local val inside validateResult, so it cannot outlive one result. This is the only real hazard the change introduces and the PR names it itself; the repeated-selection test pins it by asserting the counter keeps climbing on the second pass rather than staying at 1.
• No check is relaxed. All four per-URI conditions survive untouched — content scheme, non-blank authority, provider not owned by our own UID, and checkUriPermission returning PERMISSION_GRANTED. Only the Binder resolution is memoized. The security-relevant half is pinned by the second new test: a document that shares the first document's provider but lacks its own grant still rejects the whole selection, with the permission counter at 2.
• getOrPut with a non-local return. This is the construct I spent the most time on, because it looks wrong and is not. kotlin.collections.getOrPut is inline with a plain (non-crossinline) lambda, so return null exits validateResult rather than the lambda; the put is never reached on that path, the map stays unpopulated, and the whole selection is rejected — identical to the base behaviour. Type-wise ProviderInfo? ?: return null collapses to ProviderInfo, which is what the map's value type wants. Compilation is settled empirically anyway: connectedDebugAndroidTest builds both source sets, and both device jobs are green.
• Docs. The added supersession pointer copies the pattern already in the same file for the Phase 2 connection-profile design — same position, same shape — and lands in the English and Chinese versions together. Both link targets (mobile-file-selection.md, mobile-file-selection.zh-CN.md) exist on main; I verified rather than assumed. It also correctly leaves the historical "not implemented here" and Follow-ups text alone, which is right for a document that explicitly records a Phase 1 baseline — S5 asked for a pointer, not a rewrite.
• Test instrumentation boundary. CountingContext counts getPackageManager(), which is a faithful proxy here: NativeFilePicker touches context.packageManager in exactly one place, inside validateResult. And every fixture URI is built from content://com.qwen.mobileshell.test.picker/<index>, so one authority across all 100 documents — the expected count of 1 is correct, not lucky.

Two things for the reviewer to weigh, neither blocking:

1. This differs from the S4 candidate in a way worth a deliberate choice. The candidate keeps a HashSet<String> of seen authorities and skips both the resolution and the own-UID comparison on a repeat, hoisting ownUid out of the loop. This PR caches the ProviderInfo itself and still performs the own-UID comparison for every URI. Both end up at exactly one Binder call per authority — the uid comparison is an in-memory field read, not IPC — so there is no performance difference. This version is marginally more conservative and matches what its description claims; it costs one extra import. Either is fine, but since a maintainer already wrote the other one, the choice should be conscious.
2. getOrPut hiding a non-local return is a readability trap. Correct, but a future reader can easily take that return null for a lambda return and conclude the map caches a miss. The candidate's if (checkedAuthorities.add(authority)) says the same thing with no such ambiguity, for three more lines. I would not block on it; I would not be surprised to see it changed in review.

One gap in coverage I want to state rather than bury: the cached path's own-UID rejection is not directly exercised. The existing providerOwnedByPickerContextIsRejectedDespiteReadGrant test uses a single URI, which always misses the cache. I do not think this is a defect — the cached value is the identical ProviderInfo object, so applicationInfo.uid cannot differ between the miss and the hit, and this PR keeps that comparison per-URI where the candidate drops it. Noting it so it is a known, reasoned gap rather than an unnoticed one.

Test evidence

Unattended CI run — I did not build or execute anything from this PR. Everything below is the PR's own CI, read through the API, plus static reading of the base tree.

One naming trap worth flagging, because it makes the coverage look narrower than it is: the jobs called "Keystore and profiles (API 26/36)" are the device jobs, and mobile-shell.yml runs them as ./gradlew connectedDebugAndroidTest with no class= filter. They execute the entire instrumentation suite, FilePickerDeviceTest included — not just keystore and profile cases.

The count corroborates that independently of anything in the log body. At the base commit the androidTest source set has exactly 24 @Test methods (FilePickerDeviceTest 17, ProfileDeviceTest 5, ProfileInitializationDeviceTest 2) — I counted them in the tree. The device logs on this head report Starting 26 tests on emulator-5554 - 8.0.0 and Starting 26 tests on emulator-5554 - 16, each followed by BUILD SUCCESSFUL. The +2 is precisely the two tests this PR adds, so both new cases were compiled and ran on real emulators at two API levels, and passed. connectedDebugAndroidTest fails the build on any test failure, so green here means green.

That matters for whether the tests carry weight: both new assertions fail on the base code. Without the cache, 100 same-authority documents produce 100 package-manager reads and the two-document case produces 2, while the tests assert 1 and 1. So this is not a suite that passes identically with and without the diff — it pins the change. The permission-count and rejection assertions, by contrast, pass before and after, which is the correct shape for a regression guard.

What the tests do not pin is wall-clock latency: they count package-manager accesses, not Binder transactions or milliseconds. The PR says so itself and lists UI latency under "Not validated," which is the right call. The author's local JVM harness numbers (100→1, 100→2, on Windows) are the author's claim and I did not re-run them — but nothing load-bearing rests on them, because CI's device suite asserts the same call counts on real Android at two API levels.

Test (ubuntu-latest, Node 22.x) was still in progress when I fetched; the macOS and Windows unit jobs and the CLI integration job are skipped by path filter, as expected for a change that touches no TypeScript. I did not poll. The table below is updated in place once CI settles.

Final CI results for 70256ad (auto-updated by the triage finalize job after CI completed):

Check	Conclusion
Test (ubuntu-latest, Node 22.x)	❌ failure
Build, unit tests, and lint	✅ success
Classify PR	✅ success
Desktop Shell (ubuntu-22.04)	✅ success
Desktop Shell (windows-2022)	✅ success
Integration Tests (no-AK, No Sandbox)	✅ success
Keystore and profiles (API 26)	✅ success
Keystore and profiles (API 36)	✅ success
Lint & Static (ubuntu-latest, Node 22.x)	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success

_{One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。}

The one claim CI cannot settle is the latency benefit, and it is arguably not worth settling: finding S4 already probed it on a real device, reported 55–369 ms at 100 URIs against 35–99 ms with the candidate cache, and explicitly declined to quote a precise saving because host load swamped the spread. If a maintainer does want an A/B on this head, @qwen-code /verify is available as a sponsored run (the author has read access only, so a maintainer's comment is what approves it) — it carries a pre-execution risk screen and a full workspace wipe, and its report should be read with the same skepticism as the fork's own CI logs, since the code under verification is adversarial input. @qwen-code /tmux does not apply: there is no TUI surface here, and it is unavailable to a fork author regardless.

中文说明

代码审查

没有阻塞问题。我读的是基线 commit 上 NativeFilePicker.kt 和 FilePickerDeviceTest.kt 的完整文件，不只是 diff 片段。

在看 diff 之前我自己的方案是：在 validateResult 单次调用的作用域内，用一个 map 按 authority 缓存 resolveContentProvider 的结果，其余每个 URI 的 scheme / authority / 归属 / 权限检查原地保留。本 PR 正是这么做的，也和 S4 附带的候选 diff 收敛到同一思路——所以我拿不出更简单的路径。

已核实且认可的部分：

• 缓存生命周期。 providers 是 validateResult 内部的局部 val，不可能存活超过一次结果处理。这是本改动唯一真正的隐患，PR 自己也点明了；连续选择测试通过断言第二次计数器继续递增（而不是停在 1）把这一点固定住了。
• 没有放宽任何检查。 四个逐 URI 条件全部原样保留——content scheme、authority 非空、provider 不属于本 UID、checkUriPermission 返回 PERMISSION_GRANTED。被缓存的只有 Binder 解析。安全相关的那一半由第二个新测试固定：与第一个文档同 provider、但自身没有授权的文档，仍然会拒绝整个选择，且权限计数器为 2。
• getOrPut 配合非局部 return。 这是我花最多时间的地方，因为它看着像错的、其实不是。kotlin.collections.getOrPut 是 inline，lambda 参数不是 crossinline，所以 return null 退出的是 validateResult 而不是 lambda；这条路径上 put 永远不会执行，map 保持未写入，整个选择被拒绝——与基线行为完全一致。类型上 ProviderInfo? ?: return null 收敛为 ProviderInfo，正是 map 值类型所需。编译问题也已由实证解决：connectedDebugAndroidTest 会构建两个 source set，而两个设备 job 都是绿的。
• 文档。 新增的 supersession 指引沿用了同一文件中已有的 Phase 2 连接配置设计的写法——位置相同、形态相同——并且中英文同步落地。两个链接目标（mobile-file-selection.md、mobile-file-selection.zh-CN.md）在 main 上都存在，我是核实过的，不是假设。它也没有去改动历史上"尚未实现"和 Follow-ups 的正文，这对一份明确记录 Phase 1 基线的文档是正确处理——S5 要的是一个指引，不是重写。
• 测试的计数边界。 CountingContext 统计 getPackageManager()，在这里是可靠的代理指标：NativeFilePicker 只在一个地方访问 context.packageManager，就在 validateResult 内。而所有 fixture URI 都由 content://com.qwen.mobileshell.test.picker/<index> 构造，100 个文档共用一个 authority——所以期望值 1 是正确的，不是碰巧。

两点请评审人自行权衡，都不阻塞：

1. 本实现与 S4 候选 diff 的差异值得做一次有意识的选择。 候选版本用 HashSet<String> 记录已见 authority，重复时同时跳过解析和 own-UID 比较，并把 ownUid 提到循环外。本 PR 缓存 ProviderInfo 本身，并且对每个 URI 仍然执行 own-UID 比较。两者最终都是每个 authority 恰好一次 Binder 调用——uid 比较是内存字段读取，不是 IPC——所以性能上没有差别。本版本略更保守，也与其描述一致；代价是多一个 import。两种都可以，但既然维护者已经写过另一种，这个选择应该是有意识的。
2. getOrPut 里藏一个非局部 return 是可读性陷阱。 行为正确，但后来的读者很容易把那个 return null 当成从 lambda 返回，进而以为 map 会缓存"未命中"。候选版本的 if (checkedAuthorities.add(authority)) 表达的是同一件事，没有这种歧义，只多三行。我不会因此阻塞，但如果评审中被改掉我也不会意外。

有一处覆盖缺口我想明说而不是埋起来：缓存命中路径上的 own-UID 拒绝没有被直接覆盖。已有的 providerOwnedByPickerContextIsRejectedDespiteReadGrant 用单个 URI，必然缓存未命中。我不认为这是缺陷——缓存的是同一个 ProviderInfo 对象，命中与未命中时 applicationInfo.uid 不可能不同，而且本 PR 把这个比较保留为逐 URI 执行，候选版本反而是跳过的。写出来是为了让它成为一个已知的、有理由的缺口，而不是被忽略的缺口。

测试证据

无人值守 CI 运行——我没有构建或执行本 PR 的任何代码。以下内容全部来自通过 API 读取的 PR 自身 CI，加上对基线代码树的静态阅读。

有一个命名陷阱需要指出，因为它让覆盖面看起来比实际窄：名为 "Keystore and profiles (API 26/36)" 的就是设备 job，而 mobile-shell.yml 是用 ./gradlew connectedDebugAndroidTest 跑的，没有 class= 过滤。它们执行的是完整的 instrumentation 测试套件，包含 FilePickerDeviceTest——不只是 keystore 和 profile 用例。

测试数量也独立佐证了这一点，且不依赖日志正文的任何说法。基线 commit 上 androidTest source set 恰好有 24 个 @Test 方法（FilePickerDeviceTest 17 个、ProfileDeviceTest 5 个、ProfileInitializationDeviceTest 2 个）——这是我在代码树里数出来的。本 head 的设备日志报告 Starting 26 tests on emulator-5554 - 8.0.0 与 Starting 26 tests on emulator-5554 - 16，随后都是 BUILD SUCCESSFUL。多出的 2 个正是本 PR 新增的两个测试，所以两个新用例都已编译并在两个 API 级别的真实模拟器上运行且通过。connectedDebugAndroidTest 在任一测试失败时都会让构建失败，所以这里的绿色就是真的绿色。

这一点对判断测试是否有分量很关键：两个新断言在基线代码上都会失败。没有缓存时，100 个同 authority 文档会产生 100 次 package manager 读取，两文档场景会产生 2 次，而测试断言的是 1 和 1。所以这不是一个"有没有 diff 都同样通过"的套件——它确实固定住了这次改动。相比之下，权限计数和拒绝断言在改动前后都通过，这对回归防护来说是正确的形态。

测试没有固定的是墙钟延迟：它们统计的是 package manager 访问次数，不是 Binder 事务数或毫秒数。PR 自己也这么说了，并把 UI 延迟列在"未验证"下，这个处理是对的。作者本地 JVM harness 的数字（100→1、100→2，Windows 上）是作者的说法，我没有重跑——但没有任何关键结论依赖它，因为 CI 的设备套件在两个 API 级别的真实 Android 上断言了同样的调用次数。

我拉取数据时 Test (ubuntu-latest, Node 22.x) 仍在运行；macOS 和 Windows 的单测 job 以及 CLI 集成 job 因路径过滤被跳过，对一个不触及 TypeScript 的改动来说符合预期。我没有轮询等待。CI 结束后下方表格会被就地更新。

CI 无法定论的只有延迟收益，而这个收益可以说也不值得专门去定论：S4 已经在真机上探测过，报告 100 个 URI 时 55–369 ms、换成候选缓存后 35–99 ms，并明确拒绝给出精确节省值，因为主机负载把差异淹没了。如果维护者确实想在这个 head 上做 A/B，@qwen-code /verify 可以作为受赞助运行使用（作者只有读权限，因此需要维护者评论来批准）——它带有执行前风险筛查和完整工作区清除，但其报告应当以与 fork 自身 CI 日志同样的怀疑态度阅读，因为被验证的代码是对抗性输入。@qwen-code /tmux 不适用：这里没有 TUI 界面，而且它对 fork 作者本就不可用。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 70256adaa903619aa6241cd65242c5515937c65d · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 4/5 — clean across every stage; the only nits are cosmetic and I name them below.

Stepping back: this PR does one small thing and does it properly. Six production lines, no behaviour change, and it closes two findings a maintainer wrote down explicitly and left open. My own proposal before reading the diff was identical to what the author wrote, and also to the candidate the maintainer attached to S4 — three independent convergences on the same six lines is a good sign that there is no better shape for this change.

I went in expecting to find a security regression, because caching inside a validation loop is exactly where one would hide, and I could not find one. The four per-URI conditions all survive; only the Binder resolution is memoized; the map is a local val so it cannot leak across picker requests; and the test that would catch the obvious mistake — a second document sharing the first document's provider but lacking its own grant — exists, asserts the rejection, and asserts that the permission check still ran twice. That is the right test to have written, and it was written.

What I like most is that the evidence ended up stronger than the PR claimed. The body leans on a local Windows JVM harness with controlled Android stubs, which on its own would be thin for a fork PR touching a permission-checking path. It did not have to carry the weight: CI runs connectedDebugAndroidTest with no class filter, the androidTest source set has exactly 24 @Test methods at base and the device logs report 26 on this head, so both new cases ran on real emulators at API 26 and API 36. And they fail on the base code, which is what makes them worth having — a suite that passes identically with and without the diff would have proven nothing here.

My reservations, both non-blocking:

• getOrPut with a non-local return null in its lambda is correct but reads as though it might cache a miss. The maintainer's HashSet candidate says the same thing three lines longer and with no such trap. If this changes in review, that is why.
• The PR differs from that candidate in keeping the own-UID comparison per URI instead of skipping it on a repeat. That is the more conservative of the two and costs nothing measurable, but since a maintainer already wrote the other version, the choice ought to be deliberate rather than incidental.

The honest framing on value: the reviewer who measured this called it "a short main-thread stall, nowhere near an ANR." Nobody will notice this change. If the bar were "only merge what users can feel," it would not clear it. But it retires two explicit review findings at close to zero risk, keeps a design document from continuing to misdescribe shipped behaviour, and adds tests that pin both the optimization and its lifetime — which is a legitimate reason to merge, and the kind of follow-up that is easy to promise in review and rare to actually see land.

Approving. Because Test (ubuntu-latest, Node 22.x) is still running on this head, approval is deferred until CI lands green on 70256adaa903619aa6241cd65242c5515937c65d — I am not going to attest to a result that does not exist yet. If anything lands red or the head moves, the deferred approval is withheld rather than carried over. The pending job is the Node unit suite, which this change cannot affect (no TypeScript is touched), so this is a formality rather than a doubt.

中文说明

Confidence: 4/5 —— 各阶段都干净；仅有的几点都是措辞层面的，下面点名说明。

退一步看整体：这个 PR 只做了一件小事，而且做得规范。6 行生产代码，无行为变化，并且关闭了维护者明确写下、一直未处理的两个结论。我在看 diff 之前自己的方案与作者写的完全一致，也与维护者在 S4 附上的候选 diff 一致——三方独立收敛到同样的 6 行，说明这个改动已经没有更好的形态了。

我原本预期会找到安全回归，因为在校验循环里加缓存正是最容易藏问题的地方，但没找到。四个逐 URI 条件全部保留；被缓存的只有 Binder 解析；map 是局部 val，不可能跨 picker 请求泄漏；而能够抓住那个最明显错误的测试——第二个文档与第一个同 provider 但自身没有授权——确实存在，断言了拒绝，也断言了权限检查仍然执行了两次。这是本该写的测试，而且写出来了。

我最欣赏的一点是：证据最终比 PR 自己声称的更强。正文依赖的是本地 Windows JVM harness 加受控 Android 桩，单凭这一点，对一个触及权限校验路径的 fork PR 来说是单薄的。但它并不需要承担这个重量：CI 用不带 class 过滤的 connectedDebugAndroidTest 运行，基线上 androidTest source set 恰好有 24 个 @Test 方法，而本 head 的设备日志报告 26 个，所以两个新用例都在 API 26 和 API 36 的真实模拟器上跑过了。而且它们在基线代码上会失败，这正是其价值所在——一个有没有 diff 都同样通过的套件，在这里什么也证明不了。

我的两点保留意见，均不阻塞：

• getOrPut 的 lambda 里放一个非局部 return null，行为正确，但读起来像是会缓存"未命中"。维护者的 HashSet 候选版本多三行，表达同一件事却没有这个陷阱。如果评审中被改掉，原因就是这个。
• 本 PR 与该候选版本的差别在于：它对每个 URI 都保留 own-UID 比较，而候选版本在重复 authority 时跳过。两者中本 PR 更保守，且代价无法测量，但既然维护者已经写过另一种写法，这个选择应该是有意识的，而不是顺带的。

关于价值的如实定性：实测过这一点的评审人称之为"短暂的主线程停顿，远达不到 ANR"。没有人会注意到这个改动。如果标准是"只合入用户能感知到的东西"，它过不了。但它以接近零的风险结清了两个明确的评审结论，阻止一份设计文档继续错误描述已经发布的行为，并补上了同时固定住优化本身与其生命周期的测试——这是合入的正当理由，也正是那种在评审里容易承诺、实际很少看到落地的后续工作。

同意合入。由于 Test (ubuntu-latest, Node 22.x) 在本 head 上仍在运行，批准推迟到 CI 在 70256adaa903619aa6241cd65242c5515937c65d 上全绿之后——我不会为一个尚不存在的结果背书。如果任何 job 变红或 head 移动，这个推迟的批准会被撤回而不是顺延。仍在运行的是 Node 单测套件，本改动不可能影响它（没有触及任何 TypeScript），所以这是流程性动作，而不是存在疑虑。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 70256adaa903619aa6241cd65242c5515937c65d · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Reviewed. Suggestions are inline.

Not explored to full depth (tool budget reached): "agent 6b": none — no check was cut short by the tool ceiling..

Test Plan (not a blocker): ./gradlew — no such file or directory.

中文说明

已审查。 建议见行内评论。

未探索到全部深度（达到工具调用预算）："agent 6b"：none — no check was cut short by the tool ceiling.。

Test Plan（非阻断）：./gradlew — no such file or directory。

— qwen3.8-max via Qwen Code /review (v0.24.7)

[qwen-code-ci-bot]

Reviewed. Suggestions are inline.

Not explored to full depth (tool budget reached): "agent 6a": none — I could not execute the Gradle androidTest suite (no device/emulator in this environment), so "tests pass on hardware" is reasoned from the implementat….

Test Plan (not a blocker): ./gradlew — no such file or directory.

中文说明

已审查。 建议见行内评论。

未探索到全部深度（达到工具调用预算）："agent 6a"：none — I could not execute the Gradle androidTest suite (no device/emulator in this environment), so "tests pass on hardware" is reasoned from the implementat…。

Test Plan（非阻断）：./gradlew — no such file or directory。

— qwen3.8-max via Qwen Code /review (v0.24.7)

[qwen-code-ci-bot]

⚠️ Downgraded from Approve to Comment: CI failing: web-shell E2E Smoke (ubuntu-latest, Node 22.x). Reviewed.

Not explored to full depth (tool budget reached): "agent 1e": none — all checks above completed within budget (~9 of 42 calls)..

Test Plan (not a blocker): ./gradlew — no such file or directory.

中文说明

⚠️ 已从批准降级为评论：CI failing: web-shell E2E Smoke (ubuntu-latest, Node 22.x)。 已审查。

未探索到全部深度（达到工具调用预算）："agent 1e"：none — all checks above completed within budget (~9 of 42 calls).。

Test Plan（非阻断）：./gradlew — no such file or directory。

— qwen3.8-max via Qwen Code /review (v0.24.7)

[jabrailkhalil]

I checked the browser smoke log: this job was cancelled at its 30-minute limit, rather than completing with an Android picker assertion failure. Chromium/WebKit installation consumed 15m34s, and the browser smoke started about 22m35s into the job before cancellation during execution. Run/job log.

The native picker checks pass on API 26 and API 36, and the latest code review reports no findings. I cannot rerun upstream Actions with the repository permissions available to my account. Please rerun the timed-out browser job when reviewing; I am keeping CI timeout/download changes outside this picker fix.

[wenshao]

Local verification of #13147 — verdict: merge-ready

264/264 scripted assertions passed across 6 harness arms (base / head / 4 mutants). Verified head: 29a21f16ce2713abc89e55d2895b89eb5c71421e, base: a7deb01bcbd5a5c795bfccbf2bd0b4e445286b92.

中文摘要

结论：可以合并。 本地双臂 A/B 验证了核心声明：同一 authority 的 100 个文档，改动前解析 provider 100 次，改动后 1 次（混合双 authority 为 2 次），且每个 URI 的权限检查一次不少（100 次保留）。S3 证明缓存仅存在于单次选择内（连续两次选择 → 2 次解析，而非 1 次）。变异矩阵：无 authority 键的单槽缓存（m1）会放过同 UID 危险 provider 的文档（S11 被错误接受）， hoist PackageManager 但不缓存（m2）在混合 authority 下计数不符——两者恰被新增的 mixed-authority 测试钉住，与作者声明一致；m3（去掉 UID 检查）/m4（每 authority 只查一次授权）分别被既有安全性与逐文档授权场景击杀，构成阳性对照。另核实：两份设计文档链接目标均存在且中英对称；head 的 CI 在 API 26/36 模拟器上 29 项设备测试全部通过。未覆盖：本地 Gradle 全量构建与真机/模拟器执行（本机无 Android SDK，已由 CI 覆盖）、第三方 provider 行为、UI 延迟。详细数字见下方表格。

Central claim and A/B

Claim: validating a picker result resolves each document-provider authority once per selection while keeping every URI permission check; the cache lives only for one validation call.

Method: compiled the real NativeFilePicker.kt (extracted byte-identical from the two OIDs; the only production file the PR changes) with Kotlin 1.9.20 — the version pinned in packages/mobile-shell/build.gradle.kts — against faithful Android API stubs, and drove the real open() → result() flow with a counting Context/PackageManager. Metrics per cell: pm = getPackageManager() reads (the metric the checked-in CountingContext pins), res = actual resolveContentProvider calls, perm = checkUriPermission calls, acc = delivered URIs (null = rejected).

scenario	base a7deb01b	head 29a21f16
S1: 100 docs, one authority, all granted	pm=100 res=100 perm=100 accepted	pm=1 res=1 perm=100 accepted
S2: 100 docs, two authorities (50/50)	pm=100 res=100 perm=100 accepted	pm=2 res=2 perm=100 accepted
S3: same picker, two selections of 100	pm=200 perm=200	pm=2 perm=200 (cache is per-selection)
S4: 2nd doc lacks grant, same authority	rejected, pm=2 perm=2	rejected, pm=1 perm=2
S5: 2nd authority's doc lacks grant	rejected, pm=2 perm=2	rejected, pm=2 perm=2
S6: same-UID provider	rejected, perm=0	rejected, perm=0
S7: unknown authority	rejected, res=1	rejected, res=1
S8: file:// scheme	rejected, res=0	rejected, res=0
S9: 101 documents	rejected, res=0	rejected, res=0
S10: duplicate URIs deduped, order kept	[0,1], pm=2	[0,1], pm=1
S11: granted doc from same-UID 2nd authority	rejected, pm=2 perm=1	rejected, pm=2 perm=1

Evidence: 01-ab-base-vs-head.png (full run output for both arms).

Mutation matrix (vacuity check on the new tests)

Four single-point mutants of the head file were compiled and driven through the identical 11 scenarios; each behaves exactly as predicted, and each diverges from head on at least one cell the checked-in test class pins (evidence: 02-mutation-matrix.png).

mutant	diverging cells vs head	consequence	pinned by
m1: one-slot cache, no authority key	S5 pm 2→1; S11 accepted the same-UID provider's granted document	wrong provider's UID check applied across authorities — a real security hole, not just a count	secondAuthorityInOneSelectionIsResolvedSeparately (pm=2)
m2: packageManager hoisted, no cache	S1 res=100; S5 pm 2→1	silently loses the whole optimization; invisible to the same-provider count (pm stays 1)	the mixed-authority test's count of 2
m3 (positive control): UID check removed	S6 accepted; S11 accepted	same-UID providers accepted	pre-existing unsafe-provider tests
m4: grant checked once per authority	S4 accepted ungranted doc; S1 perm 100→1	per-document grant lost	cachedProviderDoesNotAllowAnotherDocumentWithoutItsOwnGrant (perm=2)

m3 kills prove the harness can fail (it does, on the exact cells), so the green cells are evidence, not a dead harness. This matches the PR's own R1-1 mutation claims and sharpens them: m1 is not merely a count mismatch, it accepts a document from a provider owned by the app's own UID.

Corrections to earlier review comments

• Three bot review rounds noted ./gradlew: no such file or directory under "Test Plan". The wrapper exists at packages/mobile-shell/gradlew; the Reviewer Test Plan's "From packages/mobile-shell" prefix is required. Description is fine; the bot ran from the wrong cwd.
• The CountingContext comment "The picker obtains PackageManager only when resolving a provider" holds at head: context.packageManager has exactly one access site in NativeFilePicker.kt (inside the getOrPut lambda), verified by grep.

Other checks

• Docs: both new lines link to mobile-file-selection.md / mobile-file-selection.zh-CN.md; both targets exist at head, and the EN/ZH additions are symmetric.
• Test surface: FilePickerDeviceTest.kt goes 17 → 22 @Test methods (+5, matching the diff); the fixture provider registers both authorities in the androidTest manifest.
• Upstream CI at head (already green, cited not re-run): "Build, unit tests, and lint" pass; emulator jobs "Keystore and profiles" API 26 and API 36 each report Starting 29 tests + BUILD SUCCESSFUL, i.e. the real instrumentation suite — including the 5 new tests — passed on both API levels.
• The failing web-shell E2E Smoke check on this PR is unrelated to this diff (touches only packages/mobile-shell and docs/design).

Findings

None blocking. One informational observation: a hypothetical m2-style regression (hoisting packageManager without caching resolutions) is invisible to the same-provider test's pm count alone and is caught only by the mixed-authority test — which is exactly why that test was added in this PR. Coverage is adequate as shipped.

Not covered

• Local full Gradle build (assembleDebug/testDebugUnitTest/lintDebug) and local emulator run: this machine has no Android SDK. Compilation fidelity is limited to kotlinc 1.9.20 + stubs for the single changed production file; the androidTest sources were not compiled locally (they are compiled and executed by the two upstream emulator CI jobs cited above).
• The checked-in device tests were not re-executed through JUnit-on-JVM locally (the author reports doing so); my harness re-implements their scenarios against the real production class instead.
• Third-party provider behavior, Binder latency, UI timing.
• Per-commit attribution: the PR head is a merge commit; verification used the effective base..head tree diff of the changed file.

Methodology

macOS (arm64), JBR 17.0.10, Kotlin compiler 1.9.20 (embeddable jar, matching the project's pinned Kotlin version). Sources extracted with git show <oid>:...NativeFilePicker.kt (base extraction diffed against the PR's minus-side — identical). Each arm compiles the same stubs + same harness + that arm's NativeFilePicker.kt in one kotlinc invocation, then runs harness.HarnessKt <arm>; arm-aware expectations make every cell a scripted pass/fail (11 scenarios × 4 metrics × 6 arms = 264 assertions, all executed). Stubs count getPackageManager() reads, resolveContentProvider calls, and checkUriPermission calls; grants and provider UID registry are explicit fixtures. Raw logs: tmp/pr13147-verify-20261002-224613/logs/; harness, stubs, and mutants: tmp/pr13147-verify-20261002-224613/src/.

[jabrailkhalil]

@wenshao Thank you for the detailed independent verification and mutation matrix. The mixed-authority case is useful evidence that the cache retains the provider UID boundary as well as reducing lookups. I will keep the verified head 29a21f16 unchanged. The outstanding Web Shell smoke job stopped at its 30-minute limit; I linked the timings and rerun request above. I am ready to address any further findings from the required review or completed CI run.

[wenshao]

@qewn-code /triage

[jabrailkhalil]

@wenshao The independently verified head is still 29a21f1; I have not changed it. I noticed the latest triage mention says @qewn-code, so if a rerun was intended, the trigger is @qwen-code /triage. The Android checks passed, while the Web Shell smoke run remains cancelled after its time limit. Could you retry the intended triage trigger and the smoke check on this head when convenient? I will address any confirmed current-head finding.