Skip to content

feat: reorder pinned sidebar sessions - #7509

Merged
bolichen97 merged 1 commit into
kirodotdev:mainfrom
Pearcekieser:feat/pinned-session-order
Sep 6, 2026
Merged

feat: reorder pinned sidebar sessions#7509
bolichen97 merged 1 commit into
kirodotdev:mainfrom
Pearcekieser:feat/pinned-session-order

Conversation

@Pearcekieser

@Pearcekieser Pearcekieser commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

Pinned sessions are promoted above other sessions, but their order is still recomputed from the selected automatic sort. Ordinary row-over-row dragging therefore snaps back; under Created: newest, creation time immediately wins again. The sidebar also has no visual boundary explaining where pin priority ends and automatic sorting begins.

Why it matters

Users pin a small working set because they want those sessions to stay in a deliberate, stable order. The existing whole-row drag affordance suggests that ordering is possible even though only folder, root, and chat-reference targets have an effect. A stable pinned section removes that mismatch while preserving useful automatic sorting for every other session.

What changed (motivation → approach → change)

  • Persist a browser-local order of pinned session keys and reconcile stale, duplicate, newly pinned, unpinned, and archived entries safely.
  • Capture the complete pre-mutation pinned baseline before optimistic pin changes, then persist membership only after server success so failed pin/unpin writes preserve rank exactly.
  • Apply manual rank only within the pinned section; unpinned sessions continue following the selected sort, including Created:newest, and explicit searches remain relevance-ranked.
  • Make pinned rows same-container drop targets while retaining existing cross-folder, root, board-column, and chat-reference semantics.
  • Render an accent insertion marker during drag and support Alt+ArrowUp / Alt+ArrowDown on focused pinned rows.
  • Add a quiet double-line divider only where a visible container contains both pinned and automatically sorted sessions, suppressing the ordinary row hairline at that boundary.
  • Keep droppable measurement on a separate wrapper from the draggable row, avoiding the dnd-kit registration loop and preserving the existing SessionRow memo boundary.

Tests

  • 27,208 website tests passed across 1,723 files with coverage enabled.
  • 134 focused ordering, drag-routing, flat-view, DnD-helper, mutation, and row-memo tests passed.
  • 42 migration-fix tests passed after the final GPT finding, including existing A/B pins + absent storage + pin C → A/B/C, successful unpin survivors, and optimistic rollback.
  • Latest current-main rebase at faef2c8a548c801b21dca97edd47c0f414bcf922: 77 focused tests passed; TypeScript, zero-warning source ESLint, the production/analyze build, exact bundle-size gate, feature-map gate, and narrow exact-SHA verifier passed locally.
  • The prior current-head CI round passed all substantive checks, including backend/frontend matrices, coverage, desktop builds, packaging, Offline E2E, security, policy, docs, and bundle gates. The prior GPT, Opus, Design, UX, and First Principles current-head reviews also passed; three GPT blockers were fixed: transactional concurrent pin membership, board-only shortcut suppression, and feature-map registration.

Manual verification

Scripted Playwright ran the real baseline and feature Vite applications at 1440×900 with identical session/API fixtures. It exercised actual dnd-kit mouse movement past the activation threshold, captured the insertion state, dropped a pinned row, verified the resulting DOM order, reloaded the page, and verified the persisted order remained unchanged.

The baseline and feature screenshots use the same six sessions and Created: newest. The baseline orders pins by creation date; the feature keeps the explicit pin order while the unpinned section remains newest-first.

Screenshots / video

Before — pinned sessions still follow Created:newest; no semantic boundary

Before: pinned sessions sorted automatically

After — manual pinned order and quiet double divider

After: manual pinned order above double divider

Real pointer drag, insertion feedback, drop, and persisted reload

Pinned session drag and persisted reorder

Recorded from faef2c8a548c801b21dca97edd47c0f414bcf922 · baseline/feature Vite origins · mode: Created:newest, fixed 350px sidebar · actual dnd-kit pointer events with deterministic API fixtures, no model or personal gateway data.

Insertion and reload frames

Insertion marker while dragging

Persisted order after reload

Related Issues

Fixes #7370

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

