docs(review): write back what actually shipped, and what did not

Both audit docs had drifted from the code, and a stale status doc is worse than
none: two closed SEV1s were read as open during a later pass, and one of them
had been fixed in another repo weeks earlier.

`OPEN-FINDINGS.md` claimed its contents were uncommitted work in the tree, on a
`main` that has since moved several hundred commits. Three items it listed as
open were re-checked in the source and are closed — D1 (both native receivers
route `input` now, via `routeInput` + `BridgeInputMapper`), D2's first SEV2 (the
unreachable `try/catch` is gone), D3 (the SRI guard shipped on both sides). Each
is annotated where it sits, not only in the new summary, because a reader lands
mid-document. What genuinely remains is named: D2's `controller.ts` global slot,
which is owed one clean §6 round, and electron's `flush()` not waiting for
renderers.

`cli-js-flows.md` recorded resolutions for 3 of its 8 findings while 5 had moved:
§4 dry-run and §8 strip shipped in CLI 0.7.11, §7's guard shipped on both sides
(with the two SEV1s found reviewing the first cut, both of which looked correct),
§5 became issue #8, and §3's PREMISE was disproven rather than fixed — a debug-id
on a map-less bundle is what lets the worker report `missing_sym` and name the
artefact nobody uploaded, so skipping those bundles would trade a diagnostic for
a few bytes.

§7 and §3 also record what is still open, so neither reads as finished: stamping
during `processAssets` is still the only fix correct for lazy-chunk hashes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Author: krassx

docs/review/OPEN-FINDINGS.md

@@ -1,9 +1,37 @@
 # Open findings — fix wave of 2026-08-27/28
 
-Live status doc for the in-flight fix + convergent-review cycle, in the shape of
-`docs/review/REMEDIATION-PLAN.md`. **Read this before touching the working tree.**
+Live status doc for the fix + convergent-review cycle, in the shape of
+`docs/review/REMEDIATION-PLAN.md`.
 
-## State at hand-off
+## Status — re-verified 2026-09-22 against the code, not against this file
+
+**Everything in this document is COMMITTED and on `main`.** The hand-off note below is kept for the
+record but no longer describes reality: it says the work is uncommitted, and `main` has since moved
+several hundred commits past `36ec616`. `git status` is clean.
+
+Three items were re-checked in the source because this file still listed them as open. All three are
+**closed**, and each is annotated in place below:
+
+| item | this file said | the code says |
+|---|---|---|
+| **D1** WebView `input` dropped by both native receivers (SEV1) | blocked on a human decision | **CLOSED** — `BridgeCaptureRouter.java` routes `input` via `routeInput`, and `BGSBridgeCaptureRouter.m:223` does the same on iOS. Both landed, with `BridgeInputMapper` mapping page coordinates into the native frame. |
+| **D2** first SEV2 — unreachable `try/catch` around `getAttributes()` | still unfixed | **CLOSED** — `server-instrument.ts:517-520` is a direct read now, with a comment giving the reason no catch is needed. |
+| **D3** source-map stamping breaks SRI (SEV1) | reproduced, unfixed | **CLOSED** — guards shipped on both the CLI (0.7.11) and the plugin. See `docs/review/cli-js-flows.md` §7, which carries the detail and the one part still open. |
+
+**Still genuinely open**, and the reason this file is worth keeping:
+
+- **D2's second SEV2** — `performance/src/controller.ts`'s module-global `active` slot. Partly closed:
+  `activeSpanStore` was built for the node server path (see "D2 part 2" below). The last review round
+  (2026-09-15) fixed 2 SEV2 + 8 SEV3 and **no fresh round has run since**, so CLAUDE.md §6 convergence
+  is owed one clean pass before this can be called done.
+- **`electron/src/launch-main.ts:203-204`** — `flush()` does not wait for renderers (marked OPEN (minor)
+  below, still true). Carrying a deadline is a bridge wire change.
+
+A stale status doc is worse than no status doc: two of the three closed items above were read as open
+SEV1s during a later pass, and one of them had been fixed in another repo weeks earlier. If you close
+something listed here, say so here.
+
+## State at hand-off (2026-08-28 — superseded, kept for the record)
 
 Everything below is **uncommitted work in the working tree** on `main` (80 entries in
 `git status`, last commit `36ec616`). Nothing has been committed or pushed. The tree is
@@ -225,9 +253,17 @@ it can only keep more markers, never delete more. No configuration can hold mark
 
 ---
 
-## Blocked on a human decision
+## Blocked on a human decision — D1 and D3 since CLOSED; only D2's second half remains
 
