# 审查问题处理

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

1. **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

1. **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.