fix(app): the desktop panel is centred on purpose — say so, and test it

Fourth review thread. The doc comment promised "an anchored popover on desktop.
Same split as ContextUsageButton (SPEC-37)" and D11 specified a MenuAnchor; the
code ships showDialog + Alignment.center. So this was not a stale comment, it was
an unrecorded spec deviation.

Kept the centring, and the reason is a real finding rather than a shrug: the
mechanism cannot transfer, because the DOOR TOPOLOGY differs. ContextUsageButton
is a persistent control in the composer, so a MenuAnchor has something to stay
anchored to for as long as the popover is open. Both of this panel's desktop doors
are transient MENU ITEMS -- the pane-header kebab and the mobile glass menu -- so
by the time an item is chosen its menu has been dismissed. Anchoring to where a
vanished menu item used to be is arbitrary placement dressed up as precision.

What SPEC-37 actually contributes is kept: the window-clamped width and the
SingleChildScrollView, so a panel opened from a narrow split pane cannot hang
off-screen.

The real defect underneath: the desktop host had NO test. Every case in
session_identity_widget_test.dart passed `desktop: false`, which is exactly how
the code and its own doc comment drifted apart and stayed that way through
implementation. Two tests added:

  * desktop opens a centred, window-clamped panel -- not a sheet
  * a desktop panel in a narrow window is clamped to the window (300pt window,
    340pt panel, so the clamp is load-bearing)

Both mutation-proven: Alignment.center -> topLeft fails the first, and dropping
the math.min window clamp fails the second.

D11 amended in the spec and logged as deviation 8, so the next reader does not
find a third version of the truth.

Author: leduckhc

app/lib/ui/session/session_identity.dart

@@ -431,8 +431,25 @@ class _CopyAllRow extends ConsumerWidget {
   }
 }
 
-/// Opens the identity panel: a modal bottom sheet on mobile, an anchored
-/// popover on desktop. Same split as `ContextUsageButton` (SPEC-37).
+/// Opens the identity panel: a modal bottom sheet on mobile, a **centred,
+/// window-clamped** panel on desktop.
+///
+/// Centred on purpose, and NOT the `MenuAnchor` popover D11 first specified.
+/// D11 said "verbatim the `ContextUsageButton` split", but the mechanism cannot
+/// transfer, because the door topology is different: `ContextUsageButton` is a
+/// persistent control in the composer, so a `MenuAnchor` has something to anchor
+/// to for as long as the popover is open. Both of this panel's desktop doors are
+/// transient MENU ITEMS (the pane-header kebab, the mobile glass menu) — by the
+/// time an item is chosen its menu has been dismissed, so there is nothing left
+/// on screen to anchor to, and anchoring to where a vanished item used to be is
+/// arbitrary placement dressed up as precision.
+///
+/// What was kept from SPEC-37 is the part that matters and is testable: the
+/// window-clamped width and the `SingleChildScrollView`, so a panel opened from a
+/// narrow split pane cannot hang off-screen. Both properties are pinned by tests
+/// in `test/session_identity_widget_test.dart` ("desktop opens a centred,
+/// window-clamped panel"), which exist because the desktop host previously had no
+/// test at all — which is exactly how the code and this comment drifted apart.
 ///
 /// Pass EXACTLY ONE of [sessionId] or [identity]:
 ///

app/test/session_identity_widget_test.dart

@@ -313,6 +313,77 @@ void main() {
       expect(find.text(kId), findsOneWidget);
     });
 