-### D1 · WebView `input` stream is dropped by both native receivers — **SEV1**
+### D1 · WebView `input` stream is dropped by both native receivers — ~~**SEV1**~~ **CLOSED 2026-09-22**
+
+> **Closed — both receivers landed the branch.** Android: `BridgeCaptureRouter.java` dispatches
+> `"input"` to `routeInput`, which maps each point through `BridgeInputMapper` (viewport CSS px →
+> native screen coordinates ÷ density) before handing it to the native input consumer. iOS:
+> `BGSBridgeCaptureRouter.m:223` — `else if ([fileType isEqualToString:@"input"]) [self routeInput:entry];`.
+> Neither option below was taken in the end: the wire change was completed rather than reverted. The
+> original text is kept because the mapper's existence is the reason `@bugsee/webview` must NOT emit
+> its own `video.aux` — see `packages/webview/src/launch.ts`.
 
 `@bugsee/webview` moved off `events.user` onto the new `input` stream (correct: SDK code must
 never write to a `*.user` stream). Both native routers are `if/else if` chains with **no
@@ -257,8 +293,11 @@ app's own `client.addBreadcrumb()` calls have always streamed over the bridge.
 
 Confirmed independently by two reviewers, still unfixed:
 
-- `packages/node/src/server-instrument.ts:479-483` — a `try/catch` around `getAttributes()`
-  that is **unreachable in production** (`getAttributes` is a required, non-throwing member of
+- **CLOSED 2026-09-22.** `server-instrument.ts:517-520` reads `getAttributes()` directly now, with a
+  comment stating why no defensive catch belongs there (it is a required, non-throwing member of
+  `Span`, and an outer catch already covers a hostile Transaction). The original finding follows.
+  ~~`packages/node/src/server-instrument.ts:479-483` — a `try/catch` around `getAttributes()`
+  that is **unreachable in production**~~ (`getAttributes` is a required, non-throwing member of
   `Span`, and `finishWith` already sits inside an outer catch). What it actually accommodates is
   **nine `Transaction` test doubles across seven adapter packages** that omit the method, so
   every one of their `setName` assertions now passes via the degraded path and **F-4 is untested
@@ -274,7 +313,14 @@ Confirmed independently by two reviewers, still unfixed:
 
 ---
 
-### D3 · Source-map stamping breaks Subresource Integrity — **SEV1**, reproduced 2026-09-18
+### D3 · Source-map stamping breaks Subresource Integrity — ~~**SEV1**~~ **CLOSED 2026-09-19**
+
+> **Closed — the detect-and-refuse guard shipped on both sides**, the CLI (`bugsee-cli` 0.7.11,
+> `src/inject/sri.rs`, `--allow-sri` to override) and the plugin
+> (`bundler-plugin-core/src/sri.ts`). Both, not one: a user driving the CLI by hand is not covered by a
+> plugin-side check. `docs/review/cli-js-flows.md` §7 carries the detail, the two SEV1s found while
+> reviewing the first cut, and the part that is still open — stamping during `processAssets`, before
+> the hashes are computed, which is the only fix correct for runtime-embedded lazy-chunk hashes.
 
 `@bugsee/bundler-plugin-core` runs `bugsee-cli sourcemaps inject` at webpack's `afterEmit` /
 unplugin's `writeBundle`, which appends bytes to every emitted `.js` AFTER the bundler computed its

docs/review/cli-js-flows.md

@@ -56,7 +56,7 @@ design and only a genuine failure fails the build.
 `@bugsee/bundler-plugin-core` passes it — except under `failOnError`, where a team that asked for strictness
 still hears that their build produced no maps at all. A path that does not exist stays an error either way.
 
-### 3. `inject` stamps every bundle, including ones with no map (SEV3, measured)
+### 3. `inject` stamps every bundle, including ones with no map (SEV3, measured) — **PREMISE DISPROVEN, closed 2026-09-19**
 
 `inject_paths` walks every `.js`/`.cjs`/`.mjs` and appends the runtime registration whether or not the
 bundle has a paired map. Measured on a real `next build` (`productionBrowserSourceMaps: true`):
@@ -68,7 +68,15 @@ Costs: pointless bytes in every server bundle, third-party files modified, and a
 with no map is exactly the case where the id is useless, so skipping it (or an `--exclude` glob, or
 skipping `node_modules`) is free.
 
-### 4. The whole flow cannot be dry-run (SEV3, verified in code + measured earlier)
+**Closed 2026-09-19 — the premise above is wrong, and the finding is withdrawn rather than fixed.** "A
+bundle with no map has a useless id" does not hold: the worker treats a debug-id it cannot resolve as
+`missing_sym` (`crash/javascript.py:406,467`), which is how a crash tells the operator WHICH artefact
+was never uploaded. Strip the id from those bundles and the frame arrives anonymous — the backend can no
+longer name what is missing, and the most common real cause (server maps that were never turned on, §8)
+becomes invisible. The stamped bytes are the price of that diagnostic. `--exclude` was still added for
+callers who want it, but skipping map-less bundles by DEFAULT would trade a diagnostic for a few bytes.
+
+### 4. The whole flow cannot be dry-run (SEV3, verified in code + measured earlier) — **SHIPPED (CLI 0.7.11 + `@bugsee/bundler-plugin-core`)**
 
 `sourcemaps inject --dry-run` writes nothing, so the maps still carry no debug-id, and
 `debug-files upload --dry-run` over the same directory then **fails with exit 11**. The JS plugin works
@@ -78,7 +86,7 @@ comment). So the one option documented as the safe diagnostic cannot exercise th
 Either `inject --dry-run` should report the ids it would write, or `upload --dry-run` should accept a
 map with no id when it is only packing.
 
