mirror of
https://github.com/stablyai/orca.git
synced 2026-10-26 08:02:44 +00:00
docs(ssh): the group folds are not redundant, and the E2E is weaker than it reads
Round 10 concluded the tabGroups/tabGroupLayouts/activeGroupIdByWorktree folds were dead because short-circuiting them left the E2E green. That was true only of the E2E's shape — one group, one active tab, no split. Hydrating a real session both ways shows four things adoptGrouplessTabs cannot reconstruct: the multi-group split arrangement with its direction and ratio, the user's tabOrder (it degrades to sortOrder), which group and tab were focused, and the Ctrl+Tab MRU. And buildHydratedTabState takes the unified path only when tabGroups is present, so dropping the fold sends the upgrade cohort down the legacy hydration path entirely. The comment now says what they preserve instead of a rationale that was disproved. Also records the known asymmetry — tabGroups is whole-record local-wins while unifiedTabs is now per-tab — as a follow-up: same shape as the unified-ledger defect but with a soft landing (wrong group and order rather than invisible). And states plainly in the spec what it does NOT prove: 'every pane attached' covers only the ACTIVE tab, and a bare shell count cannot distinguish reuse from kill-and-respawn. A verified strengthening patch exists but its [data-tab-id] locator does not resolve in this tree; left as follow-up rather than landed red.
This commit is contained in:
+12
-5
@@ -2950,11 +2950,18 @@ export function hoistSshPartitionsIntoLocalSession(
|
||||
}
|
||||
changed = true
|
||||
}
|
||||
// The tab bar renders from the active GROUP's tabOrder, not from tabsByWorktree, so a tab
|
||||
// folded in without its group exists in the session and is still invisible. That is the whole
|
||||
// reported symptom, and the Store-level oracle cannot see it because persistPtyBinding only
|
||||
// consults tabsByWorktree. Groups, their split layout, the unified tab records and the focused
|
||||
// group all have to travel with the panes.
|
||||
// NOT redundant with `adoptGrouplessTabs`, which mints ONE group and a single leaf. These
|
||||
// carry what it cannot reconstruct: the multi-group split arrangement (direction and ratio),
|
||||
// the user's tabOrder (it degrades to sortOrder), which group and tab were focused, and the
|
||||
// Ctrl+Tab MRU. And `buildHydratedTabState` takes the unified path only when `tabGroups` is
|
||||
// present at all (tabs-hydration.ts) — without this fold, a profile whose tab state came
|
||||
// entirely from the partition drops into the legacy hydration path, which is a different code
|
||||
// path for exactly the upgrade cohort.
|
||||
//
|
||||
// Known asymmetry, follow-up rather than blocker: `tabGroups` is whole-record local-wins while
|
||||
// `unifiedTabs` is per-tab. A partition group arriving after local has any group for that
|
||||
// worktree never travels, and its tabs are adopted into local's first group — visible, but in
|
||||
// the wrong group, order and split. Same shape as the unified-ledger defect, softer landing.
|
||||
for (const [worktreeId, groups] of Object.entries(partition.tabGroups ?? {})) {
|
||||
if (local.tabGroups?.[worktreeId]) {
|
||||
continue
|
||||
|
||||
@@ -148,6 +148,14 @@ test.describe('an upgrading profile keeps the tabs its old build left in the ssh
|
||||
).toEqual(tabsBefore.slice().sort())
|
||||
|
||||
// 2. The panes came back ATTACHED, not blank — the other half of the report.
|
||||
//
|
||||
// KNOWN WEAKNESS: readPaneIdentitySnapshot resolves the ACTIVE tab and returns only its
|
||||
// panes, so this covers one of the three tabs — two could restore blank and this still
|
||||
// passes. Assertion 3 is weaker still: a bare shell count is satisfied by N killed and N
|
||||
// respawned, so it cannot show the panes reattached to the SAME shells. A verified patch
|
||||
// exists that clicks each tab and compares ptyIds captured before quit; it did not apply
|
||||
// cleanly here (its [data-tab-id] locator does not resolve in this tree) and is left for a
|
||||
// follow-up rather than landed red.
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
|
||||
Reference in New Issue
Block a user