审查问题处理

Important已修复：两所学校页面完成回调传入代次与生命周期有效性检查，异步URL读取及脚本返回后复核；保持同URL并让代次失效的回归测试观察到RED→GREEN。Minor已修复：beangle=null不再判断为库已加载，现有Node脚本测试覆盖null值。未要求再次审查；完整测试、静态检查与原生验证由主实现者完成并记录。以下为修复前独立审查及当时处理计划。

Browser diagnostics 0.7.2 review

Reviewed the uncommitted mobile changes against a4c2df838efe01f39fb27cc6062544a476d86949, including the new diagnostics implementation, Dart tests, JavaScript guard test, and Android fixture. No original user logs or key files were opened. This is a read-only source review; no full tests were repeated and no application source was changed.

Strengths

Console messages become fixed category labels. The snapshot decoder exports only bounded integer counts and explicitly selected booleans, discarding unknown fields.
The JavaScript reads neither page text nor resource URLs, cookies, storage, forms, or HTML. Password-page and URL guards fail closed; inspection exceptions are swallowed.
Diagnostic capture is launched independently of the login/import callback and changes no navigation policy, import parser, or school security settings.
Export headers identify app version/build and the UTC export time. The fixture checks real WebView behavior while asserting private fixture text is absent from exported diagnostics.

Issues

Critical

None found.

Important

URL equality cannot reject a snapshot from a replaced document with the same URL. mobile/lib/features/school/school_browser_diagnostics.dart:110-118; callback sites mobile/lib/features/school/school_session.dart:112-114 and mobile/lib/features/school/undergraduate/session.dart:142-144.

Capture awaits both native URL reads and JavaScript execution. If a portal document reloads at the same URL while one of these operations is pending, all three URL comparisons still pass. In particular, JavaScript may return the old document's counts, onPageStarted can advance the session document generation, and the final currentUrl() can still equal the original URL. Those old counts are then appended after the new navigation. This violates the requested different-document guard and can make the diagnostic evidence misleading even though it does not change imported data.

Pass a captured document generation plus a current-generation predicate (or equivalent) into diagnostics, and recheck after awaits and immediately before recording. The session already maintains _pageGeneration / _document. Include lifecycle validity where relevant. Add a focused asynchronous test that holds the JavaScript result, advances the document generation while keeping the URL unchanged, then verifies that no events are recorded. Existing tests exercise a different URL but do not cover this case.

Minor

A null Beangle global is reported as a loaded library. mobile/lib/features/school/school_browser_diagnostics.dart:40.

JavaScript reports typeof null === 'object', so window.beangle = null produces beangle: true. Require a non-null value in addition to the object type and cover the null case in the JavaScript test. This is a small diagnostic accuracy issue, not a login failure.

Declined to judge

The cause and resolution of the authenticated portal's styling/script failure: real affected-page evidence is absent from this change, and this release only refines diagnostic evidence.
Whether the queued complete Dart suite or native fixture passes: the coordinator is running them; this review makes no independent runtime success claim.
Untracked output artifacts: excluded from application-source review and not opened because they could include original user material.

Assessment

Ready to merge: With fixes. Privacy boundaries and isolation from login/import behavior are sound in the inspected code. Address the document-generation gap before claiming that snapshots reject replaced documents; do not describe this diagnostic refinement as a fix for the portal rendering problem.

Coordinator disposition

The coordinator accepted the Important finding and plans to pass an isCurrent predicate from each native page-finished callback: graduate capture will use _pageGeneration and route/activity guards, and undergraduate capture will use _generation, _document, and _valid. The predicate will be checked around asynchronous native capture. A focused regression will keep the URL unchanged while invalidating the generation during a pending JavaScript result. This is the planned disposition, not an independently verified fix; no second review was requested.

4.7 KiB Raw Permalink Blame History