@Pearcekieser
Pearcekieser requested a review from a team September 1, 2026 04:40
@Pearcekieser
Pearcekieser requested a review from a team as a code owner September 1, 2026 04:40
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention fork Pull request from a fork (external contributor) readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 1, 2026
@Pearcekieser
Pearcekieser force-pushed the feat/pinned-session-order branch 2 times, most recently from e68f6e5 to ad84a70 Compare September 1, 2026 06:39
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 1, 2026
@Pearcekieser

Copy link
Copy Markdown
Contributor Author

CI status for current head ad84a7075fec560b38aa6c179122443165ff6da6:

  • All feature, frontend, lint, screenshot, build, E2E, security, and applicable backend checks passed.
  • Backend Tests (Windows) (2) failed only the existing wall-clock linearity assertion (0.0631s -> 0.2114s looks super-linear).
  • Backend Tests (3.12) (2) was cancelled, so the aggregate coverage/readiness checks could not complete.
  • No test introduced or touched by this PR failed, and there are no review findings or unresolved threads.

The fork credential cannot invoke gh run rerun (Must have admin rights to Repository). Could a maintainer please rerun the failed/cancelled CI jobs and trigger the current-head fork GPT/Opus reviews if they do not start automatically?

@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Sep 1, 2026
@Pearcekieser
Pearcekieser force-pushed the feat/pinned-session-order branch from ad84a70 to 5d58419 Compare September 1, 2026 16:50
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 1, 2026
@Pearcekieser
Pearcekieser force-pushed the feat/pinned-session-order branch from 5d58419 to 9f16935 Compare September 2, 2026 22:11
@Pearcekieser

Copy link
Copy Markdown
Contributor Author

Maintainer workflow approval requested for current head 9f1693509f3e923f2ab21ae89875aa294a9e434b.

The branch was rebased onto current main and locally passes 73 focused tests (including AppIconLocalFallback), TypeScript, the exact ESLint 597-warning ceiling, and the production build. Current pull-request workflows have remained queued since 22:13 UTC, while GPT 5.6 and Opus 4.8 were skipped, so fresh current-head reviewer stamps and the main CI round require maintainer approval. No merge or auto-merge is requested.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of faef2c8a548c801b21dca97edd47c0f414bcf922 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound feature, but persisting rank browser-locally while membership stays server-side spawns a reconciliation state machine disproportionate to a cosmetic preference.

Watch

  • The split persistence (server slot.pinned, localStorage mc-pinned-session-order) is the root cause of ~150 lines of transactional machinery in useSessionActions.tsPinMutationBatch, snapshotVersion, slotsGeneration, per-key slotPinGenerations, bounded refetch loops, plus a fresh GET /api/chat/slots after every pin settle — and of module-level mutable singletons (activePinMutationBatch, pinMutationTails) that ChatSidebar reads via pinMutationKeysInFlight(), a hidden channel outside Redux/react-query. The codebase already has a server-side precedent for manual sidebar ordering (folder order + reorderFolders); rank stored there would make the server the single writer, give cross-device consistency, and delete most of this. Every future contributor touching pin flows now maintains this ownership-generation protocol.
  • The failure path defends a purely cosmetic local rank against a failed pin PATCH with ownership reconstruction across three generation counters ("rolls back an owned failure when another key was superseded", etc.). Last-write-wins plus a query invalidation would lose, at worst, one rank slot on a rare failed write — far cheaper than this surface.

Suggestions

  • Track a follow-up to migrate pinned rank onto the server following the folder-order pattern; that migration also retires the batch/generation machinery rather than entrenching it.
  • The dead-key removal (show_collapsed_folders, 12 locale files, deadKeys baseline 29→28) is unrelated cleanup — move it to its own chore PR.

[DESIGN-REVIEWED] faef2c8

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of faef2c8a548c801b21dca97edd47c0f414bcf922 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All counts verified against the trusted base. temp-screenshots/ is an established convention (1000+ files, README), so the screenshots are fine. show_collapsed_folders exists only in the 13 locale JSONs — a dead key, so its removal is an unrelated rider. commitPinnedSessionMembership is called only from its own test file; production imports the plural commitPinnedSessionOperations/commitPinnedSessionSnapshot.

First-Principles-Verdict: CONCERNS

Exported commitPinnedSessionMembership has zero production consumers, and a dead-i18n-key cleanup rides along undeclared in a pinned-ordering feature.

What this change ships

Intent: let users keep their pinned sessions in a deliberate, stable order instead of the automatic sort — an ADDITION (feature request #7370).

  1. Dragging a pinned row onto a pinned peer reorders it, persisted per browser — justified
  2. Alt+ArrowUp/Down moves a focused pinned row — justified
  3. Accent insertion line shown while dragging over a pinned peer — justified
  4. Double-line divider where the pinned section meets automatic sorting — justified
  5. Manual pin order also applies in the hover session flyout — undeclared (feature map only, not the description)
  6. Failed or concurrent pin/unpin writes reconcile rank against a fresh server snapshot — justified
  7. New browser storage key mc-pinned-session-order, synced across tabs — justified
  8. Dead i18n key show_collapsed_folders deleted from 13 locales, dead-key baseline 29→28 — rides along, undeclared
  9. Exported helper commitPinnedSessionMembership — zero consumers
  10. Session-sort read moved into sessionOrder.ts as readSessionSortKey — justified (gains a second consumer, useSessionActions)

Watch

  • Item 8: nothing in the description mentions the locale deletions; grep shows show_collapsed_folders appears only in the 13 locale files at base (no source consumer), so it is a safe deletion — but it belongs in its own chore commit, not hidden in this feature.
  • Item 5: the flyout now reorders with the sidebar; a reader of the description would not expect that surface to change.

Subtractions

  • Delete commitPinnedSessionMembership (and its three test calls) from website/src/utils/pinnedSessionOrder.ts — 0 production consumers (grep: only pinnedSessionOrder.test.ts); production already uses commitPinnedSessionOperations.

[FIRST-PRINCIPLES-REVIEWED] faef2c8

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5, fork) — 🟡 CONCERNS

UX-level review of faef2c8a548c801b21dca97edd47c0f414bcf922 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: CONCERNS

Fork lane: no first-time reader has seen the divider, insertion marker, or reorder gesture — the added screenshots/gif exist only in the PR, not on disk.

Watch

  • The only surface of Alt+ArrowUp Alt+ArrowDown is aria-keyshortcuts (ChatSidebar.tsx), which only assistive tech reads — sighted keyboard users can never discover the shortcut, and nothing else announces that pinned rows are now reorderable at all (discovery relies on retrying a drag that used to snap back). Low frequency × friction × persistent. Smallest fix: add "Move up" / "Move down" items to the pinned row's context menu, which makes both the feature and the shortcut discoverable.

Evidence gaps

  • Pinned-row drag reorder + accent insertion marker (pinned-session-insertion) — after-insertion.png and pinned-session-drag.gif are binary markers in the patch; no blind reader has seen them.
  • The double-line PinnedSessionDivider boundary — shown only in after.png/after-reload.png, unseen by a blind reader; whether a first-timer reads it as "manual order ends here" is unknown.
  • The reorder relocation itself uses framer-motion layout/layoutId (continuous, and a recording is committed), but a human must watch pinned-session-drag.gif to confirm.
  • Close all of these by pushing the branch to this repository so the blind-read lane runs against the committed screenshots.

[UX-REVIEWED] faef2c8

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed faef2c8a548c801b21dca97edd47c0f414bcf922 via the fork AI-review pipeline; updated in place on each push.

Review details

I've read the full diff and verified the key facts against source. Let me confirm my analysis of Candidate 2's reachability.

The determining fact: SessionFlyout (line 8864) renders only when flyoutEligible && flyout.open, and flyoutEligible requires !sidebarOpen (line 8693). The reorderPinned drag droppables and Alt+Arrow keyboard handlers live on the expanded sidebar rows, which are only interactive when sidebarOpen is true. The two surfaces are therefore mutually exclusive: a same-tab reorder can only occur while the sidebar is open (flyout not rendered), and when the user later collapses and opens the flyout, SessionFlyout mounts fresh with useState(readPinnedSessionOrder), reading the freshly-persisted order from localStorage. The missing event produces no stale surface — Candidate 2 fails to establish (c), an observable wrong outcome. Cross-tab writes use the native storage event, which works.

Candidate 1 (committed temp-screenshots/ binaries) is a repo-hygiene concern with no execution path — it cannot ground (a)/(b)/(c) and falls into a category this review does not own.

No findings.

[OPUS-REVIEWED] faef2c8

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed partial sidebar state corrupting initial pinned orderspan=a3f2266a57bf

