mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-09 00:49:41 +02:00
fix(tabs): review-driven hardening for the tab-layout foundation and vertical rail
Post-merge follow-ups from the deep review of #334 and #335, so they ship in the same release as the features. Tab-layout foundation (#335): - PUT /api/session-order drops unknown/foreign ids again instead of 400ing the whole write, in both the owner and the admin path (single-user requests are the synthetic admin, so that path is the one the browser hits). The frontend debounces its reorder push and swallows errors, so a session deleted inside the debounce window silently cost the user the entire reorder - and the endpoint sits on the stable /api/v1 surface, where the pre-layout server merged leniently. - A failed mux restore no longer locks explicit deletions into 500s for the process lifetime: runSessionDeletion and webviewDeleted degrade to best-effort without layout coordination, while the automated stale sweep (runStaleSessionCleanup) stays fail-closed. - sse-events doc comment: no 'suppressed' hook event exists; hooks stay 8. - registerSessionWithLayout resolves its owner through ownerLayoutKey() instead of a hardcoded '@single'. Vertical rail (#334) - all rail-awareness gaps in sidebar-only predicates, unified behind the new _isVerticalTabList() (sidebar OR rail): - Drag-reorder read the insertion side from clientX in the rail, so before/after was effectively arbitrary on vertical rows; the drag-over indicators now draw as top/bottom edges there like the sidebar's. - The active tab is scrolled into view in the rail (Alt+N/palette selection used to leave the row below the fold). - Floating subagent/ultracode windows anchor to the RIGHT of rail tabs, and the connector redraw gates (render tail + strip scroll) cover the rail. - Server-seeded tabOrientation is applied when the async settings load resolves, not only at boot, so a fresh device shows the rail immediately. - The pre-paint script stamps data-tab-orientation and --tab-rail-width (sidebar-wins and solo carve-outs included), removing the flash of the header strip on every vertical-mode load. - The session name font defaults to 12px, the sidebar's historical 0.75rem size, so installs that never touch the new slider are not restyled. Also documents the rail in CLAUDE.md (second #sessionTabs host, mover ordering, the axis-predicate rule) and gives tab-rail-resize.js its @dependency/@loadorder header. Full gate green (6093 tests); the excluded browser suite was run by hand - only the known environmental failures (opencode/codex binaries) remain, identical to pristine master. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -376,10 +376,14 @@ export class TabLayoutService {
|
||||
.filter((item) => item.kind === 'session' && item.ownerValid && item.visible)
|
||||
.map((item) => item.id)
|
||||
);
|
||||
const invalid = normalized.find((id) => !visible.has(id));
|
||||
if (invalid) throw new TabLayoutValidationError(`session is not owned by layout owner: ${invalid}`);
|
||||
// Unknown or foreign ids are DROPPED, never a 400: the browser debounces
|
||||
// its reorder push (and swallows errors), so a session deleted inside
|
||||
// that window would otherwise cost the user the whole reorder — and the
|
||||
// endpoint sits on the stable /api/v1 surface, where the pre-layout
|
||||
// server merged leniently. Same philosophy as resolveParentSessionId.
|
||||
const requestedVisible = normalized.filter((id) => visible.has(id));
|
||||
const currentKnown = flattenOwnerSessionOrder(prepared.authoritative).filter((id) => visible.has(id));
|
||||
const effective = mergeSessionOrder(normalized, currentKnown);
|
||||
const effective = mergeSessionOrder(requestedVisible, currentKnown);
|
||||
const ranked = applyLegacySessionRank(prepared.authoritative, effective, prepared.metadata);
|
||||
const needsLayout = prepared.needsReconciliationCommit || !sameLayout(prepared.authoritative, ranked);
|
||||
const base = prepared.current ?? { ...prepared.authoritative, version: -1 };
|
||||
@@ -407,8 +411,10 @@ export class TabLayoutService {
|
||||
const { persisted, live } = this.sessionRecords();
|
||||
for (const record of persisted) knownOwners.set(record.id, ownerOf(record));
|
||||
for (const record of live) knownOwners.set(record.id, ownerOf(record));
|
||||
const invalid = normalized.find((id) => !knownOwners.has(id));
|
||||
if (invalid) throw new TabLayoutValidationError(`session is not visible machine-wide: ${invalid}`);
|
||||
// Unknown ids are DROPPED, never a 400 — see putOwnerLegacyOrder. In
|
||||
// single-user mode every request is the synthetic admin, so this path
|
||||
// IS the one the browser's debounced (error-swallowing) push hits.
|
||||
const known = normalized.filter((id) => knownOwners.has(id));
|
||||
|
||||
const publications: OwnerProjectionPublication[] = [];
|
||||
const updates: Record<string, TabLayout> = Object.create(null) as Record<string, TabLayout>;
|
||||
@@ -419,7 +425,7 @@ export class TabLayoutService {
|
||||
.filter((item) => item.kind === 'session' && item.ownerValid && item.visible)
|
||||
.map((item) => item.id)
|
||||
);
|
||||
const requestedOwner = normalized.filter((id) => visible.has(id));
|
||||
const requestedOwner = known.filter((id) => visible.has(id));
|
||||
const currentKnown = flattenOwnerSessionOrder(prepared.authoritative).filter((id) => visible.has(id));
|
||||
const effective = mergeSessionOrder(requestedOwner, currentKnown);
|
||||
const ranked = applyLegacySessionRank(prepared.authoritative, effective, prepared.metadata);
|
||||
@@ -429,7 +435,7 @@ export class TabLayoutService {
|
||||
if (needsLayout) updates[owner] = next;
|
||||
publications.push({ owner, previous: prepared.current, next, metadata: prepared.metadata });
|
||||
}
|
||||
const change = this.publish(updates, publications, normalized);
|
||||
const change = this.publish(updates, publications, known);
|
||||
return { order: [...change.globalOrder], ...change };
|
||||
});
|
||||
if (result) return result;
|
||||
@@ -489,6 +495,12 @@ export class TabLayoutService {
|
||||
* commits only after the resource cleanup finishes.
|
||||
*/
|
||||
async runSessionDeletion<T>(removed: readonly RemovedTabLayoutSession[], action: () => Promise<T>): Promise<T> {
|
||||
// A failed restoration must not lock the user out of explicitly closing a
|
||||
// tab for the rest of the process lifetime: degrade to best-effort deletion
|
||||
// without layout coordination. Only the AUTOMATED stale sweep stays
|
||||
// fail-closed on 'failed' (runStaleSessionCleanup), because that one picks
|
||||
// its victims itself from state a failed restore may have left incomplete.
|
||||
if (this.restorationState === 'failed') return action();
|
||||
this.assertDeletionReady();
|
||||
if (this.restorationState === 'skipped' || removed.length === 0) return action();
|
||||
const owners = new Set(removed.map(ownerOf));
|
||||
@@ -643,6 +655,9 @@ export class TabLayoutService {
|
||||
}
|
||||
|
||||
async webviewDeleted(owner: string, id: string): Promise<void> {
|
||||
// Same explicit-user-action escape hatch as runSessionDeletion: a failed
|
||||
// restore skips layout coordination instead of failing the delete.
|
||||
if (this.restorationState === 'failed') return;
|
||||
this.assertDeletionReady();
|
||||
if (this.restorationState === 'skipped') return;
|
||||
await this.withOwner(owner, async () => {
|
||||
|
||||
Reference in New Issue
Block a user