Skip to content

DO NOT MERGE: wrapper design and alignment plan - #11

Draft
krassx wants to merge 9 commits into
cursor/upm-package-scaffoldfrom
docs/wrapper-alignment
Draft

krassx wants to merge 9 commits into
cursor/upm-package-scaffoldfrom
docs/wrapper-alignment

Conversation

@krassx

@krassx krassx commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Updates DESIGN.md so the Unity wrapper follows the wrapper channel, data-request, and workbook specs.
  • Adds the implementation plan for the bridge and public-API alignment work.

Draft for reference only. Do not merge. Later tasks and agents should point at this plan instead of copying it into feature branches.

Test plan

  • Read DESIGN.md against the wrapper specs
  • Confirm the plan's file split still matches the open task pull requests

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Deep review: wrapper design + alignment plan

Reviewed DESIGN.md and docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md against the current bridges on this base (BugseeUnityBridge.mm, BugseeWrapperProxy, ReportHandlerProxy, IosNativeCallbacks), the pinned iOS 7.0.0-beta5 headers (+getAppearance → BugseeTheme *, not BGSAppearance), and the open task PRs (#3–#10). This PR is docs-only, but it is the spec remaining tasks 6/9–13 will copy. Three of those copies would ship product bugs.

Findings

Sev Issue
P1 Task 5 publishes the first real rects at version 1; Task 13 already served empty [1, 0]. SDK treats version as unchanged and keeps the empty mask. #8 already had to bump first non-empty to 2.
P1 Task 11 short-circuits isTerminating only in iOS forwardReport:. Android ReportHandlerProxy still JNI-round-trips into C# on a dying process.
P1 No R8 -keep for UnityWrapper methods C# calls by string (refineContext, later channelLog / setSecureBuffer). Minified release: NoSuchMethodError or identity stuck at unknown. #9 already added this.
P1 DESIGN says beta5 getAppearance is id<BGSAppearance>. Pinned Bugsee.h is + (BugseeTheme *)getAppearance with report*Color properties and no BGSAppearance conformance. The bridge on this branch already no-ops.
P2 Runtime tree still marks BugseeWrapperProxy as REQUIRED: setWrapper(impl), which is the crash-recovery bug the rest of the doc closes.
P2 Task 3 accepts only byte/short/int/long/float/double. uint/ulong/sbyte/decimal/ushort are numbers per DESIGN and were added in #7.
P2 Task 9 names HostLogForwarder but never subscribes ILogHandler / logMessageReceived. After all 15 tasks, Unity Debug still never hits the channel.
P2 DESIGN: hold addNetworkEvent until Launched on iOS. Task 12 submits immediately; iOS drops those events during Launching.
P2 Task 7 manifest omits android:directBootAware="true" that #9 needed so the provider actually runs before launch on Direct Boot.

1. Overall risk: High

Docs, but they are the implementation contract. Following this plan as written ships non-redacting secure rects, stripped JNI in release, and dying-process Android handlers that never complete.

2. Merge recommendation: Request changes

Absorb the corrections already in #8 (version 2), #9 (R8 keep, directBootAware, .meta), and #7 (TypeCode numbers). Fix the appearance pin claim against the headers this package already compiles. Add the Android isTerminating native path and the host-log / launch-gated network wiring that the minimum contract lists.

3. Most important to fix

  1. First empty publish = version 1; first non-empty = version 2 (Task 5 test today expects {1, 1, …}).
  2. Android isTerminating must complete in UnityWrapper / a native proxy, not ReportHandlerProxy.
  3. ProGuard keep for every JNI-called UnityWrapper method, updated again in Tasks 9 and 13.
  4. Appearance: BugseeTheme * via getAppearance on beta5; BGSAppearance is not what that getter returns.

4. What is solid

The wrapper-channel vs public-log split, breadcrumb mapping by name (Android/iOS ints are inverted), fail-closed filters, requestData always answering, iOS hop list (launch/stop/dialog/delete only), and the one-instance-before-launch rationale are the right traps. Task 10 correctly flips today’s keep = 1 catch paths.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread DESIGN.md Outdated
Comment thread DESIGN.md
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread DESIGN.md
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md
Correct iOS appearance pinning, secure-rect versioning, Android registration/R8/Direct Boot, and plan gaps for host logs, iOS network gating, and terminating handlers.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at 7b2dd35

The follow-up commit absorbs the prior P1s and most P2s. Checked against the same bridges, the beta5 BugseeTheme headers, and #7/#8/#9.

Prior Verdict
P1 secure-rect first non-empty at v1 fully_addressed — test is {2, 1, …}; Task 13 empty [1, 0] is the v1 baseline
P1 Android isTerminating still C# fully_addressed — Task 11 now includes ReportHandlerProxy
P1 no R8 keep / .meta fully_addressed — proguard-user.txt { *; } + PluginImporter note
P1 getAppearance is BGSAppearance fully_addressed — pin recorded as BugseeTheme *
P2 tree REQUIRED: setWrapper fully_addressed
P2 attribute TypeCodes partially_addressed — types listed, but ulong.MaxValue is asserted accepted and Step 3 routes integers through Convert.ToDouble
P2 HostLogForwarder unwired fully_addressed — Step 4b
P2 iOS network during Launching fully_addressed — Task 12 buffer
P2 directBootAware fully_addressed

Remaining

P2 Task 3: ulong.MaxValue (2⁶⁴−1) is ≥ 2⁶³, which this same task says to reject so Android cannot clamp it to Long.MAX_VALUE. Convert.ToDouble(long.MaxValue) is IEEE 9223372036854775808d (2⁶³), so the Step 3 recipe also fails the existing long.MaxValue is-accepted test. #7 already does this with TypeCode and an integer path that does not go through double.

1. Overall risk: Low

No remaining P0/P1. One P2 that would make Task 3 lock in the clamp the policy exists to prevent.

2. Merge recommendation: Request changes

Flip the ulong.MaxValue assertion to rejected and range-check integers without Convert.ToDouble.

3. Most important to fix

That Task 3 test + Step 3 recipe.

4. What improved

Appearance pin, secure-rect version invariant, Android terminating path, R8/Direct Boot, host-log wiring, and iOS launch-gated network are now in the spec the remaining tasks will copy.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Reject ulong above long.MaxValue and range-check integers without double conversion so long.MaxValue stays accepted.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at e8948fc

The remaining Task 3 P2 is fixed: ulong.MaxValue is rejected, (ulong)long.MaxValue / uint.MaxValue are accepted, and Step 3 range-checks integers via TypeCode without Convert.ToDouble (so long.MaxValue is not IEEE-rounded to 2⁶³).

All prior P0–P3 threads are fully_addressed. No new findings on this commit.

1. Overall risk: Low

Docs-only spec. Remaining wrapper tasks now match the corrections already in #7/#8/#9 and the beta5 BugseeTheme pin.

2. Merge recommendation: Approve

3. Most important issues to fix

None remaining.

4. Positive

The plan now states the secure-rect v1-empty / v2-first-rects invariant, Android isTerminating native completion, R8 keep + Direct Boot, host-log wiring, and iOS launch-gated network — the traps that would have shipped if the first draft had been copied as-is.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

@krassx
krassx marked this pull request as draft October 9, 2026 07:24
@krassx krassx added the DO NOT MERGE Reference only. Do not merge. label Oct 9, 2026
@krassx krassx changed the title Record the wrapper design and alignment plan DO NOT MERGE: wrapper design and alignment plan Oct 9, 2026

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at e8948fc

The Task 3 ulong.MaxValue / Convert.ToDouble P2 is fully_addressed. Integers are compared to long.MaxValue without going through double, and ulong.MaxValue is asserted rejected. Prior P1s remain closed.

This pass checked the plan against the bridges on this base (AndroidBridge.EnsureWrapperRegistered still setWrappers a C# proxy, ReportHandlerProxy still hops terminating reports into C#, IosNativeCallbacks catch still keep = 1), Editor/BugseeAndroidGradleSetup.cs (// Bugsee Gradle plugin), and the parallel task PRs (#7 attribute policy, #8 rect version 2, #9 5-arg refineContext, #10 legacy Gradle marker).

Remaining

Sev Issue
P2 Task 7 Interfaces + Java snippet: 4-arg refineContext, getWrapperVersion() hardcoded "0.1.0". Step 4 C# already calls five arguments.
P2 DESIGN minimum item 7 and implementation-order step 2 require routing UnityWebRequest on the wrapper channel. After all 15 tasks the plan only has HostLogForwarder + a public AddNetworkEvent.
P2 Task 14 treats // bugsee:gradle-plugin as the only marker. This package already writes // Bugsee Gradle plugin.

1. Overall risk: Medium

No P0/P1. Three P2s that an agent copying this plan would ship: JNI miss or wrapper version stuck at 0.1.0, Unity HTTP missing from reports, duplicate Gradle plugin lines on existing templates.

2. Merge recommendation: Request changes

Align Task 7 with the 5-arg refineContext already in #9. Add a UnityWebRequest → channel step (or list it under Out of scope). Recognize the existing Gradle marker in Task 14, as #10 does.

3. Most important to fix

  1. Task 7 Java/Interfaces signature and getWrapperVersion field.
  2. UnityWebRequest capture vs an explicit out-of-scope note.
  3. Task 14 legacy marker // Bugsee Gradle plugin.

4. What is solid

Appearance pin (BugseeTheme *), secure-rect empty v1 / first non-empty v2, Android terminating path in Task 11, R8 { *; } + Direct Boot, host-log subscription, iOS launch-gated network buffer, and the Task 3 integer bounds are now the right traps for the remaining tasks to copy.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md
…capture, and the existing Gradle marker.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at ddaa617

The three remaining P2s from the last pass are fully_addressed.

Prior Verdict
Task 7 4-arg refineContext / hardcoded "0.1.0" fully_addressed — Interfaces, Java sample, and C# call are the same 5-arg method; getWrapperVersion() returns the field (default "unknown")
No UnityWebRequest on the channel fully_addressed — Task 9 Step 4c HostNetworkForwarder + Task 12 iOS Launching buffer
Task 14 ignores // Bugsee Gradle plugin fully_addressed — legacy marker and an active com.bugsee.android.gradle line count as already patched; test Legacy_bugsee_marker_is_not_duplicated

No remaining P0–P3. No new findings on this commit.

1. Overall risk: Low

Docs-only. The plan now matches the corrections in #7/#8/#9/#10 and the beta5 BugseeTheme pin.

2. Merge recommendation: Approve

3. Most important issues to fix

None remaining.

4. Positive

JNI identity, host logs, Unity HTTP, secure-rect versioning, terminating handlers, R8/Direct Boot, and Gradle marker recognition are now written so an agent copying the remaining tasks will not reintroduce the bugs the sibling PRs already had to fix.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at ddaa617

The three P2s from the last pass are closed: 5-arg refineContext + wrapperVersion field, a HostNetworkForwarder step, and Task 14 treating // Bugsee Gradle plugin as already patched. Checked against this base (AndroidBridge.EnsureWrapperRegistered still setWrappers a C# proxy, ReportHandlerProxy still hops terminating reports into C#, IosNativeCallbacks catch still keep = 1), Android 7.3.0 BugseeWrapper / DataRequestProvider in the published AAR, current Editor/BugseeAndroidGradleSetup.cs launcher ndk block, and the parallel task PRs (#9 registration, #10 marked Gradle).

Remaining

Sev Issue
P1 Task 14 Apply only inserts a plugin line. Today's launcher write (and #10) also inserts marked bugsee { ndk { enabled = true } }. Existing Custom Launcher templates get the plugin without NDK.
P2 Task 9 Step 4c still has no Unity API that observes every UnityWebRequest. Unity 2021.3 has no global completion event. The source test only requires the string UnityWebRequest.
P2 Task 12 Android calls UnityWrapper.channelAddNetwork. Task 9 only adds channelLog. Task 12 does not modify the Java file.
P2 Task 7 requestData uses contracts.internal.DataRequestResultCallback. 7.3.0 DataRequestProvider takes contracts.common.DataRequestResultCallback. That sample does not compile.

1. Overall risk: High

Docs, but this is the contract remaining tasks will copy. Task 14 as written drops NDK enablement on the exact existing-template path the task exists to handle. Native crash capture and IL2CPP linemap primary apply travel with plugin.ndk.enabled.

2. Merge recommendation: Request changes

Absorb #10's launcher payload (apply plugin + marked bugsee { ndk }). Give Task 9 a real Android channelAddNetwork. Either specify a UnityWebRequest weaver/wrapper or move auto-capture to Out of scope. Fix the Task 7 callback package to match the AAR (#9 already does).

3. Most important to fix

  1. Task 14 launcher: plugin line and bugsee { ndk { enabled = true } }, with a test on a default apply plugin: 'com.android.application' file.
  2. UnityWrapper.channelAddNetwork in Task 9 (or add the Java file to Task 12).
  3. UnityWebRequest: real hook, or Out of scope — do not claim DESIGN minimum 7 is done.
  4. DataRequestResultCallback → com.bugsee.library.contracts.common.

4. What is solid

5-arg refineContext / no baked 0.1.0, legacy Gradle marker + active-plugin short-circuit, appearance pin (BugseeTheme *), secure-rect empty v1 / first non-empty v2, Android isTerminating in ReportHandlerProxy, R8 { *; } + Direct Boot, host-log logMessageReceived, iOS launch-gated network buffer, and Task 3 integer bounds without Convert.ToDouble.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
…cher ndk patch.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at ad1c5d6

ad1c5d6 closes three of the four findings from the last pass. Checked against this base (AndroidBridge.EnsureWrapperRegistered still setWrappers a C# proxy, ReportHandlerProxy still hops terminating reports into C#, IosNativeCallbacks catch still keep = 1), Android 7.3.0 classes.jar (DataRequestProvider.requestData → contracts.common.DataRequestResultCallback; LogSource.fromRawValue is (byte) / (byte, LogSource) with a null miss default; Bugsee.getExchangeFactory() exists; getSecureRectangles default returns null), current Editor/BugseeAndroidGradleSetup.cs launcher ndk block, merged #10 MarkedGradleBlock.Apply (independent plugin + ndk satisfaction), and the parallel task PRs.

Prior Verdict
P1 Task 14 launcher drops bugsee { ndk } fully_addressed — default launcher payload and a second-apply test
P2 no real UnityWebRequest hook fully_addressed — moved to Out of scope; coverage table no longer claims DESIGN min 7
P2 channelAddNetwork missing partially_addressed — Interfaces/prose mention it; the Java sample and the compiled test still do not
P2 DataRequestResultCallback wrong package fully_addressed — contracts.common

Remaining

Sev Issue
P2 Task 9 Step 4 Java is still two-arg channelLog only. Task 12 CallStatic("channelAddNetwork") has no method unless the agent ignores the fence and follows later prose.
P2 Task 14 Interfaces + Step 3 still treat any plugin marker/com.bugsee.android.gradle line as already patched, which skips ndk upsert. The new test only covers a launcher with no Bugsee. #10 already had to split satisfaction per region.

1. Overall risk: Medium

Docs, but remaining tasks 9/12 will copy the Java fence. That path still ships Android AddNetworkEvent as NoSuchMethodError. Task 14 as specified can still skip plugin.ndk.enabled on a launcher that already has the plugin line.

2. Merge recommendation: Request changes

Put channelAddNetwork in the Java sample and in Android_channel_log_passes_custom_source (or a sibling assert). Drop the whole-file “already patched” short-circuit; upsert plugin and ndk independently, matching #10.

3. Most important to fix

  1. Task 9 Java: channelAddNetwork(NetworkEvent) → channel.addNetworkEvent(event, true) next to channelLog, and a source assert for addNetworkEvent + true.
  2. Task 14: plugin present / ndk absent must still insert the marked bugsee { ndk { enabled = true } } block.

4. What is solid

requestData now matches 7.3.0 DataRequestProvider. UnityWebRequest auto-capture is honestly out of scope instead of a string check. Default launcher ndk is specified. Appearance pin (BugseeTheme *), secure-rect empty v1 / first non-empty v2, Android isTerminating in ReportHandlerProxy, R8 { *; } + Direct Boot, host-log logMessageReceived, iOS launch-gated network buffer, and Task 3 integer bounds without Convert.ToDouble remain the right traps.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
…separately.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at 82d2dd9

82d2dd9 closes both remaining P2s from the last pass. Checked against this base (AndroidBridge.EnsureWrapperRegistered still setWrappers a C# proxy, ReportHandlerProxy still hops terminating reports into C#, IosNativeCallbacks catch still keep = 1), merged #8 SecureRectRegistry (one rect per (ownerId, displayId)), merged #9 UnityWrapper (onWrapperChannelAvailable is still a comment; getSecureRectangles returns {1, 0}), merged #10 MarkedGradleBlock.Apply (independent plugin + ndk satisfaction), and open #5 HostLogForwarder (logMessageReceivedThreaded + ThreadStatic depth + main-thread JNI hop).

Prior Verdict
P2 Task 9 Java fence missing channelAddNetwork fully_addressed — 3-arg channelLog with LogSource.fromRawValue((byte) source, Custom) plus channelAddNetwork → addNetworkEvent(event, true). Source test asserts channelAddNetwork / addNetworkEvent / true / fromRawValue.
P2 Task 14 whole-file “already patched” skip fully_addressed — plugin and ndk regions are satisfied separately; a plugin marker no longer skips ndk. New test is plugin-present / ndk-absent, matching #10 Plugin_only_launcher_patch_adds_ndk_block.

Remaining

Sev Issue
P2 Task 13 sends every AddSecureRectangle through Set("manual", 0, …). #8’s registry stores one rect per owner+display, so a second add unmasks the first. RemoveSecureRectangle is in the file list and has no behavior.
P2 Task 9 Step 4b still subscribes Application.logMessageReceived with no reentrancy guard. #5 already crashed (filter Debug.Log → channel → filter) and missed off-main Debug output.

1. Overall risk: Medium

Docs, but remaining Task 13 will copy the single "manual" owner onto a one-slot registry. That ships a privacy hole on the public Add API. Task 9 as written would also regress the HostLogForwarder #5 already had to patch.

2. Merge recommendation: Request changes

Give each imperative rect its own owner (or a list under manual) and specify RemoveSecureRectangle. Absorb #5’s HostLogForwarder contract: logMessageReceivedThreaded, a reentrancy guard around ChannelLog, and a main-thread hop before Android JNI.

3. Most important to fix

  1. Task 13: unique owner per AddSecureRectangle (or append under manual); RemoveSecureRectangle drops that rect only; RemoveAll still clears the manual set.
  2. Task 9 Step 4b: subscribe logMessageReceivedThreaded, guard reentrancy, hop off-main callbacks to the player thread before channelLog.

4. What is solid

channelAddNetwork now matches Android 7.3.0 BugseeWrapperChannel.addNetworkEvent(NetworkEvent, boolean). Task 14 no longer regresses #10’s independent ndk upsert. Appearance pin (BugseeTheme *), secure-rect empty v1 / first non-empty v2, Android isTerminating in ReportHandlerProxy, R8 { *; } + Direct Boot, iOS launch-gated network buffer, UnityWebRequest auto-capture honestly out of scope, and Task 3 integer bounds without Convert.ToDouble remain the right traps.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Task 13 uses per-rect owner ids; Task 9 matches #5 threaded log subscribe, reentrancy, and Stop uninstall.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at 8dcf03e

8dcf03e closes both P2s from the last pass. Checked against merged #8 SecureRectRegistry (one rect per (ownerId, displayId); RemoveOwner does not call Snapshot or bump version) and open #5 HostLogForwarder (logMessageReceivedThreaded + ThreadStatic depth + main-thread hop + Uninstall).

Prior Verdict
P2 Task 13 single Set("manual") owner fully_addressed — per-rect owner ids, RemoveSecureRectangle drops one owner, RemoveAll clears the manual set, test requires count 2 then a surviving rect
P2 Task 9 HostLogForwarder no guard / main-only subscribe fully_addressed — logMessageReceivedThreaded, player-thread hop, reentrancy depth, skip LogType.Exception, Uninstall from Bugsee.Stop; source test requires those strings

Remaining

Sev Issue
P2 Task 13 Step 1 pushes Snapshot only on AddSecureRectangle. Remove / RemoveAll update the C# map and call RemoveOwner, which in #8 does not publish. Native getSecureRectangles keeps the last pushed buffer.

1. Overall risk: Low

Docs. The remaining hole is fail-closed (a dismissed rect stays masked) rather than a leak, but the public Remove API Task 13 is specifying would be a no-op on video / screenshot / hit-test.

2. Merge recommendation: Request changes

Push after every registry mutation that changes the union (Add, Remove, RemoveAll), including the empty [1, 0] after the last manual rect is gone.

3. Most important to fix

  1. Task 13: RemoveSecureRectangle / RemoveAllSecureRectangles must Snapshot and setSecureBuffer the same way Add does. The new owner test should also assert a push / version bump on remove, not only C# Snapshot count.

4. What is solid

Per-rect owners match #8 and the additive Add/Remove facade. HostLogForwarder now matches the contract #5 already had to ship. channelAddNetwork, independent Gradle plugin/ndk upsert, appearance pin, empty-v1 / first-non-empty-v2, Android isTerminating, R8 + Direct Boot, iOS launch-gated network, and UnityWebRequest auto-capture out of scope remain correct.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at 8dcf03e

8dcf03e closes both remaining P2s from the last pass. Checked against this base (ExceptionPipeline.Install returns before any subscription when CaptureManagedExceptions is false; MainThreadDispatcher.Run queues off-main work until the next Update), merged #8 SecureRectRegistry (one rect per (ownerId, displayId)), and open #5 HostLogForwarder (InstallOnce from Launch/Relaunch, Uninstall clears _installed, _generation drops stale queued Send).

Prior Verdict
P2 Task 13 Set("manual", 0) overwrites the first rect fully_addressed — per-rect owner ids, RemoveSecureRectangle drops that owner only, RemoveAll clears the manual set.
P2 Task 9 Step 4b logMessageReceived with no reentrancy / off-main path fully_addressed — logMessageReceivedThreaded, player-thread hop, [ThreadStatic] depth, Uninstall on Stop.

Remaining

Sev Issue
P2 Task 9 hangs HostLogForwarder.InstallOnce on ExceptionPipeline. That method no-ops when CaptureManagedExceptions is false, so host Debug never reaches the channel.
P2 Step 4b InstallOnce is a process flag and Uninstall does not reset it; off-main hops have no generation. Stop then Launch leaves host logs dead, and a queued Send can still hit the next session.

1. Overall risk: Medium

Docs, but Task 9 is still the spec remaining work will copy. Threaded subscribe and per-rect owners are right; the HostLogForwarder lifecycle is not. Following Step 4b as written ships a channel that is dark after Stop/Launch, or never installs when managed-exception capture is off — both cases #5 already had to patch.

2. Merge recommendation: Request changes

Keep this draft unmerged (title already says so). Absorb the rest of #5’s HostLogForwarder contract before any agent copies Task 9: install from Launch/Relaunch independently of ExceptionPipeline, reset the installed flag on Uninstall, and drop stale queued sends with a generation counter.

3. Most important to fix

  1. Task 9: HostLogForwarder.InstallOnce from AndroidBridge/IOSBridge Launch and Relaunch, not from ExceptionPipeline.Install.
  2. Uninstall must clear the process flag so the next Launch can subscribe again; stamp _generation on the queued main-thread hop.

4. What is solid

Task 13 no longer collapses two HUD rects onto one #8 slot. Task 9 now names logMessageReceivedThreaded, the reentrancy guard, and Stop → Uninstall. Appearance pin (BugseeTheme *), secure-rect empty v1 / first non-empty v2, Android isTerminating in ReportHandlerProxy, R8 { *; } + Direct Boot, iOS launch-gated network buffer, channelAddNetwork → addNetworkEvent(event, true), and Task 14 independent plugin/ndk upsert remain the right traps.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at 8dcf03e

8dcf03e closes both remaining P2s from the last pass. Checked against this base (ExceptionPipeline.Install returns before any subscription when CaptureManagedExceptions is false; MainThreadDispatcher.Run queues off-main work until the next Update), merged #8 SecureRectRegistry (RemoveOwner only drops the dict entry — version/buffer change happens in Snapshot), and open #5 HostLogForwarder (InstallOnce from Launch/Relaunch, Uninstall clears _installed, _generation drops stale queued Send).

Prior Verdict
P2 Task 13 Set("manual", 0) overwrites the first rect fully_addressed — per-rect owner ids, RemoveSecureRectangle drops that owner only, RemoveAll clears the manual set.
P2 Task 9 Step 4b logMessageReceived with no reentrancy / off-main path fully_addressed — logMessageReceivedThreaded, player-thread hop, [ThreadStatic] depth, Uninstall on Stop.

Remaining

Sev Issue
P2 Task 9 hangs HostLogForwarder.InstallOnce on ExceptionPipeline. That method no-ops when CaptureManagedExceptions is false, so host Debug never reaches the channel.
P2 Step 4b InstallOnce is a process flag and Uninstall does not reset it; off-main hops have no generation. Stop then Launch leaves host logs dead, and a queued Send can still hit the next session.
P2 Task 13 pushes Snapshot only from AddSecureRectangle. #8 RemoveOwner does not publish. Native pull keeps the previous mask after Remove / RemoveAll.

1. Overall risk: Medium

Docs, but Tasks 9 and 13 are still the spec remaining work will copy. Threaded subscribe and per-rect owners are right; HostLogForwarder lifecycle and remove-side publish are not. Following this plan as written ships a channel that is dark after Stop/Launch (or never installs when managed-exception capture is off), and a public RemoveSecureRectangle that cannot clear the native mask.

2. Merge recommendation: Request changes

Keep this draft unmerged (title already says so). Absorb the rest of #5’s HostLogForwarder contract, and push Snapshot on every registry mutation including remove/clear, before any agent copies Tasks 9 and 13.

3. Most important to fix

  1. Task 9: HostLogForwarder.InstallOnce from AndroidBridge/IOSBridge Launch and Relaunch, not from ExceptionPipeline.Install.
  2. Uninstall must clear the process flag so the next Launch can subscribe again; stamp _generation on the queued main-thread hop.
  3. Task 13: after RemoveOwner / RemoveAll, push Snapshot the same way Add does (empty [version, 0] is a real change and must bump).

4. What is solid

Task 13 no longer collapses two HUD rects onto one #8 slot. Task 9 now names logMessageReceivedThreaded, the reentrancy guard, and Stop → Uninstall. Appearance pin (BugseeTheme *), secure-rect empty v1 / first non-empty v2, Android isTerminating in ReportHandlerProxy, R8 { *; } + Direct Boot, iOS launch-gated network buffer, channelAddNetwork → addNetworkEvent(event, true), and Task 14 independent plugin/ndk upsert remain the right traps.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
Install host logs from Launch/Relaunch with generation-aware Uninstall; push native buffer after every registry mutation.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at adcbc87

adcbc87 closes the three remaining P2s. Checked against merged #8 SecureRectRegistry.Snapshot (first empty is version 1; a later empty after rects is version 3 / [3, 0], not [1, 0]) and open #5 HostLogForwarder (Launch/Relaunch install, _installed = false + _generation++ on Uninstall).

Prior Verdict
P2 Task 9 install only from ExceptionPipeline fully_addressed — Launch/Relaunch on both bridges; source test no longer requires ExceptionPipeline.InstallOnce
P2 Uninstall leaves process flag; no generation on the hop fully_addressed — _installed = false, _generation captured before MainThreadDispatcher, drop mismatches
P2 Task 13 push only on Add fully_addressed — shared PushSecureBufferForDisplay after Add/Remove/RemoveAll

Remaining

Sev Issue
P2 Step 1 and SecureRectManualOwnerTests require the pushed buffer after the last remove to be [1, 0]. #8’s Snapshot after Add-then-RemoveLast is [3, 0]. Forcing version 1 reuses the launch empty baseline; the SDK keeps the previous mask.

1. Overall risk: Low

Docs. HostLogForwarder lifecycle now matches #5. The leftover is the same version-key trap Task 5 already encoded: a later all-clear must not republish version 1.

2. Merge recommendation: Request changes

Assert count 0 and a bumped version after the last remove (>= 2), not the literal first-empty [1, 0].

3. Most important to fix

  1. Task 13: first empty publish is [1, 0]. A later empty after real rects is [version, 0] with version changed from the last non-empty. Align Step 1 and the test with #8 / Task 5 (Ios_scale_3 already pins first non-empty at 2).

4. What is solid

Launch-gated host-log install, Uninstall that can re-subscribe, generation-dropped queued sends, per-rect owners, and push-on-remove. channelAddNetwork, independent Gradle plugin/ndk upsert, appearance pin, Android isTerminating, R8 + Direct Boot, and UnityWebRequest auto-capture out of scope remain correct.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Comment thread docs/superpowers/plans/2026-10-09-wrapper-bridge-alignment.md Outdated
After all rects are removed, push count zero with a bumped version, not a reused v1 baseline.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Re-review at 4087d29

4087d29 closes the last remaining P2. Checked against merged #8 SecureRectRegistry.Snapshot (first empty [1, 0]; add-then-remove-last [3, 0]) and the HostLogForwarder contract already absorbed from #5.

Prior Verdict
P2 last-remove pushed [1, 0] fully_addressed — empty union after rects is count 0 with a bumped version; test requires version > last non-empty; first-empty [1, 0] stays only for “nothing ever pushed”

No remaining P0–P3.

1. Overall risk: Low

Docs-only living spec. The traps remaining tasks will copy now match the landed registries and the HostLogForwarder #5 already shipped.

2. Merge recommendation: Approve as reference draft

Keep the PR unmerged (title already says so). Point later tasks at this plan instead of copying it onto feature branches.

3. Most important to fix

None.

4. What is solid

Per-rect owners + push on Add/Remove/RemoveAll + later-empty version bump; HostLogForwarder logMessageReceivedThreaded, reentrancy, Launch/Relaunch install, Uninstall that resets _installed and _generation; channelAddNetwork → addNetworkEvent(event, true); independent Gradle plugin/ndk upsert; appearance pin (BugseeTheme *); Android isTerminating in ReportHandlerProxy; R8 { *; } + Direct Boot; iOS launch-gated network; UnityWebRequest auto-capture honestly out of scope; Task 3 integer bounds without Convert.ToDouble.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at 4087d29

4087d29 closes the remaining Task 13 P2. Checked against merged #8 SecureRectRegistry.Snapshot (first empty is version 1; add then last-remove is [3, 0]) and merged #5 HostLogForwarder (Launch/Relaunch install, _installed = false + _generation++ on Uninstall).

Prior Verdict
P2 last-remove pushed [1, 0] fully_addressed — Step 1 and SecureRectManualOwnerTests now require count 0 and a version strictly greater than the last non-empty publish; the prose cites #8 [3, 0] and forbids reusing version 1

No remaining P0–P3. No new findings on this commit. Also re-checked Tasks 9–14 against the current scaffold (UnityWrapper.channelLog + empty {1, 0} pull, ReportHandlerProxy still hops terminating reports, iOS keep = 1 catch, #10 independent plugin/ndk upsert). Those remaining tasks still specify the corrections the earlier P1/P2s asked for.

1. Overall risk: Low

Docs-only spec. The last version-key trap (a later all-clear must not republish the launch empty baseline) is now written the same way as Task 5 / #8.

2. Merge recommendation: Approve the spec; keep the PR unmerged

Title already says DO NOT MERGE. The plan is ready for remaining tasks to copy.

3. Most important issues to fix

None remaining.

4. What is solid

Empty-v1 / first-non-empty-v2 / later-empty-bumped, push-on-remove, per-rect owners, Launch-gated HostLogForwarder with generation-dropped hops, channelAddNetwork → addNetworkEvent(event, true), independent Gradle plugin/ndk upsert, appearance pin (BugseeTheme *), Android isTerminating in ReportHandlerProxy, R8 + Direct Boot, and UnityWebRequest auto-capture honestly out of scope.

Open in Web View Automation 

Sent by Cursor Automation: Bugsee code review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DO NOT MERGE Reference only. Do not merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant