Release integration: reconcile dev and main session recovery

[fluck-boss]

Validation-only integration branch for #1728. Do not merge or dispatch a release until the current merged-tree CI, review, crash pass and version/notes plan are cleared.

Merge parents: main 3170b381, dev 4fa78542. The sole textual conflict in BossAppStartupEffects.kt was resolved by retaining main's updateSessionSpace identity rebinding and dev's owner-only paired writeInSessionRecovery. The old saveSessionRecovery two-step helper was removed because it called the saveLastSessionRecord method dev removed. The shared session-set extractor reads the active in-memory Space first so a newly assigned identity is included. Local git diff --check passed; local Gradle could not start because the available JVM is 11 and this build needs 17+.

At preparation, main was already 9.5.27 with release notes while dev still read 9.5.25. The merged tree retains main's 9.5.27 and its existing notes. Do not release with skip_version_increment=true; the next version must be incremented and its own notes generated. #1728's old review covered earlier heads and explicitly did not evaluate the app-testing findings.

[supabase]

This pull request has been ignored for the connected project pcnwqamqdnsadranufjv due to reaching the limit of concurrent preview branches.
Go to Project Integrations Settings ↗︎ if you wish to update this limit.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[fluck-boss]

@claude Please review release integration #1738 at exact head 88c07f339ecda18981d49660b722a62c1c614bac against current main 3170b381b83bf0ce6b3cd179baa92887be60804a. This is the merged tree of dev 4fa78542 and main, not the old #1728 diff. Focus on the sole textual conflict in BossAppStartupEffects.kt: it combines #1736's updateSessionSpace identity rebinding with #1713's owner-only paired writeInSessionRecovery; the obsolete saveSessionRecovery was removed. Check that a newly assigned single-Space identity is preserved on clean shutdown and hard kill, that the in-session set/record stay paired, and that the last-window/shutdown path and current WorkspaceManager APIs remain correct. Also check the semantic interaction of main's #1736 with all newly integrated dev changes, version 9.5.27/release notes, and security/behavior regressions. Please give a specific blocking/non-blocking verdict for this head; do not rely on the old #1728 review. App crash testing is separate and pending. No merge or release dispatch is authorized from this review request.

[claude]

Claude finished @fluck-boss's task in 9m 52s —— View job

---

Review: release integration #1738 @ 88c07f33