Head c78e9597a764dd3122204477f6c7e0a2c5d8facf trusts dashboard.sidebarOrder only when its duplicate-free pinned set exactly covers every currently pinned slot. Empty or partial projections fall back to the complete pin set sorted by the validated persisted comparator before the new pin appends. The regression uses name-desc Alpha/Zulu with only Alpha rendered and verifies Zulu/Alpha/new.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed successful pin completion overwriting concurrent manual reorderspan=9f720da7d6f8

Head b1b7aa765e8b5f10a9b7ea58ff79d99c3415f2bf applies stale newly-pinned-rank filtering only when current storage still equals the mutation's captured stored baseline. If storage changed, that newer manual rank is preserved and reconciled against authoritative membership. A regression captures stale [new,a,b], concurrently reorders to [a,new,b], then completes the pin snapshot and verifies the middle placement survives.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

Maintainer approval needed for current head b1b7aa765e8b5f10a9b7ea58ff79d99c3415f2bf: please approve and run https://github.com/kirodotdev/KiroCrew/actions/runs/33946374071. GitHub marked the fork pull_request workflows action_required; contributor approval is admin-only. This head preserves concurrent manual rank via captured-storage compare-and-apply during successful pin completion; 101 tests and all static/bundle gates pass on latest main. No merge or auto-merge is requested.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed live frames causing unbounded reconciliation refetchspan=aa43add523ec

Head d829c6a818eb30f13e1ad5e5faeab48282988698 bounds stable-snapshot reconciliation to three GET attempts. If every request overlaps a newer authoritative full-slot frame, it commits from the latest accepted Redux slots snapshot instead of issuing another request, while still honoring newer local mutation versions. A continuous-frame regression forces all three GETs stale, asserts exactly three calls, and verifies current authoritative membership/rank commits.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

Maintainer approval needed for current head d829c6a818eb30f13e1ad5e5faeab48282988698: please approve and run https://github.com/kirodotdev/KiroCrew/actions/runs/33947133619. GitHub marked the fork pull_request workflows action_required; contributor approval is admin-only. This head bounds reconciliation to three GETs before using the latest authoritative Redux snapshot; 102 tests and all static/bundle gates pass on latest main. No merge or auto-merge is requested.

bolichen97
bolichen97 previously approved these changes Sep 5, 2026

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full diff at d829c6a. The reconciliation is coherent after the review rounds: a single module-level batch joined by every in-flight toggle, monotonic snapshotVersion so only the newest settlement reconciles, refetch bounded at 3 then falling back to Redux, rank persisted only after the server outcome, and a rejected pin restores membership without touching survivor rank. pinnedSessionOrder.ts prunes on write and degrades malformed JSON to natural order, so the store cannot grow or corrupt. No hardcoded English; screenshots under temp-screenshots/.

Maintainer decision recorded: pin ORDER stays browser-local (localStorage, cross-tab synced) while pin MEMBERSHIP stays server-side -- accepted for this PR. Two follow-ups, non-blocking: hide PinnedSessionDivider in board view where reordering is impossible, and give Alt+ArrowUp/Down a visible hint (tooltip or shortcuts help) since aria-keyshortcuts alone makes it undiscoverable. Approving.

@bolichen97

Copy link
Copy Markdown
Collaborator

Approved, but the PR went CONFLICTING after this evening's merges (#8180 hover-hold and #8542 interrupted-session marker both landed in ChatSidebar.tsx). Auto-merge is armed and my approval stands; please rebase onto current main and force-push with --force-with-lease -- the approval will be dismissed by the push (require_last_push_approval), so ping me and I will re-approve right away. Please keep the diff byte-identical apart from the conflict resolution so the re-approve is a formality.

@bolichen97

Copy link
Copy Markdown
Collaborator

Resolved the conflict for you (maintainer_can_modify) by merging current main into the branch as a second commit 6d2016645 -- no history rewrite, so your original commit is untouched and the PR stays at 2 commits.

Conflicts were all against #8180's hover-hold lane scopes and #8542 in ChatSidebar.tsx: kept the pinned-section divider and startsAutomaticSection gating, adopted main's renderSessionRow(s, indent, showDivider, scope, navScope, holdContainer) signature at the four call sites (board folder / tree folder / tree root / board flat column), and unioned the feature-map Sessions row. Diff vs main is the same 22 files as before.

Locally green: tsc -b, and vitest for hoverHoldOrder, arrowNav, dragFreezeOrder, boardStateLanes, boardFolderReorder, ephemeralCreate, pinnedSessionOrder, useSessionActions.cov80 (98 tests). One thing worth a glance: heldLane can transiently re-seat a hovered pinned row, so startsAutomaticSection now runs over the held order -- during a hold the divider follows the visual list, which reads as intended, but shout if you disagree. Re-approving once CI settles.

bolichen97
bolichen97 previously approved these changes Sep 5, 2026

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving on 6d20166: the only delta from the head I approved is the conflict-resolution merge commit I pushed myself (main's renderSessionRow signature at four call sites + the feature-map row union); the diff against main is the same 22 files. Auto-merge is armed and will land once PR Readiness reports green on this head. Maintainer decision on browser-local pin order stands as recorded above.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed existing pins lacking a stable manual baselinespan=c3bc99e7b8e9

Head fd8143a53cd8951c444f25ff5cd6d7497a88295a seeds empty pinned-order storage once from the complete natural pin order after authoritative slots and tag-column projection have loaded. The effect is disabled in board projection, preserving the board no-ordering invariant; switching to list/flat may initialize it. A regression verifies newest-first pins persist as [k2,k1], while the board suppression regression remains green.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed unrelated slot updates suppressing pin rollbackspan=a3f2266a57bf

Head 8ca6d8a5e74cc01321f15a2095c05b9f0877ff2c replaces whole-slot identity ownership with dashboard.slotPinGenerations, incremented only by per-key updateSlotPin writes. Failed reconciliation now requires unchanged authoritative slotsGeneration, unchanged key-specific pin generation, and matching pin value. A regression applies an unrelated title update between optimistic pin and rejected PATCH+failed snapshot, proving membership rolls back while the title survives.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

Maintainer approval needed for current head 8ca6d8a5e74cc01321f15a2095c05b9f0877ff2c: please approve and run https://github.com/kirodotdev/KiroCrew/actions/runs/34004333997. GitHub marked the fork pull_request workflows action_required; contributor approval is admin-only. This head uses per-key pin generations so unrelated slot updates cannot suppress rejected-pin rollback; 104 frontend tests, 2,010 security tests, and all static/bundle gates pass on latest main. No merge or auto-merge is requested.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed silent tag-column load failurespan=c3bc99e7b8e9

Head 47289a54e59f136031258f7fbd757c67509879da captures the tag-column query error and renders it through the shared block ErrorNotice with Ask Agent (data-testid=tag-columns-load-error). Query failure now counts as a settled list fallback for board-safe baseline initialization rather than leaving it pending forever. A regression rejects the endpoint and verifies an alert containing the transport error.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed failed unpin corrupting rank during concurrent reorderspan=c3bc99e7b8e9

Head 8c123e28629db714912eabc954ecdb513a69de26 exposes the active pin-reconciliation batch keys and includes only those temporary keys when reconciling a manual reorder. Thus B remains seated while its optimistic unpin is unresolved; stale unrelated keys still drop. The exact [A,B,C], failed-unpin B, A→below C regression preserves [B,C,A] through authoritative rollback. The tag-column ErrorNotice is also scoped to enabled board mode, restoring unrelated alert tests.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed divider shown before collapsed dormant rowsspan=4b27d04f5c9a

Head 27dbb37b1b79f568706228a880e306c9b0707584 gates both folder and root stale-boundary dividers on the corresponding staleExpanded state. A regression renders a visible pinned row plus collapsed dormant unpinned row, verifies no divider while hidden, expands the section, then verifies the divider appears.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed baseline seeding during rejected optimistic pinspan=c3bc99e7b8e9

Head 76bcf843593410d4e72bf915f8c69f6da131a62d defers one-time baseline persistence whenever pinMutationKeysInFlight() is non-empty. Authoritative settlement then either commits order itself or changes membership and lets the effect retry safely. The failed-pin regression now asserts the key is present in the active batch during optimism and absent after rollback.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed concurrent reorder preventing failed pin rollbackspan=a3f2266a57bf

Head de62b76a994e884caa4e4e6cf36cb3e0554ec079 computes authoritative expected membership from the captured baseline plus successful owned writes, then explicitly commits final membership for every owned key—including rollback membership for failures—against the latest rank. The exact rejected optimistic pin + failed snapshot + concurrent survivor reorder regression removes the failed key while preserving [C,A].

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed empty roster permanently latching baseline initializationspan=4b27d04f5c9a

Head bf711bfd740b83f418635d22cd0d84a0603f651a leaves pinnedBaselineInitialized false while naturalPinnedOrder is empty. It latches only when existing storage already supplies authority or when a non-empty authoritative pin set is persisted. A same-instance regression starts with an empty loaded roster, receives pins later, and verifies newest-first baseline storage is seeded.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed projected sidebar order corrupting persisted pin rankspan=a3f2266a57bf

Head a47dcd25cb498ee0d7fef1d1ec9223da6548c6bd removes dashboard.sidebarOrder from mutation baseline authority. Every batch now seeds from the complete pinned set ranked by the validated persisted sort; existing stored manual rank then overlays through reconciliation. The first-pin regression supplies a conflicting complete projection and verifies the complete natural baseline wins.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Removed unrelated tag-column failure UX changespan=4b27d04f5c9a

Head a47dcd25cb498ee0d7fef1d1ec9223da6548c6bd removes the tag-column error capture, ErrorNotice banner, and its test scaffolding while retaining isFetched settlement for board-safe baseline timing. The unrelated ChatPageFollowup alert suite remains green.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed reconciliation bypassing React Query fetch lifecyclespan=aa43add523ec

Head bb932a40cb56708fe81e908383986788e29c5148 routes every bounded reconciliation attempt through queryClient.fetchQuery<ChatSlot[]> under a unique [chat-slots,pin-reconcile,requestId] key with zero stale/cache time. Existing retry-count and continuous-frame regressions remain green. Latest-main tag-column ErrorNotice/retry recovery is preserved alongside the baseline settled-state gate.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

Maintainer approval needed for current head bb932a40cb56708fe81e908383986788e29c5148: please approve and run https://github.com/kirodotdev/KiroCrew/actions/runs/34015363223. GitHub marked the fork pull_request workflows action_required; contributor approval is admin-only. This head routes bounded pin reconciliation through React Query and preserves latest-main tag-column recovery; 127 frontend tests, 2,018 security tests, and all static/bundle gates pass. No merge or auto-merge is requested.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed remote pin changes reviving stale rankspan=c3bc99e7b8e9

Head 4eca47c9f9ae617abaa12af77b9d7d00d2349a37 converts the list/flat baseline effect into ongoing authoritative membership reconciliation whenever no local pin batch is pending. Remote unpins prune storage immediately; later remote re-pins append because the stale key is gone. A same-instance regression verifies [a,b,c] → [a,c] → [a,c,b].

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed concurrent new pins using slot orderspan=aa43add523ec

Head 2bb9eb7b12bda10e53e8719018e808917e0d1f50 sorts authoritative keys absent from the captured baseline with the validated active SortKey, then commits baseline survivors followed by those sorted new pins. A name-desc regression concurrently pins Alpha and Zulu and verifies [base,Zulu,Alpha].

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed board mode skipping stale-rank cleanupspan=4b27d04f5c9a

Head 2bb9eb7b12bda10e53e8719018e808917e0d1f50 tracks whether browser-local rank authority has been established. Board mode still performs no initial write when storage is empty, but once authority exists it reconciles remote unpins and later re-pins. The board regression verifies [b1,b2] → [b1] → [b1,b2] while the no-ordering board test remains green.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author

Maintainer rerun requested for current head 2bb9eb7b12bda10e53e8719018e808917e0d1f50: please rerun failed jobs in https://github.com/kirodotdev/KiroCrew/actions/runs/34018450753. The sole root failure is test_snapshot.py::TestNotificationCopyWhenNoLiveFileExists::test_a_note_delivered_during_the_copy_survives_the_READER, which failed its timing assertion that the append overlapped the copy; Coverage Gate and PR Readiness are downstream. All current-head reviews are fresh/green with zero findings, and contributor rerun is admin-only.

@Pearcekieser

Copy link
Copy Markdown
Contributor Author
  • Fixed cross-tab reconciliation ping-pongspan=c3bc99e7b8e9

Head 45f26485dfeab99bfbc06faa4f2cb2cbfd0d279d marks changed state loaded from the browser storage event. Reconciliation still updates this tab’s local order, but storage-originated changes never persist or emit a same-tab event; local actions and later authoritative slot changes remain writers. A regression injects incompatible remote storage and verifies this tab does not write back. The confirmed unused show_collapsed_folders catalog entry was removed across locales so the dead-key ratchet remains at 29 without raising its baseline.

Persist a manual order for pinned sessions while keeping the selected automatic sort for all other sessions. Add explicit section and insertion affordances, keyboard movement, rollback-safe pin lifecycle handling, documentation, and regression coverage.
@Pearcekieser

Copy link
Copy Markdown
Contributor Author

Maintainer action requested: current head faef2c8a548c801b21dca97edd47c0f414bcf922 is mergeable and has zero findings/unresolved threads, but PR Readiness is blocked because GPT and Opus review stamps still name the previous head. Please rerun/approve the required current-head reviews. No merge or auto-merge requested.

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full diff at faef2c8a548c801b21dca97edd47c0f414bcf922, not carried over from my dismissed approvals -- that head was 22 files, this one is 37 and the reconciliation machinery grew substantially after it (slotPinGenerations, React Query routed reconciliation, ongoing authoritative membership reconciliation, board-mode rank authority, cross-tab ping-pong guard). Re-reviewed from scratch.

Correctness of persistence. pinnedSessionOrder.ts cannot corrupt anything. Every write goes through reconcilePinnedSessionOrder, which intersects against the authoritative pinned set and dedupes, so a stale or duplicate key cannot survive a write and the store cannot grow. readPinnedSessionOrder degrades malformed JSON, a non-array, and non-string members to natural order. movePinnedSession is inert on unknown or equal keys. Rank is persisted only after the server outcome, never on the optimistic write, so a rejected pin leaves previous rank byte-identical.

No corruption of other sessions or users. Order is a separate mc-pinned-session-order localStorage key holding nothing but pinned session keys; the server is never written with order, only the existing pin-membership PATCH. Cross-user is not a vector: localStorage is per-origin-per-profile and the store is pruned against server-confirmed membership on every write. The slotPinGenerations per-key counter correctly isolates ownership so an unrelated slot update no longer suppresses a rejected pin's rollback.

Reconciliation. The three-generation model closes the races the review rounds surfaced: snapshotVersion monotonic per batch so only the newest settlement reconciles, slotsGeneration so an in-flight authoritative frame invalidates a snapshot, per-key slotPinGenerations plus matching pin value for failure-path ownership. The refetch loop is bounded at 3 then commits from the latest accepted Redux slots, so continuous live frames cannot spin. setSlotPinInOrder serializes per-key tails, so rapid toggles reach the server in invocation order.

Scope. 1,484 additions reads large for a reorder feature, but the production delta is ~560 lines across 6 files; the rest is tests. The dnd.tsx disabled pass-through is optional and backward compatible. Lowering the dead-key BASELINE 29 -> 28 by deleting the confirmed-unused show_collapsed_folders across all 13 locales is a strict improvement, not ratchet gaming. temp-screenshots/ is a sanctioned committed deliverable per .gitignore. The feature is documented in the module spec, so the spec-management rule is satisfied. No hardcoded user-facing English.

Non-blocking follow-ups, none worth another round:

  1. activePinMutationBatch clears only once every entry settles, so a request that never settles leaves pinMutationKeysInFlight() non-empty for the page lifetime, which permanently defers baseline seeding and membership reconciliation. Soft failure (stale order, reload recovers), but a settle deadline would remove the latch.
  2. If commitPinnedSessionSnapshot itself throws after the try block has already nulled activePinMutationBatch, the catch path runs a second commit with fallback semantics. Requires safeSetItem to throw, so unreachable in practice.
  3. slotPinGenerations is never pruned when reconcileSlots drops a slot. Memory-only and bounded by keys touched in one page load, so trivial, but it is a Record that only grows.

Maintainer decision, restated and unchanged: pin ORDER stays browser-local while pin MEMBERSHIP stays server-side. This is the source of the entire reconciliation surface and means order does not follow a user across browsers, profiles, or the desktop app versus the localhost tab. I am accepting it for this PR because the blast radius is confined to sidebar ordering and the worst case is a wrong order that a reload fixes -- no server state and no other user's data can be reached. If pinned order is ever expected to sync, the right change is moving rank server-side next to membership, which deletes most of this machinery rather than adding to it.

All 66 checks green on this head, PR Readiness green at 10:04, all five AI review lanes fresh and clear, zero unresolved threads. Approving as a kirocrew-ux-reviewers member, which also satisfies the UX ruleset for the website/src/** non-test files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make pinned sessions manually reorderable in the sidebar

2 participants