Copy a session's id, and its transcript, mid-turn (SPEC-52) - #164
Conversation
Copy the underlying agent session id (pi's ACP sessionId / codex threadId) and its on-disk transcript path, mid-turn. Typing pi's /session gets queued behind the running turn; a client command and a menu never do. Spec is rev 2: two independent reviews ran against rev 1 before any code. They caught three things worth having: - D3 was unbuildable. Session has no project path, and pi's transcript slug follows the WORKTREE cwd, not the project root. The nearest precedent (attachPiSession) uses the project path, so copying it would have missed the transcript for every worktree-bound session -- and D9 would have hidden the failure. Resolution moves to SessionManager, keyed on worktreePath ?? project.dto.path, memoized (a readdir per snapshot was unaffordable). - D15: the mockup's 'pi --session 019ff121' was unsafe. pi ids are UUIDv7, whose first 48 bits are a ms timestamp, so 8 chars leave ~65s of ambiguity; real collisions exist on this machine. Review went further and drove it: pi does not error on an ambiguous prefix, it silently picks one and offers to fork it. Full id everywhere. - D4's '~/' abbreviation was unimplementable: the path belongs to the server host and the app cannot know that host's home dir. Cut on review: the identity section inside the context-usage panel (the ring is absent in exactly the four states where the id is wanted), and one of the three panel doors (a tab-menu item duplicating the pane kebab). Added on review: a11y as a locked decision (D18), watch-not-snapshot so a panel opened before the id is assigned fills in live (D19), and the server-path disclosure written down rather than assumed (D21).
Phase A of the plan: build and pixel-verify the whole surface BEFORE freezing
the wire contract. The widgets take an app-level SessionIdentity, never a
SessionDTO, which is what makes that order possible -- and is the right
dependency direction anyway.
SessionIdentity per-agent vocabulary as a lookup TABLE, not a switch
(ENGINEERING.md's OCP rule), with a safe default that
gives an unknown agent a generic label and no resume
line rather than an invented CLI.
sessionIdentityText() pure; THE copy contract, one place, four callers.
SessionIdentityDetails one Copy all (D5), rows omitted when unmeasured (D9),
read-only (D8), semantics labels (D18).
showSessionIdentity() sheet on mobile, window-clamped popover on desktop.
18 tests, each with a mutation proven to fail it. Two of those mutations
initially did NOT bite, and both were my bugs rather than the tests':
- the clipboard-failure test asserted on rendered toast text, but the test host
has no toast overlay, so findsNothing could never fail. It now asserts against
the StatusCenter directly.
- two mutation scripts silently no-oped on a Python escaping bug, which looked
exactly like a vacuous test until checked.
Found by the pixel gate on the real macOS app, not by any test: "1 lines".
Reachable in production on a stub/detached session whose only measured value is
the makit id. Fixed, and the 1-line case is now asserted in both the visible
label and the semantics label.
Also fixes a real bug in test/status/status_lifetime_test.dart (SPEC-48 D3's
repo-wide guard): it sliced the body out of the ORIGINAL source, so a comment
naming the rule it follows supplied the "first await" and turned the
correctly-hoisted line below it into an offender -- and a commented-out
ref.status counted as a real one. It now scans the blanked copy; offsets are
preserved so line numbers still map. Red test added for the false positive, and
the guard is proven still to bite by un-hoisting.
QA evidence (cua-driver, real macOS window, both themes, 320 and 375pt):
both uuid rows render on ONE line -- the measured payoff for dropping the
per-row copy column; light-mode Copy all measures 10.95:1 (needs 4.5); line
counts 4/3/3/2/1 all correct; draft renders 'Agent not started yet'.
Note for anyone repeating that gate: Flutter's macOS AX tree exposes nothing but
the menu bar, and synthesized clicks never reached the app (the staleness stamp
proved it), so the harness renders every state in one pass and needs no input at
all. scroll DID work.
Mockup corrected to the shipped type scale (sentence case, as
ContextUsageDetails ships) and its in-panel footnotes dropped.
Two OPTIONAL fields on SessionDTO, and their app-side decode. Committed on its own, before either implementation, so the parallel server/app work cannot both edit the shared shape. Frozen spelling: `agentSessionId?: string`, `transcriptPath?: string`. Optional is the decision, not an accident: a newer app paired with an older server must render one fewer row rather than a fabricated one, which is the rule createdAt already follows (SPEC-47 D12). App side normalises '' to null at the decode edge. That is not paranoia -- '' is what a sloppy or partially-migrated server sends for "no value", and an empty string here would render a copy affordance that copies nothing, i.e. exactly the placeholder D9 forbids. Non-strings are rejected the same way, so a malformed snapshot degrades one field instead of failing the whole session list. Nothing populates the fields yet -- that is C1b, and its tests are where the bite lives. The vacuous "a DTO built without them is undefined" test from plan rev 1 is deliberately NOT here: optional TS fields are absent by default with zero production code, and JSON.stringify drops undefined. server: tsc clean, 1313 pass / 0 fail (baseline preserved) app: analyze clean, 5 new codec tests green
Two agents on disjoint trees, then verified here rather than taken on report.
server (C1):
transcript-path.ts NEW. Agent-agnostic dispatcher: resumeSessionPath
verbatim -> pi-only directory scan -> undefined. In its
own module because two of its three branches are not
pi-specific, so P2's codex resolver is additive rather
than a move out of a pi-named file. Never throws.
manager.ts projectSessionDTO() populates both fields at the two
wire sites (sessions.snapshot, session.listClosed).
Session.toDTO() CANNOT do this -- a Session holds
projectId + worktreePath, never the project's FS path.
cwd = worktreePath ?? project path, because pi's slug
follows the cwd pi actually ran in and that is usually
the worktree; the nearest precedent (attachPiSession)
uses the project path and would have missed every
worktree-bound session, with D9 hiding the failure.
Memoized per session id including misses: a snapshot is
rebroadcast on every metaChanged, so resolving per
projection meant a readdir per session per broadcast.
app (C2):
sessionIdentityProvider store -> SessionIdentity; null FIELDS, never null,
never throws, so no call site has to branch.
/session a CLIENT command next to /name, so it is
intercepted before sendMessage and works MID-TURN.
That is the whole bug: pi's own /session is an
agent command and gets queued behind the turn.
`/session id` copies the bare id; bare opens panel.
two doors + tab copy glass menu, pane kebab, and Copy session id on the
tab menu (not a third panel door -- cut on review).
showSessionIdentity is now exactly-one-of {sessionId, identity}, asserted. The
first wiring took a required identity AND an optional sessionId, so all three
doors did a ref.read whose result was then discarded: an argument that is
ignored depending on another argument is a trap, not an API.
Mutations proven to bite (9 across both trees), including the two that matter
most: cwd -> project path fails the worktree test, and a prefix-relaxed suffix
match fails the D15 collision test.
server: tsc clean, 1326 pass / 0 fail (baseline 1313)
app: analyze clean, 0 non-loading failures, 55 SPEC-51 tests green
Phase D step 5. The Verification section said "filled in on implementation" and the status still said Draft; both are now true statements about the shipped code. The one gate that had no recorded result was the live probe, so it was run: resolveTranscriptPath against the real ~/.pi/agent/sessions tree, for this branch's own worktree, using the pi session id of the session that implemented the feature. It returned the transcript path byte-identical to the one pi itself reports. The same call with the 8-char prefix returned undefined -- D15 holds on real data, not just in the unit test's fixture dir. `pi --help` confirms the resume line's spelling is `--session <path|id>`. Also noted the tenth mutation: relaxing handleClientCommand's exact name match to startsWith, which `/sessions` is NOT intercepted catches. That mutation was left in the working tree by the previous session and has been reverted -- exact matching is the shipped behaviour. Status is Implemented for P1 only; the codex resolver branch and D12's section inside the usage panel stay deferred, so the header says so rather than implying the whole spec landed.
Whitespace and trailing commas only — verified by comparing both revisions with all whitespace and commas stripped, so no token changed. `dart format --set-exit-if-changed` is now clean across lib/test/tool, analyze reports no issues, and the 52 SPEC-51 tests in these files stay green. The pre-push hook writes these fixes itself and then fails the push, so they have to land as a commit rather than as a dirty tree.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_64ebd4ae-bb05-4e54-9b39-0b08066b0ec7) |
|
Warning Review limit reached
Next review available in: 4 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (25)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Four conflicts, all of one shape: main and this branch each added optional
fields/methods in the same place. Resolved as a union, main's first.
protocol.ts SessionDTO gains SPEC-46's parentId/handoffReason/origin next to
this branch's agentSessionId/transcriptPath.
models.dart the same union in four places -- ctor, fields, copyWith params,
copyWith body.
codec.dart both decode blocks.
manager.ts SPEC-46's lineage/readTranscript methods and this branch's
projectSessionDTO/transcriptPathFor.
Checked rather than assumed: SPEC-46 added a wire path of its own (server.ts
visibleSessions, which filters the snapshot per principal), so the question was
whether any NEW SessionDTO site bypasses projectSessionDTO and would therefore
ship without the two new fields. It does not -- session.toDTO() is still called
from exactly one place, projectSessionDTO, and visibleSessions filters
manager.listSessions() rather than building DTOs itself.
Worth recording because the two features interact: an agent-token principal now
receives a filtered snapshot, so it sees transcriptPath for its own session only.
That is inside D21's disclosure (an agent already knows its own cwd) and narrower
than the paired-device case D21 actually argued.
server: tsc clean, 2065 pass / 0 fail
app: analyze clean, 0 non-loading failures
…the menu
Three review findings, each with a red test written first.
1. HIGH -- `/session id` awaited Clipboard.setData with no try/catch, so a
PlatformException (another process holds the clipboard on Windows; the host
denies the write) escaped the handler: the user got neither the id nor a word
about why. The panel's `Copy all` already had this guard; the two bare-id paths
did not.
The review flagged client_commands.dart. The IDENTICAL bug sat in
split_view.dart's tab-menu copy, unflagged, so both are fixed -- three copy
paths in one feature must not disagree about what a failed write does.
All three now report `failure`, not `warning`: an action the user asked for did
not happen, which is what status.failure is for. The panel's `warning` was the
odd one out and moved too. Severity is now ASSERTED in all three tests, so the
consistency is enforced rather than incidental -- flipping any one back to
warning fails its test (proven).
2. HIGH -- the tab menu's ref.read(sessionIdentityProvider) ran AFTER the showMenu
await. That menu lives in the Navigator's overlay, so it outlives the _TabChip
that opened it: close the tab while the menu is open (a server snapshot dropping
the session does it for real) and the read hits a dead ref --
`Bad state: Using "ref" when a widget is about to or has been unmounted is
unsafe.` A crash instead of a copy. The new test reproduces it exactly that way,
by calling unbindSession while the menu is up, and it failed with that message
before the fix.
Hoisted next to `status`, which is already hoisted for this very reason
(SPEC-48 D3). Chose that over the reviewer's alternative of a context.mounted
guard: the guard keeps the read fresh but still leaves `ref` use after an await,
which is the hazard D3 exists to remove. The cost is that the id is sampled at
menu-open rather than at click -- sub-second for right-click -> click, and the
surface that must fill in live is the panel, which watches (D19).
Measured rather than assumed: broadening status_lifetime_test.dart to catch
ref.read/ref.watch after an await repo-wide -- which would have caught this
automatically -- reports 24 bare sites across lib/. Out of scope here, so it is
named as a follow-up instead of pretending this PR fixed a repo-wide pattern.
3. LOW -- three stale statements in mockups/session-identity.html, each of which
misleads someone reading it as the design source:
- "Three things on this page were wrong" above a list of six;
- prose still claiming the panel DISPLAYS `~/` paths, contradicting both the
rendered frames and the amendment 20 lines above it (D4 dropped `~/`);
- the delta table still promising `Session details...` in the tab menu -- the
fourth door D13 explicitly cut. That is the dangerous one: a delta table
reads as a work list.
Markup re-validated (balanced tags) after the edits.
Not a cosmetic rename. While this branch was in flight, `main` took BOTH numbers next to it: SPEC-51 Preview groups (#163) -- docs/UX.md + 14 shipped source files SPEC-50 Profiles (#162) -- a whole server-profiles feature So `SPEC-51` in this branch's code comments pointed at a different, already-shipped feature, and `docs/specs/` held two SPEC-51 files. Neither shipped spec can move, so this one takes 52, the next free number. Renumbered ONLY this feature's own files, derived from this branch's commits (1b92874..2a95bf7 plus the fix commit) rather than by a blanket search-replace: a global `SPEC-51 -> SPEC-52` would have rewritten preview groups' own references in 14 files and docs/UX.md, silently pointing them at this spec instead. Each of the remaining 14 SPEC-51 references was checked to be preview groups'. The branch name (feat/get-session-id) is left alone deliberately -- renaming a pushed branch orphans its PR for no gain. Recorded in the spec header, including the part a rename does not fix: nothing in this repo allocates spec numbers, so two branches drafted in one week collide in silence. `main` already carries that proof -- SPEC-48 names both 2026-08-09-SPEC-48-status-and-activity.md and 2026-08-10-SPEC-48-per-repo-settings.md. Verified after the renumber, on the merge with the NEWER main: server: tsc clean, 2072 pass / 0 fail app: analyze clean, dart format clean, 0 non-loading failures
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f1fa2a7b-61d7-42ef-8763-df11b81268c8) |
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.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a0f2b12c-ad49-400f-a864-ca3b66cb30b7) |
Fifth review thread, and it was right: window - 2 * margin goes NEGATIVE once the
window is narrower than the margins, and a BoxConstraints/SizedBox with a negative
max is not normalized -- the layout ASSERTS ("BoxConstraints has both width and
height constraints non-normalized") instead of rendering a cramped panel. "The
window is absurd" should cost an ugly panel, not a crash.
Confirmed by repro before fixing, and the repro is the interesting part. My first
attempt -- open the panel in a 20x20 window -- PASSED while proving nothing: at
that size the trigger button is unhittable, so the tap lands on nothing and the
panel never opens. A test that cannot fail. Both tests now shrink the window WHILE
the panel is open, which is also the realistic path: a resize animation or an
embedded host hands us one degenerate frame.
session_identity.dart maxWidth AND maxHeight floored at 0 (threshold 24pt,
margin 12).
context_usage.dart the SPEC-37 panel this one copied its sizing from has
the SAME bug on the width axis, at a 16pt threshold
(margin 8). Its height axis was already safe, floored by
_kUsagePanelMinHeight; the width axis had no floor. Not
in this PR's diff, but confirmed real with a red test, so
it is fixed here per AGENTS.md rather than left for
someone to hit.
I nearly recorded that second one as "not reproducible": my first attempt used a
20pt window, which is negative for a 12pt margin but still POSITIVE for an 8pt
one, so the assert did not fire. It needed 10pt. Worth stating because the
near-miss is the whole lesson -- a threshold bug needs a size chosen from the
constant, not reused from the previous test.
Same coverage hole under both: neither panel had a single `desktop: true` test, so
neither desktop host was covered at all. That is also how the identity panel's doc
comment came to describe an anchored popover while shipping a centred dialog.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_bc9ecd03-7e95-40de-b8e8-45561322f7da) |
Typing pi's own
/sessioninto makit's composer does not answer you — it is an agent command, soit falls through to
store.sendMessageand, mid-turn, lands in the server's pending queue. Itexecutes only after the turn it was meant to help you hand off. This branch makes session identity
something makit itself can answer, at 100% of a turn.
What changed
A panel (
app/lib/ui/session/session_identity.dart) showing the agent session id, its on-disktranscript path, a ready-to-paste resume command, and the makit session id — one
Copy all, rowsomitted when unmeasured rather than shown as placeholders, read-only, with semantics labels. Per-agent
vocabulary is a lookup table, not a switch, so an unknown agent gets a generic label and no
invented CLI.
Three doors, deliberately not four: the mobile glass menu, the desktop pane-header kebab, and
Copy session idon the desktop tab menu. A tab-menu Session details… was cut on review — it wouldhave duplicated the pane kebab one pixel away on the same platform.
/sessionas a CLIENT command, registered next to/name, sohandleClientCommandintercepts itbefore the wire. Bare
/sessionopens the panel;/session idcopies only the bare id. Thatinterception is the bug fix.
Two optional wire fields —
agentSessionId?andtranscriptPath?onSessionDTO— frozen intheir own commit before either implementation, so the parallel server/app work could not both edit the
shared shape. Optional is the decision, not an accident: a newer app against an older server renders
one fewer row instead of a fabricated one. The app normalises
''tonullat the decode edge,because an empty string would render a copy affordance that copies nothing.
Server-side resolution in a new
server/src/transcript-path.ts:resumeSessionPathverbatim →pi-only directory scan →
undefined, never throwing. It is keyed onworktreePath ?? project path,because pi's transcript slug follows the cwd pi actually ran in, which is usually the worktree — the
nearest precedent (
attachPiSession) uses the project path and would have missed the transcript forevery worktree-bound session. Memoized per session id including misses, since a snapshot is
rebroadcast on every
metaChangedand resolving per projection meant areaddirper session perbroadcast.
Why the full id, and no
~/The mockup showed
pi --session 019ff121. pi session ids are UUIDv7, whose first 48 bits are amillisecond timestamp, so 8 chars leave ~65s of ambiguity — and real same-prefix collisions exist on
this machine. Driving the real binary settled it:
pi --session <ambiguous prefix>does not error,it silently resolves to one session and offers to fork it. Full id everywhere, and the transcript
suffix match is an exact
_<id>.jsonl, never a prefix. The~/display abbreviation was droppedbecause the path belongs to the server host, whose home dir the app cannot know.
How it was tested
tsc --noEmitclean;pnpm test1326 pass / 0 fail (baseline 1313).flutter analyze --fatal-infos --no-pub→ "No issues found!";dart format --set-exit-if-changedclean; 0 non-loadingfailures influtter test. The 15–17 reportedfailures are all
loading <file>, vary run to run, and each passes when run directly — the recordedflake baseline, not a regression. 71 SPEC-52 tests green.
line:
cwd→ project path fails the worktree test; a prefix-relaxed suffix match fails thecollision test;
handleClientCommand's exact match →startsWithfails "/sessionsis NOTintercepted". Two mutations that initially did not bite were fixed as real bugs in the tests.
app/tool/session_identity_demo.dart, geometry read from the accessibility tree withcua-driver,both uuid rows measured at one line,
Copy allat 10.95:1 contrast. That gate — not any test — iswhat caught the "1 lines" pluralisation bug.
resolveTranscriptPathagainst the real~/.pi/agent/sessionstree, using the pisession id of the session that implemented this feature, returned the transcript path
byte-identical to what pi itself reports. The 8-char prefix returned
undefined. Non-pi, draft andunreadable-dir inputs returned
undefinedwithout throwing.Scope
P1. Deferred to P2: the codex/
threadIdresolver branch (additive totranscript-path.tsby design)and D12's identity section inside the context-usage panel — cut because the ring is absent in exactly
the four states where the id is most wanted.
Spec:
docs/specs/2026-08-11-SPEC-52-session-identity.md· plan:docs/specs/2026-08-11-SPEC-52-PLAN.md· design board:mockups/session-identity.htmlNote
Medium Risk
Touches the session wire contract and SessionManager DTO projection, and discloses absolute server filesystem paths to paired clients. Well-covered by tests, but the surface area is large and path resolution sits on the hot snapshot path.
Overview
Makes session identity answerable mid-turn by intercepting
/sessionas a client command (instead of queuing behind the agent) and exposing a read-only identity panel with one Copy all.Panel + doors. New
session_identity.dartshows agent id, transcript path, resume command, and makit id — omitting unmeasured rows. Opened from the mobile glass menu and desktop pane kebab; the tab menu gets Copy session id only (bare id, no dialog). Bare/sessionopens the panel;/session idcopies just the id.Wire + server. Adds optional
agentSessionId/transcriptPathonSessionDTO. The server resolves pi transcripts intranscript-path.ts(exact_<id>.jsonlsuffix, keyed on worktree cwd, memoized including misses) and projects them throughSessionManager. Empty strings normalize to null at the app codec edge.Also floors usage-panel width at zero so a shrink-under-open popover cannot assert, and hardens clipboard-failure / disposed-
refhandling on all three copy paths.Reviewed by Cursor Bugbot for commit 8873c6a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add session identity panel with clipboard copy for session ID and transcript path
SessionIdentityDetailspanel (bottom sheet on mobile, anchored popover on desktop) surfacing session ID, transcript path, agent-specific vocabulary, and resume commands viashowSessionIdentity./sessionclient command (/session idcopies the bare agent session ID to clipboard).StatusCenter.SessionDTOand theSessionmodel withagentSessionIdandtranscriptPathfields; the server resolves transcript paths forpisessions using exact UUID suffix matching inresolveTranscriptPath.ContextUsageButtonwhere the popover width could go negative on very small windows.Macroscope summarized 8873c6a.
Updates since the PR was opened
Merged
main(873a121a). Four conflicts, all "both sides added optional fields in the sameplace", resolved as a union:
SessionDTOnow carries SPEC-46'sparentId/handoffReason/originalongside this branch's two fields, and
manager.tskeeps both sides' methods. The question worthchecking was whether SPEC-46's new wire path (
server.ts visibleSessions) bypassesprojectSessionDTOand would therefore ship without the two new fields — it does not:session.toDTO()still has exactly one caller, andvisibleSessionsfiltersmanager.listSessions()rather than building DTOs. The two features do interact, and it is benign:an agent-token principal now gets a per-principal-filtered snapshot, so it sees
transcriptPathforits own session only — inside D21's disclosure and narrower than the paired-device case D21 argued.
Three review threads fixed (
8809cfc9), each with a red test first:/session idswallowed a throwingClipboard.setData. Fixed — and the identical bug insplit_view.dart's tab-menu copy, which review did not flag, because three copy paths in onefeature must not disagree about what a failed write does. All three now report
failure(thepanel's
warningwas the odd one out); severity is asserted in all three tests.sessionIdentityProviderafter theshowMenuawait, so closing the tab whilethe menu was open crashed with
Bad state: Using "ref" … unmounted is unsafe. Hoisted next tostatus, which is hoisted for that same reason (SPEC-48 D3), rather than guarded withcontext.mounted— the guard would leaverefuse after an await, which is the hazard D3 removes.D13 cut. A delta table reads as a work list, so that one could have put the door back.
Renumbered SPEC-51 → SPEC-52 (
1a0bb8a1). While this branch was in flight,maintook bothadjacent numbers: 51 by Preview groups (#163, referenced from
docs/UX.md+ 14 shipped files) and50 by Profiles (#162). Neither shipped spec can move, so this one took 52. Renumbered only this
feature's own files, derived from this branch's commit range rather than by blanket search-replace —
a global replace would have silently repointed preview groups' own 14 references at this spec.
Verification re-run on the merged tree (the earlier numbers were pre-merge):
tscclean, 2072 pass / 0 fail.analyze --fatal-infosclean,dart format --set-exit-if-changedclean, 0 non-loadingfailures.
strictEqual, expected4) and didnot recur in seven subsequent full-suite runs; it is not in this branch's files, and
transcript-path.test.ts+manager.test.tswere run 12 consecutive times (110 pass) to be sure.Flagging rather than burying it.
status_lifetime_test.dartfromref.statustoany
ref.read/ref.watchafter an await would have caught finding feat: rich tool detail views for read/write/bash/grep #2 automatically, but reports24 pre-existing bare sites across
lib/— too wide for this PR.Round 2 (conflicts + five review threads)
Two more review threads arrived on the fixes and both were right, so they are fixed too
(
6914b5c3,8873c6aa):MenuAnchorpopover" (D11) but ships a centreddialog. Kept the centring, with a reason: the mechanism cannot transfer, because the door topology
differs.
ContextUsageButtonis a persistent composer control, so aMenuAnchorstays anchoredto it; both of this panel's desktop doors are transient menu items, so by the time one is chosen
its menu is gone and there is nothing left to anchor to. D11 amended and logged as deviation 8.
window - 2 * margingoes negative below the margin width, and a negativeBoxConstraints/SizedBoxis non-normalized — the layout asserts instead of rendering a cramped panel. Floored atzero. The SPEC-37 panel this one copied its sizing from has the same bug on the width axis (16pt
threshold vs 24pt here; its height axis was already floored), so that is fixed here too, with its
own red test.
Both threads share one root cause worth naming: neither panel had a single
desktop: truetest, soneither desktop host was covered at all. That is how the doc comment and the code drifted apart and
stayed that way through implementation. Four desktop tests added (centred, window-clamped, narrow-window
clamp, degenerate-window no-throw), all mutation-proven.
Two self-corrections from this round, recorded because the near-misses are the lesson:
unhittable, so the tap landed on nothing and the panel never opened. Both tests now shrink the window
while the panel is open, which is also the realistic path.
context_usage.dartbug as "not reproducible" by reusing that 20pt window: it isnegative for a 12pt margin but still positive for an 8pt one. It needed 10pt. A threshold bug needs a
size derived from the constant, not inherited from the previous test.
All five review threads are resolved with replies. All 13 CI checks pass.