-### 5. A symbol upload cannot carry the commit it came from (SEV2, verified in code)
+### 5. A symbol upload cannot carry the commit it came from (SEV2, verified in code) — **TRACKED: [bugsee-javascript#8](https://github.com/bugsee/bugsee-javascript/issues/8)**
 
 `bundler-plugin-core` already collects VCS metadata (`vcs.ts`, 315 lines: commit SHA, branch, dirty
 state, CI provider) and then has nowhere to put it: `orchestrate.ts:44-50` carries it only to echo it
@@ -105,7 +113,7 @@ The CLI blocker named above is now precise: `upload build` requires `--artifact`
 normal — register the build, ship no bytes — is the one a web build cannot express. The register-only
 mode exists in Rust and is used by `xcode post-action`; nothing exposes it on the CLI.
 
-### 7. Stamping after emit BREAKS Subresource Integrity (SEV1, reproduced 2026-09-18)
+### 7. Stamping after emit BREAKS Subresource Integrity (SEV1, reproduced 2026-09-18) — **(3) SHIPPED both sides; (1) still the real fix**
 
 Confirmed, and it is the worst finding here: the app's entry script is blocked and nothing runs.
 
@@ -143,6 +151,28 @@ all it can do. Three candidate fixes, none free:
 
 At minimum the JS plugin must do (3); (1) is the real fix.
 
+**Shipped 2026-09-19 — (3), on BOTH sides, because either alone leaves a hole.** The JS plugin scans the
+output directory before stamping (`bundler-plugin-core/src/sri.ts`, step 0 of `orchestrate`) and the CLI
+refuses independently (`bugsee-cli` `src/inject/sri.rs`, `--allow-sri` to override), so a user who drives
+the CLI by hand is covered too. Both read `<script>` AND `<link rel=modulepreload|preload>`, since a
+preload failing its integrity check poisons the module map just as hard.
+
+Two SEV1s were found in review of the first attempt, both worth recording because both looked correct:
+
+- a **CDN `publicPath`** build was stamped and shipped blank — the pinned URL resolved to nothing in the
+  output tree, so the literal-path lookup missed it. Fixed with a file-name fallback.
+- a page under a dot-directory or `node_modules` was **skipped by the guard but stamped by the walk** —
+  two different lists of files. Fixed by deriving both from one list. A test of mine had asserted that
+  skip as correct behaviour.
+
+Re-running on an already-stamped build used to exit 20 even though `--help` promises a no-op; the guard
+now mirrors `inject`'s own rewrite decision (`would_rewrite`), with a test pinning the two in agreement.
+
+**(1) remains open and is still the real fix.** Stamping during `processAssets`, before the hashes are
+computed, is the only approach correct for the runtime-embedded lazy-chunk hashes as well as the HTML;
+it needs an in-memory stamping path the CLI does not offer. Until then the guard trades a broken page
+for a failed build, which is strictly better but is not the same as working.
+
 ### 8. Not a CLI gap, but in the same flow
 
 - **Next.js server maps.** A stock `next build` emits maps for the client and for edge bundles only —
@@ -153,6 +183,9 @@ At minimum the JS plugin must do (3); (1) is the real fix.
   hand today.
 - **`sourcesContent`** rides into the upload with the map, which is how symbolication shows source lines.
   There is no `--strip-sources-content` for customers who would rather not ship source. Worth an option.
+  — **SHIPPED (CLI 0.7.11 + the plugin's `stripSourcesContent`).** Review caught a SEV1 in the first cut:
+  an INDEXED map (`sections[].map.sourcesContent`) kept shipping its source, silently, because only the
+  top-level key was removed. The removal is recursive now.
 
 ## Evidence
 - Next.js: `packages/nextjs-e2e` built with `productionBrowserSourceMaps: true` (`next build`, exit 0),