+    testWidgets('desktop opens a centred, window-clamped panel — not a sheet', (
+      tester,
+    ) async {
+      // The desktop host had NO test at all: every case here passed
+      // `desktop: false`, which is how the panel came to be documented as an
+      // "anchored MenuAnchor popover" (D11) while shipping a centred dialog. It
+      // is centred on purpose — both desktop doors are transient MENU ITEMS, so
+      // by the time one is chosen the menu is gone and there is nothing left on
+      // screen to anchor to (unlike SPEC-37's ContextUsageButton, a persistent
+      // control in the composer). This test pins the presentation that actually
+      // ships, so the doc and the code cannot drift apart again.
+      tester.view.physicalSize = const Size(1400, 900);
+      tester.view.devicePixelRatio = 1.0;
+      addTearDown(tester.view.reset);
+      await tester.pumpWidget(
+        _host(
+          Builder(
+            builder: (context) => ElevatedButton(
+              onPressed: () => showSessionIdentity(
+                context: context,
+                identity: identity(),
+                desktop: true,
+              ),
+              child: const Text('open'),
+            ),
+          ),
+        ),
+      );
+      await tester.tap(find.text('open'));
+      await tester.pumpAndSettle();
+      expect(find.byType(BottomSheet), findsNothing);
+      expect(find.byType(SessionIdentityDetails), findsOneWidget);
+      // Centred: the panel's centre sits on the window's centre, both axes.
+      final panel = tester.getRect(find.byType(SessionIdentityDetails));
+      expect(panel.center.dx, moreOrLessEquals(700, epsilon: 1));
+      expect(panel.center.dy, moreOrLessEquals(450, epsilon: 1));
+      // Window-clamped, never wider than the fixed panel width (SPEC-37).
+      expect(panel.width, lessThanOrEqualTo(kIdentityPanelWidth));
+    });
+
+    testWidgets('a desktop panel in a narrow window is clamped to the window', (
+      tester,
+    ) async {
+      // The SPEC-37 lesson for the desktop host: a fixed-width panel opened from
+      // a narrow split pane hung off-screen. 300pt is narrower than the panel's
+      // 340pt, so the clamp is what keeps both margins.
+      tester.view.physicalSize = const Size(300, 700);
+      tester.view.devicePixelRatio = 1.0;
+      addTearDown(tester.view.reset);
+      await tester.pumpWidget(
+        _host(
+          Builder(
+            builder: (context) => ElevatedButton(
+              onPressed: () => showSessionIdentity(
+                context: context,
+                identity: identity(),
+                desktop: true,
+              ),
+              child: const Text('open'),
+            ),
+          ),
+        ),
+      );
+      await tester.tap(find.text('open'));
+      await tester.pumpAndSettle();
+      final panel = tester.getRect(find.byType(SessionIdentityDetails));
+      expect(panel.left, greaterThanOrEqualTo(0));
+      expect(panel.right, lessThanOrEqualTo(300));
+      expect(panel.width, lessThanOrEqualTo(300 - 2 * 12));
+    });
+
     testWidgets(
       'at 320x360 nothing paints off-screen and Copy all is still reachable',
       (tester) async {

docs/specs/2026-08-11-SPEC-52-PLAN.md

@@ -243,3 +243,4 @@ Per `makit-transcript-row-qa-harness`:
 | 5 | D12 (identity section inside the usage panel) **cut** from P1 to P2. | The ring is absent in four states including the likeliest moment of need, so that door is missing exactly when wanted; it also added an import edge and mixed two row styles in one panel. | Spec D12, rev 2. |
 | 6 | Three panel doors → **two**; the desktop tab menu keeps only *Copy session id*. | A tab-menu *Session details…* duplicates the pane-header kebab one pixel away on the same platform. | Spec D13, rev 2. |
 | 7 | Added D18 (a11y), D19 (watch not snapshot), D20 (i18n), D21 (path disclosure). | Review found a11y absent where SPEC-47 had locked it; a panel opened before the id is assigned would have shown stale rows. | Spec rev 2. |
+| 8 | D11's desktop host is a **centred, window-clamped panel**, not the specified `MenuAnchor` popover. | The mechanism cannot transfer: `ContextUsageButton` is a persistent composer control, so a `MenuAnchor` stays anchored to it, whereas both of this panel's desktop doors are transient menu items — by the time one is chosen the menu is dismissed and there is nothing left to anchor to. Found by review of the shipped code, not by a test, because the desktop host had **no test at all** (every case passed `desktop: false`) — which is how the code and its own doc comment drifted apart. | Spec D11 amended; two desktop tests added, both mutation-proven (alignment → topLeft, and dropping the clamp). |

docs/specs/2026-08-11-SPEC-52-session-identity.md

@@ -107,7 +107,7 @@ them. **The absence rule is not weakened** (D12) — the panel is reachable with
 | **D8** | The panel is **read-only**. No rename, no regenerate, no delete, no resume button. | Lifecycle already lives in the same menus (Close / Quit agent) and must not sit one mis-tap from the copy row. |
 | **D9** | A row whose value is unmeasured is **omitted**, never rendered blank or as a placeholder. `Copy all` then copies fewer lines. | Same rule SPEC-37 settled on for the ring. A fabricated path is worse than no path: it will be pasted into a prompt and the next agent will report it missing. |
 | **D10** | Per-agent vocabulary is a **lookup table** keyed by `SessionDTO.agent` (not a `switch`): pi → label `pi session`, resume `pi --session <id>`; codex → label `Thread`, resume `codex resume <id>`; anything else → label `Agent session`, **no** resume row. | Codex's own word for it is a thread (`thread/start` → `thread.id`). An unknown ACP agent gets no resume line because we do not know its CLI — inventing one is D9's failure mode in command form. A table rather than a `switch` because `docs/ENGINEERING.md`'s OCP rule is explicit: adding an adapter must not mean editing a growing `switch`. The safe default *is* the open/closed escape hatch — a third agent works unedited, just without a resume line. |
-| **D11** | One host-agnostic body (`SessionIdentityDetails`) presented as a modal bottom sheet on mobile and a `MenuAnchor` popover on desktop. | Verbatim the `ContextUsageButton` / `ContextUsageDetails` split (`context_usage.dart:199-300`), including the window-clamped width and the `SingleChildScrollView`. |
+| **D11** | One host-agnostic body (`SessionIdentityDetails`) presented as a modal bottom sheet on mobile and a **centred, window-clamped panel** on desktop. | Originally specified as a `MenuAnchor` popover, "verbatim the `ContextUsageButton` / `ContextUsageDetails` split (`context_usage.dart:199-300`)". **Amended on review of the implementation (deviation 8):** the mechanism does not transfer, because the door topology differs. `ContextUsageButton` is a *persistent* composer control, so a `MenuAnchor` has something to stay anchored to; both of this panel's desktop doors are *transient menu items*, and by the time one is chosen its menu is gone. What the split actually contributes — the window-clamped width and the `SingleChildScrollView`, so a panel opened from a narrow split pane cannot hang off-screen — is kept and is now pinned by tests. |
 | **D12** | **CUT from P1 → P2.** No `SessionIdentitySection` inside `ContextUsageDetails`. The ring's absence rule (`context_usage.dart:218`) is untouched, as before. | Cut on review, and the argument is this spec's own: the ring is absent in the four states above, *including the likeliest moment of need*, so a door hung off it is missing exactly when it is wanted — near-zero marginal value on top of the two menu doors and `/session`. It is not free either: it adds a `session_identity → context_usage` import edge, and it would put stacked mono rows beside `_Row`'s label/value rows in one panel. Deferring also deletes a whole task whose test was checking the wrong invariant (see §Review findings). |
 | **D13** | **Two** panel doors, in menus that already exist and are always present: mobile `_glassMenu` (`session_screen.dart:505`) and the desktop pane-header kebab (`pane_header.dart:154`). The desktop **tab** menu gets **Copy session id** only. | Cut from three on review: a tab-menu *Session details…* is redundant with a pane-header kebab one pixel away on the same platform. The tab menu keeps **Copy session id** because that is a different job (right-click → one click → done), not a second way to open the same sheet. Zero permanent chrome either way; SPEC-40's 375 pt crowding is untouched. |
 | **D14** | The clipboard payload is produced by one **pure** function, `sessionIdentityText()`, shared verbatim by the panel, `/session` and both menus. Format: one `label: value` per line, labels padded to a common width, absolute paths, omitted rows absent. | Pure ⇒ unit-tested directly, the same seam `formatTokens` / `headroomLabel` use. One function ⇒ the copy contract cannot diverge between four call sites. Plain lines survive being pasted into a prompt, a commit message, an issue or a terminal comment; markdown or JSON would need escaping. |