• Confirm merge parents and scope the real merged-tree diff
• Review the BossAppStartupEffects.kt conflict resolution (feat(macOS): add native title-bar navigation and sidebar controls #1736 × fix(workspaces): stop an earlier session's Space set overruling the crash-recovery record #1713)
• Verify newly assigned single-Space identity survives clean shutdown and hard kill
• Verify in-session set/record stay paired; owner-only write correctness
• Verify last-window / shutdown path and current WorkspaceManager APIs
• Check feat(macOS): add native title-bar navigation and sidebar controls #1736 interaction with all newly integrated dev changes
• Check version 9.5.27 / release notes state
• Security & behavior regression pass over the integrated dev changes
• Verdict

Verified locally: HEAD = 88c07f33, parents 3170b381 (main) + 4fa78542 (dev), main is an ancestor. Reviewed with git diff HEAD^1 HEAD and git diff HEAD^2 HEAD so the merge's own decisions are separable from what each parent brought — not the old #1728 diff.

---

Verdict

NON-BLOCKING on the conflict resolution. The BossAppStartupEffects.kt resolution is correct and, in one respect, better than either parent. Two release gates are unmet and one comment should be fixed before merge; none is a defect in the merged logic.

Conflict resolution correctness	✅ non-blocking
Identity preserved on clean shutdown	✅ verified by inspection
Identity preserved on hard kill	✅ verified by inspection
Set/record pairing	✅ with one inherited gap (F3)
Last-window / shutdown path, WorkspaceManager APIs	✅ no dangling refs, no API drift
Release notes cover the merged tree	❌ gate (G1)
Green 🔨 Build & Test at this head	⏳ gate (G2)

---

The conflict resolution is right, and the identityFor change is load-bearing

The merged settle job is:

updateSessionSpace(write.current, splitViewState)      // main / #1736
writeInSessionRecovery(                                 // dev / #1713
    windowId = windowId,
    record = write.record,
    set = { liveSessionSet(splitViewState, selectedProject.path, defaultWorkingDirectory) },
)

composeApp/src/commonMain/kotlin/ai/rever/boss/app/BossAppStartupEffects.kt:839-848

I traced the identity chain end to end and it holds:

1. layoutWatcherWrite returns current = record (id last-session) for a window with no Space.
2. updateSessionSpace → prepareSessionSpace → sessionSpaceIdentity mints a new id + "Recovered Space" when enableLastSessionSpace is false, then rebindCurrentWorkspace(space.id) and updateCurrentWorkspace(space). Both are plain synchronous assignments (SplitView.kt:424, WorkspaceManager.kt:787).
3. liveSessionSet therefore reads the new id from splitViewState.currentWorkspaceId for activeWorkspaceId, and extractRunningWorkspaces reads it from running.workspaceId. MINIMUM_SET_SIZE = 1 (main's value survived the merge, LastSessionSet.kt:100), so a one-Space window writes a set — which is what carries the id that Last_Session.json cannot. Pinned by LastSessionSetTest.kt:62.
4. writeInSession writes set first, then record, so a kill mid-pair leaves the file restore prefers.

Ordering is the whole thing here and it is correct: updateSessionSpace runs before the set is built, so the set sees the rebound id. Reversing those two lines would silently write the pre-rebind identity.

The identityFor change is not cosmetic and is easy to mistake for one. The merge replaced dev's identityFor = { id -> workspaceManager.savedCopyOf(id) } with a currentWorkspace-first lookup (BossAppStartupEffects.kt:896-901). That is required, because prepareSessionSpace reaches WorkspaceManager.importWorkspace, which returns the new Space synchronously but persists it and inserts the list row inside a fire-and-forget scope.launch (WorkspaceManager.kt:625-639). At the moment liveSessionSet runs — the same tick — savedCopyOf(newId) can still be empty, so extractRunningWorkspaces would fall through to name = running.workspaceName.ifEmpty { running.workspaceId } and record the raw generated id as the Space's display name. updateCurrentWorkspace has already landed synchronously, so the currentWorkspace branch closes that window. The PR body's phrasing ("so a newly assigned identity is included") understates it slightly — the id comes from SplitViewState either way; what this fixes is the name/description, which is all identityFor supplies.

The shutdown path is safe, but for a non-obvious reason worth recording. claimAndWrite calls save(extractLayout()) → saveLastSessionBlocking, which overwrites _currentWorkspace.value with the last-session-stamped record (WorkspaceManager.kt:539), and only then calls window.extractSet() → liveSessionSet, which now reads currentWorkspace. The takeIf { it.id == id } guard is the only thing that stops the clobbered value being adopted as a running Space's identity: for a Space with a real id the guard fails and it falls back to savedCopyOf, which by shutdown has the imported row. Correct as written, but this is a coupling nobody would expect from reading either side alone — dropping the takeIf would corrupt the shutdown set specifically.

Other checks that came back clean:

• No dangling references to the removed saveSessionRecovery or saveLastSessionRecord anywhere in the tree.
• writeLastSessionRecordBlocking and saveLastSessionBlocking both resolve through fileNameFor(record) with the same last-session id, so the in-session and shutdown record writes target the same file.
• noteLastSessionRecordWritten correctly sits after the withContext(Dispatchers.IO + NonCancellable) so a closing window skips the list refresh — matching its own KDoc.
• writeLock is taken by both writeLastSession and writeInSession, no nesting, no deadlock.
• ownsSessionRecord's canSave predicate matches the watcher's own workspaceRestorationComplete && !sessionRestoreRefused guards.
• All 12 InSessionRecoveryTest methods use runBlocking<Unit>, satisfying the AGENTS.md "a @Test that returns a value never runs" rule — relevant because this merge moves that guard to allprojects.

---

Findings

F1 — liveSessionSet's KDoc contradicts the code it sits on (non-blocking, fix before merge).
composeApp/src/commonMain/kotlin/ai/rever/boss/app/BossAppStartupEffects.kt:883

"Null for a window running fewer than two, which the single-Space record already describes on its own"

That is dev's rule. The merged code uses MINIMUM_SET_SIZE = 1 and returns a set for one Space — the identity fix is the entire point of #1736. The merge author correctly updated three of these comments (the extractSet call site at :163, the restore fallback at :611, and WorkspaceManager.loadLastSessionSet) and missed this one, which is the newly authored one sitting directly on the reconciled function. InSessionRecovery.kt:19 carries the same stale claim (inherited verbatim from dev), as does LastSessionCoordinator.register's extractSet param KDoc (pre-existing on main). In a repo where these comments are the spec, the risk is concrete: the next reader "fixes" sessionSetOf to match the comment and silently reverts #1736. Fix this →

F2 — the merge's own resolution has no test (non-blocking, strongest recommendation).
Neither parent's suite reaches it. InSessionRecoveryTest never calls updateSessionSpace and builds its sets from an explicit lambda, so the identityFor fallback and the rebind-then-extract ordering are untested; LastSessionSetTest covers sessionSetOf in isolation. The one behaviour this integration branch exists to establish — a window with no Space mints an identity, and both recovery files then carry it — is verified only by inspection. Given app crash testing is separately pending, a single test driving updateSessionSpace followed by writeInSessionRecovery against a real WorkspaceManager on a temp directory would be cheap and would pin the ordering that a future reader could reverse without any test noticing.

F3 — the in-session pair can still half-succeed without the caller knowing (non-blocking, inherited from #1713).
LastSessionCoordinator.writeInSession (:196-214) evaluates saveSet(set); saveRecord(record) and the block's value is saveRecord's, so a set write that returns false without throwing is dropped from the return value. writeInSessionRecovery then reports success and notes the record as written, while the stale set — which restore prefers — stays on disk. saveLastSessionSetBlocking does log its own failure at WARN (WorkspaceManager.kt:571), so this is not silent, but the pairing guarantee the reviewer asked about is "both or the set alone", and this is the one path that yields "record alone". Verbatim from dev, not created by the merge.

F4 — the two write paths order the pair oppositely (non-blocking, follow-up).
writeInSession writes set→record and documents why ("if the pair is ever cut short… the file that landed is the one restore reads"). claimAndWrite writes record→set (:262-267). By writeInSession's own argument the shutdown order is the wrong one: a kill between shutdown's two writes leaves a fresh record beside a set that is up to one settle interval stale, and restore prefers the set. The in-session writes shrink that window to ~2s, which is presumably why it was left, but having both orders in one class each documented as correct will not survive contact with the next editor.

F5 — WorkspaceManager.loadedFileNames is a plain LinkedHashMap read from two threads, and the merge adds an interleaving (PLAUSIBLE, non-blocking, pre-existing).
private val loadedFileNames = mutableMapOf<String, String>() (:161). The merge now runs, within one settle tick: updateSessionSpace → importWorkspace → scope.launch { … loadedFileNames[id] = fileName } on Main, immediately followed by writeInSessionRecovery → writeLastSessionRecordBlocking → fileNameFor reading the same map on Dispatchers.IO. A concurrent read during a rehash can misreport, and the consequence is the record written to last-session.json instead of a legacy Last_Session.json that restore reads. The class already had a worse version of this (the shutdown hook mutates the map from its own thread), so this is a pre-existing hazard newly widened rather than introduced — ConcurrentHashMap would close the class of problem.

F6 — prepareSessionSpace's checkNotNull sits above code that must not throw (theoretical).
SessionSpaceIdentity.kt:17 throws if importWorkspace returns null; it is called from the watcher's saveJob, one line above writeInSessionRecovery, whose KDoc goes to some length to guarantee "an exception escaping into it would end the watcher for the rest of the window's life". In practice importWorkspace returns null only if WorkspaceSerializer.deserialize throws on a string it just serialized, so this is close to unreachable — but the merge is what placed an unguarded line above a function built around that invariant.

---

#1736 × the integrated dev changes

Checked the pairings that could actually interact; no regressions found.

• enableLastSessionSpace (main) × dev's corrupt workspace-settings recovery. Dev's new SerializationException branch in DesktopWorkspaceSettingsManager.loadSettingsSync resets to WorkspaceSettings(settingsVersion = CURRENT), i.e. enableLastSessionSpace = false — the same value a fresh install and every existing file get, so a corrupt settings file lands a user in the documented default rather than a new state. I specifically checked for an ordering race, since prepareSessionSpace reads currentSettings.value synchronously: the load is synchronous in init (:52-59), so there is no window in which a user who enabled the slot is read as having disabled it. Clean.
• visibleWorkspaces (main) has no dev-side caller that collides.
• junit.platform.discovery.issue.severity.critical moved composeApp→allprojects (dev). Main's eight new tests are all in composeApp, which already carried the property, so the move cannot newly break them. The move can surface pre-existing value-returning tests in other modules — that is G2's job, and the property's JUnit-1.13 floor means it is inert in modules that resolve an older platform, as AGENTS.md now records.
• detekt baseline. The four baselined entries touching these files key on signatures and literal strings, not the KDoc the merge edited, so no entry is invalidated. WorkspaceManager's TooManyFunctions and BossAppStartupEffects's LongMethod/CyclomaticComplexMethod are baselined and their signatures are unchanged. The one baseline edit in the diff is a KeymapSettingsManager.loadSettingsSync signature refresh.
• No new over-120-character lines in any touched session-recovery file; the three that exceed it are the pre-existing baselined ones.
• Em-dash guard: clean. No U+2014 on any added *.md/*.html line across the whole origin/main...HEAD diff, so build.yml's added-lines check will not fail.

Spot-checked the dev security changes for merge-induced breakage rather than re-reviewing them: McpArgumentSanitizer's possessive-quantifier fix plus its StackOverflowError backstop, McpPolicyEngine.isProviderDenied (deny-only legacy inheritance, resolved at runtime rather than copied — correct per the AGENTS.md asymmetry rule, and all four former providerRules[x] == DENY sites were converted), McpRiskEvaluator.isFormatCommand (single call site, DESTRUCTIVE_WORDING has exactly one consumer so dropping "format " opens no gap; VOLUME = [a-z]:.* is a full matches so https: cannot satisfy it), and FileSystemDataProviderImpl's Downloads widening (real-path comparison, component-wise Path.startsWith rather than string prefix, root-level Downloads refused, delete deliberately left home-only). Nothing there depends on anything main changed.

---

Release gates

G1 — release notes do not describe this tree. version.properties is 9.5.27 and docs/release-notes/v9.5.27.md exists, both from main; the merge added nothing to either, which matches the PR body. But that file documents #1736 only — I grepped all of docs/release-notes/ for 1713, 1725, 1734, 1737, terminal-relay and terminal broadcast and got zero hits. So the ~90 files of dev content in this tree (#1713 session recovery, #1725 keymap recovery notice, the Cloudflare terminal relay, the MCP ledger CLI, Chromium extraction limits, decode-failure logging, atomic settings writes, relocated-Downloads access) are undocumented. Confirming the PR body: do not dispatch with skip_version_increment=true — the next version must be incremented and its own notes generated from the merged range, not from main's.

G2 — 🔨 Build & Test has not completed at this head. ⚡ Edge Functions ✅ and Filesystem resource limits ✅; 🔨 Build & Test and Authenticated IPC boundaries were still in progress when I finished. I could not compile locally — this runner's JDK is 11 and the build needs 17+, the same limitation the PR body reports — so compilation and the full test suite are unverified by me and G2 is the only signal for them. Worth watching specifically because of the allprojects JUnit discovery-guard move, whose failure mode is loud but module-wide: a DiscoveryIssueException makes every test in the affected module vanish and reports one initializationError in their place.

Nothing here authorizes a merge or a release dispatch, and app crash testing remains separate and pending.
· branch release-integration-20260926

[fluck-boss]

@claude Follow-up at new exact head 46fe12aeb651fb907899e726a9e860939104f18c: the three stale single-Space set KDocs from your review at 88c07f33 have been corrected in BossAppStartupEffects.kt, InSessionRecovery.kt, and LastSessionCoordinator.kt. Please confirm the comment fixes match the code and review this new head for any newly introduced blocker; CI is rerunning. The test suggestion and inherited pair-write/shutdown-order risks remain noted separately; no merge or release dispatch yet.

[claude]

Claude finished @fluck-boss's task in 2m 38s —— View job

---

Follow-up review: release integration #1738 @ 46fe12a

• Scope the new commit (88c07f33..46fe12a)
• Verify the three KDoc fixes match the code
• Check for newly introduced blockers (detekt baseline, ktlint, em-dash guard)
• CI status at this head
• Verdict

---

Verdict

No new blocker. F1 is resolved. 46fe12a is comments only - git diff 88c07f33 46fe12a is 9 insertions / 9 deletions across exactly the three files named, no code, no version.properties, no release notes, and it is an ordinary commit on the branch so merge parentage (3170b381 + 4fa78542) is unchanged. Everything in my 88c07f33 review stands as written; only F1 moves.

F1 - three stale single-Space KDocs	✅ resolved, all three now match the code
Newly introduced blocker at this head	✅ none found
detekt baseline invalidation	✅ none (checked, see below)
ktlint MaxLineLength / em-dash guard	✅ clean
Prior gates G1 (notes) / G2 (Build & Test)	⏳ still open, unchanged by this commit
F2 (no test for the resolution), F3, F4, F5, F6	⏳ still open as previously described

---

The comment fixes match the code

I re-derived the contract from sessionSetOf rather than from the prose:

internal fun sessionSetOf(spaces, activeWorkspaceId): LastSessionSet? =
    activeWorkspaceId
        ?.takeIf { spaces.size >= MINIMUM_SET_SIZE && spaces.any { space -> space.id == it } }
        ?.let { LastSessionSet(activeWorkspaceId = it, spaces = spaces) }

/** Even a single Space needs its original identity on restart. */
private const val MINIMUM_SET_SIZE = 1

composeApp/src/commonMain/kotlin/ai/rever/boss/components/workspaces/LastSessionSet.kt:148-163

So the true rule is: a set for one or more Spaces; null only when the active id is absent, or names no Space in the list (spaces.size >= 1 is subsumed by the any once the id must match, but it is the explicit floor). All three edits now say that:

• BossAppStartupEffects.kt:883 - "Even one Space needs a set to preserve its id; an empty session or unmatched active id produces no set." ✅ Sits directly on liveSessionSet, which is the function whose behaviour the old text denied.
• InSessionRecovery.kt:19 - "One Space produces a set to preserve its id; an empty session or unmatched active id produces null and removes a stale set an earlier session left." ✅ And I verified the removal half of that sentence rather than taking it on trust: writeInSession → saveSet(null) → saveLastSessionSetBlocking(null) → fileManager.writeDocumentBlocking(LAST_SESSION_SET_FILE, null), and writeDocumentBlocking's content = null is the documented remove verb (WorkspaceManager.kt:565-579). The function body's own inline comment already said "a null result is a real answer (remove the set)", so the KDoc and the code comment now agree instead of contradicting.
• LastSessionCoordinator.kt:88-93 (the extractSet param) - "Even one Space produces a set to preserve its id; an empty session or unmatched active id produces null." ✅ extractSet's default is still { null }, which is the "this window supplies no set" case and is unaffected by the wording.

All five sites are now consistent, which was the actual risk in F1 (a reader "fixing" sessionSetOf to match a comment and silently reverting #1736). grep -n 'fewer than two|needs no set|already describes on its own' across the tree returns one hit and it is an unrelated test name (ReloadJarPathTest.kt:262). The two the merge author had already fixed still read correctly: the extractSet call site at BossAppStartupEffects.kt:163-165 and the restore-fallback comment at :610-616, plus WorkspaceManager.loadLastSessionSet's "New builds write a set even for a single Space to retain its id" (:585). MINIMUM_SET_SIZE = 1 is unchanged at this head, and LastSessionSetTest still pins it.

AGENTS.md's own paragraph already reads "A set is written for one or more Spaces. Even one Space needs its own identity restored", so the code comments and the repo spec now say the same thing. Nothing needed there.

---

Nothing newly introduced

I checked the three things a comment-only commit can still break in this repo:

• detekt baseline. This is the real hazard for a KDoc edit - AGENTS.md records that baseline signatures embed KDoc and string text, so editing a comment inside a baselined declaration invalidates its entry. It does not happen here. The only LastSessionCoordinator.kt entry is TooGenericExceptionCaught:LastSessionCoordinator.kt$LastSessionCoordinator$e: Exception, a catch-parameter signature. The BossAppStartupEffects.kt entries key on @Composable internal fun BossAppStartupEffects(state: BossAppState) (Cyclomatic/LongMethod), three MaxLineLength entries keyed on literal lines none of which changed, and one e: Exception. The edited KDoc sits on the private top-level liveSessionSet and on register's extractSet parameter, neither of which appears in the baseline at all. InSessionRecovery.kt has no baseline entries.
• MaxLineLength. No line over 120 characters in any of the three files; the longest added line is ~96.
• Em-dash guard. No U+2014 on any line added by 46fe12a. The em-dashes that exist in LastSessionCoordinator.kt (:25, :28, :262) and BossAppStartupEffects.kt are pre-existing code comments, which AGENTS.md puts out of scope entirely, and the guard only inspects added *.md/*.html lines regardless. The new prose uses spaced hyphens.

---

One cosmetic nit, explicitly not worth blocking on

The phrase "unmatched active id" covers two of sessionSetOf's three null paths precisely and the third only by implication: activeWorkspaceId is splitViewState.currentWorkspaceId, a String?, and a window with no Space loaded passes null, which is absent rather than unmatched. That is the ordinary state the previous review's shutdown-path analysis turns on, so it is the one a reader is most likely to look this comment up for. LastSessionSet.kt's own KDoc words it as "Empty sessions or an absent active id". Purely a wording nit - the behaviour is identical either way and "an id naming no running Space" is a fair reading of the phrase. Fix this →

---

Still open from the 88c07f33 review

Carried forward unchanged, since this commit touches no code:

• F2 - the merge's own resolution (rebind-then-extract ordering, and the currentWorkspace-first identityFor fallback) has no test. Strongest recommendation, and the comment fix does not substitute for it: a comment cannot fail when someone reverses those two lines.
• F3 - writeInSession's saveSet(set); saveRecord(record) block yields saveRecord's value, so a non-throwing false from the set write is dropped and the pair reports success. Inherited from fix(workspaces): stop an earlier session's Space set overruling the crash-recovery record #1713.
• F4 - writeInSession writes set→record, claimAndWrite writes record→set. Both documented as correct.
• F5 - WorkspaceManager.loadedFileNames is a plain LinkedHashMap read across Main and Dispatchers.IO. Pre-existing, newly widened.
• F6 - prepareSessionSpace's checkNotNull sits one line above a function built on "nothing may throw into the watcher".

Release gates

• G1 - unchanged. version.properties is still 9.5.27 and docs/release-notes/v9.5.27.md still documents feat(macOS): add native title-bar navigation and sidebar controls #1736 only; the ~90 files of dev content in this tree remain undocumented. Do not dispatch with skip_version_increment=true.
• G2 - ⚡ Edge Functions ✅ at this head; 🔨 Build & Test, Authenticated IPC boundaries and Filesystem resource limits are in progress (runs started 18:56Z). I still cannot compile locally - this runner's JDK is 11 and the build needs 17+ - so G2 remains the only signal for compilation and the suite, and the allprojects JUnit discovery-guard move is the thing to watch there.

App crash testing remains separate and pending. Nothing here authorizes a merge or a release dispatch.
· branch release-integration-20260926

[fluck-boss]

@claude Re-review release integration #1738 at new exact head 5355cfc830128f42839f329d5d1ab64f708416ae (parents: prior reviewed integration 46fe12ae, current dev 754711e). This adds merged #1740's supabase-kt heartbeat crash containment and its tests, with no merge conflict; inspect its interaction with the integrated tree and verify current main ancestry, version/notes, and any new release blocker. Earlier 46fe12ae review is not a verdict on this head. Exact-head CI is running, and the signed-in crash re-smoke is separate and pending. No merge or release dispatch from this request.

[claude]

Claude finished @fluck-boss's task in 4m 42s —— View job

---

Re-review: release integration #1738 @ 5355cfc8

• Verify parents and current main ancestry
• Scope the new delta (46fe12ae..5355cfc)
• Review fix(crash): swallow supabase-kt heartbeat/disconnect race instead of crashing (#1739) #1740's supabase-kt heartbeat crash containment
• Check interaction with the integrated tree
• Version 9.5.27 / release notes state
• New release blockers, CI at this head
• Verdict

---

Verdict

No new blocker at this head. The #1740 containment is correct and correctly narrow. The two release gates from the prior reviews are unchanged and still unmet; G1 is now slightly worse because #1740 is a user-visible crash fix that this tree's notes also do not mention. Nothing here is a defect in the merged logic.

Merge parentage, main ancestry	✅ verified
Merge contributed no resolution of its own	✅ delta is byte-identical to 754711e6
#1740 containment correctness / narrowness	✅ non-blocking
No existing negative assertion weakened	✅ verified frame-by-frame
detekt baseline / ktlint / em-dash guard	✅ clean
Release notes cover the merged tree	❌ gate (G1), now also missing #1740
Green 🔨 Build & Test at this head	⏳ gate (G2)
F2-F6 from 88c07f33	⏳ still open, untouched by this commit

---

Ancestry and scope

HEAD          5355cfc8   Merge remote-tracking branch 'origin/dev' into HEAD
  parent 1    46fe12ae   (prior reviewed integration head)
  parent 2    754711e6   fix(crash): ... (#1739) (#1740)
                 parent  4fa78542   (the dev head already reviewed)
origin/main   3170b381   ancestor of HEAD: YES

754711e6 has a single parent, 4fa78542, so the dev side advanced by exactly one commit and brought nothing else with it. More usefully: the merge made no decisions of its own. git diff 46fe12ae HEAD and git show 754711e6 are the same three-file, +130/-29 change, so there is no resolution to review here - only #1740's own content, now sitting on the integrated tree. The BossAppStartupEffects.kt / session-recovery reconciliation that the previous two reviews turned on is untouched at this head (git diff 46fe12ae HEAD names no file under app/ or components/workspaces/).

---

The containment is right, and the narrowness is load-bearing

The change splits the two supabase-kt realtime rules out of CrashHandler into CrashIgnorableRealtime.kt (isStaleRealtimeRejoin moved verbatim from private in the object to internal top-level in the same package), and adds one new term to isIgnorable:

isStaleRealtimeRejoin(t) ||
isRealtimeHeartbeatRace(t) ||

composeApp/src/desktopMain/kotlin/ai/rever/boss/crash/CrashHandler.kt:220-221

The new rule requires all three of: exactly IllegalStateException, exactly the message "Websocket not yet initialized", and frames 0-1 being exactly RealtimeImpl.getWebsocket then RealtimeImpl.sendHeartbeat, anchored at index 0 (CrashIgnorableRealtime.kt:53-67). The two-frame prefix rather than three is the one judgement call in the diff, and it is both argued in the KDoc (the third frame varies with coroutine stack recovery) and pinned by a test that asserts an app frame past the prefix stays ignorable (CrashHandlerIgnorableTest.kt:143-150). That is the right shape: the message is only ever produced by the library's own throwing getter, so the frames the rule refuses to look at cannot change who threw.

I checked frame-by-frame that the new term weakens no existing assertion, which was the one way this could have gone wrong - the two rules share frame 0, so a rule matching only getWebsocket would have silently swallowed the cases the rejoin rule's tests exist to keep fatal. They diverge at index 1 (RealtimeChannelImpl.unsubscribe vs RealtimeImpl.sendHeartbeat), so:

• direct unsubscribe without a connection is still a crash (:155) - frames are getWebsocket / RealtimeChannelImpl.unsubscribe, no heartbeat match. Still false. ✅
• other errors on the retry path remain visible (:165) - same, plus wrong type and wrong package variants. Still false. ✅
• the installed-SDK test's directFailure from a real channel.unsubscribe() (:91) - the assertion that actually exercises the live SDK's frames rather than hand-built ones. Still false. ✅
• matching message from application code is still a crash (:231) - real test-class frames, no match. ✅

And the new negatives are the right set: wrong message, wrong type, wrong package, empty stack (:120-142). Both new tests are block-bodied funs, so they return Unit and actually run - the trap AGENTS.md records, and relevant here because this tree moves that discovery guard to allprojects.

Other checks that came back clean:

• No dangling references. isStaleRealtimeRejoin / isRealtimeHeartbeatRace are referenced only from isIgnorable and their own declarations; nothing else in the tree (including the detekt baseline XML) names the moved function.
• detekt baseline intact. CyclomaticComplexMethod:CrashHandler.kt$CrashHandler$internal fun isIgnorable(throwable: Throwable): Boolean is baselined on an unchanged signature, so the extra || term is still suppressed; TooManyFunctions:CrashHandler.kt$CrashHandler is keyed on the object and only loses a member. No baseline entry existed for the removed private function, so nothing is stranded. The new file introduces no finding of its own (two returns, trivial complexity, comments ruleset inactive).
• ktlint. No line over 120 characters in either new or changed file (.editorconfig allows 140); the file ends with a newline.
• Em-dash guard. No U+2014 on any line added by this merge, and no *.md/*.html touched at all, so build.yml's added-lines check is unaffected. The new prose uses spaced hyphens. (The em-dashes that remain in CrashHandler.kt are pre-existing code comments, out of scope by AGENTS.md.)
• Interaction with the integrated tree: none found. The three files touch nothing the dev or main side changed - CrashHandler.kt's isIgnorable was not modified by either parent, and the only other integrated crash-path work in this tree is CrashDisposition/chainOfCauses, which this change consumes unchanged. chainOfCauses is the shared bounded, cycle-guarded walk (CauseChain.kt:16), so extending isIgnorable inherits its twelve-cause bound rather than adding a new unbounded traversal.

---

Findings (all non-blocking)

N1 - the heartbeat rule has no SDK-drift guard, where the rejoin rule does.
composeApp/src/desktopTest/kotlin/ai/rever/boss/crash/CrashHandlerIgnorableTest.kt:78-96

installed SDK delayed rejoin produces the recognized failure drives the real supabase-kt and asserts the resulting failure is recognized, so a version bump that renames or restructures the rejoin path fails a test. The heartbeat rule has nothing equivalent - both its tests build StackTraceElements by hand, and the KDoc correctly says the frames were copied from the #1739 crash trace rather than inferred, which is the best available evidence but is not a guard. The failure mode is the asymmetric-bad one: after a supabase bump, isRealtimeHeartbeatRace silently stops matching and #1739 comes back as a fatal crash dialog with every test still green. That matters more than usual here because libs.versions.toml:47-58 already documents supabase/ktor as a coupled pin someone will bump, and the KDoc names the upstream fix (supabase-kt#1395) as landed-but-unreleased, i.e. a bump is expected. sendHeartbeat is private so it cannot be called from a test, but a reflection assertion that RealtimeImpl declares a method of that name would cost three lines and turn a silent regression into a red test. I could not verify the member against the installed jar myself - no dependency cache on a fresh checkout and no network - so this is also the one claim in the diff I am taking on the author's evidence rather than checking. Fix this →

N2 - the two rules duplicate their matcher exactly (simplification).
composeApp/src/desktopMain/kotlin/ai/rever/boss/crash/CrashIgnorableRealtime.kt:22-67

Both functions carry the same type-and-message guard, the same realtimePackage constant and the same nine-line expected.withIndex().all { … frames.getOrNull(index) … } prefix walk; only the expected list differs. The file's own KDoc says the split exists "so each rule carries its evidence comment beside the match", which is a good reason to keep two declarations and is fully compatible with factoring the matcher into one matchesRealtimeFramePrefix(throwable, expected) - each rule would then be its comment plus its frame list, which is the only part that is actually evidence. As it stands a fix to the walk (say, tolerating a Coroutine boundary frame) has to be made twice, and a third containment would copy it a third time.

N3 - inherited: isIgnorable matching anywhere in the cause chain means an unrelated fault that merely wraps this ISE is swallowed too.
composeApp/src/desktopMain/kotlin/ai/rever/boss/crash/CrashHandler.kt:210-211

The new test deliberately pins RuntimeException("heartbeat tick failed", realtimeHeartbeatRace()) as ignorable, which is right for the real wrapping the library does. The consequence is that any exception with this ISE anywhere in its (up to twelve-deep) cause chain is now also swallowed, including a genuine host fault that captured it as a cause. This is exactly the existing semantics of every other term in isIgnorable - the rejoin rule and TokenExpiredException have the same property, and the KDoc states it - so it is inherited rather than introduced, and mentioned only because this commit widens the set of chain members that can trigger it by one.

---

Version and release notes

G1 - unchanged, and now one item worse. version.properties is still app.version=9.5.27 with app.build.date=2026-09-26; docs/release-notes/v9.5.27.md still opens on #1736 and documents #1736 only. I grepped the whole of docs/release-notes/ for 1739, 1740, supabase-kt heartbeat and heartbeat - the only hits are unrelated older releases (v9.2.2, v9.5.22, v8.17.20, v9.4.9). So this merge adds a user-visible crash fix (BOSS no longer dies with a crash dialog on a realtime heartbeat/disconnect race) to the pile of undocumented dev content already noted at 88c07f33 (#1713, #1725, the Cloudflare terminal relay, the MCP ledger CLI, Chromium extraction limits, decode-failure logging, atomic settings writes, relocated-Downloads access). Confirming the PR body: do not dispatch with skip_version_increment=true; the next version must be incremented and its notes generated from the merged range rather than inherited from main's.

G2 - 🔨 Build & Test has not completed at this head. At the time of writing, for the runs created 19:55Z: ⚡ Edge Functions ✅, and 🔨 Build & Test, Authenticated IPC boundaries and Filesystem resource limits all still in progress. I could not compile or run the suite locally (this runner's JDK is 11, the build needs 17+ - the same limitation the PR body reports and the prior two reviews recorded), so G2 remains the only signal for compilation and tests. Two things specifically worth watching there: the allprojects JUnit discovery-guard move, whose failure mode is module-wide (a DiscoveryIssueException makes every test in the module vanish behind one initializationError), and CrashHandlerIgnorableTest's installed-SDK test, which is the one test in this file that instantiates a real SupabaseClient and so is the most environment-sensitive.

---

Carried forward from 88c07f33 / 46fe12a

Unchanged - this commit touches no code either review covered:

• F1 ✅ resolved at 46fe12a (three stale single-Space set KDocs), re-verified as still correct at this head.
• F2 - the session-recovery merge resolution (rebind-then-extract ordering, currentWorkspace-first identityFor fallback) still has no test. Still the strongest recommendation.
• F3 - writeInSession's saveSet(set); saveRecord(record) block yields saveRecord's value, so a non-throwing false from the set write is dropped and the pair reports success. Inherited from fix(workspaces): stop an earlier session's Space set overruling the crash-recovery record #1713.
• F4 - writeInSession writes set→record, claimAndWrite writes record→set, both documented as correct.
• F5 - WorkspaceManager.loadedFileNames is a plain LinkedHashMap read across Main and Dispatchers.IO.
• F6 - prepareSessionSpace's checkNotNull sits one line above a function built on "nothing may throw into the watcher".

The signed-in crash re-smoke is separate and pending, and nothing in this review authorizes a merge or a release dispatch.
· branch release-integration-20260926