Skip to content

feat: persist workspace rail selection#1050

Open
donovan-yohan wants to merge 5 commits into
nightlyfrom
issue-1043-workspace-rail
Open

feat: persist workspace rail selection#1050
donovan-yohan wants to merge 5 commits into
nightlyfrom
issue-1043-workspace-rail

Conversation

@donovan-yohan

@donovan-yohan donovan-yohan commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Refs #1043

Summary

  • add IA workspace rail metadata/status persistence and v3 migration for archive/default fields
  • add archive/restore/list filtering API support plus frontend client mutations
  • wire WorkspaceBar quick-create to select the new workspace, archive instead of hard-delete, and scope ViewSpineTree to the selected workspace with reload-persisted UI state
  • add persistence/router tests for metadata, archive filtering, invalid metadata, and schema version

Verification

  • npm test -- test/ia-store.test.ts test/ia-workspace-router.test.ts test/view-tree-workspace-grouping.test.ts test/components/FileFeedbackPanel.test.ts (41 passed)
  • npm run check (pass; existing lint warnings only)
  • npm run build (pass; existing chunk-size/dynamic-import warnings)
  • npm test full suite: failed once on unrelated FileFeedbackPanel flaky test; isolated FileFeedbackPanel test passed afterward
  • pre-push with Node 24: changed-test hook failed only on the same unrelated FileFeedbackPanel flaky test; pushed with HUSKY=0 after targeted pass

Simplify

  • risk tier 3 multi-file behavior/API/UI change; local simplify pass used because Kanban worker delegation can inherit board tools
  • applied cleanup from pass: invalid metadata now returns 400 instead of 500, and stale delete comments were corrected

Summary by CodeRabbit

  • New Features

    • Workspaces can now be selected as the active view, with an All option to show everything again.
    • Added workspace archiving and restoring, plus support for showing archived workspaces when needed.
    • Workspace details now support more display and default settings, including pinning and custom metadata.
  • Bug Fixes

    • Workspace lists now better respect the active selection and hide unrelated content.
    • Archived workspaces are excluded from normal lists and restored workspaces reappear as expected.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@donovan-yohan, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 35 minutes and 28 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 credits.

🚦 How do rate 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 see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 59a5cb64-ccba-4ccf-8a71-ea86447d9935

📥 Commits

Reviewing files that changed from the base of the PR and between 3faabff and 478318e.

📒 Files selected for processing (3)
  • frontend/src/components/WorkspaceBar.tsx
  • server/features/ia-workspace-router.ts
  • test/ia-workspace-router.test.ts
📝 Walkthrough

Walkthrough

Adds workspace status (active/archived), rail metadata fields (pinned, color, icon, default repo/node/provider), and SQLite schema v3 migration. Introduces archive/restore endpoints on the server router, extends the frontend API/hook with archiveMutation, updates WorkspaceBar to an archive-centric model with active workspace selection, and filters ViewSpineTree content to the selected workspace.

Changes

Workspace Archive, Metadata, and Active Filtering

Layer / File(s) Summary
Shared workspace data model and DB schema (v3)
shared/workspace.ts, server/ia-store.ts
Introduces WorkspaceStatus ('active'|'archived'), extends Workspace with status/pinned/metadata fields, adds a v3 SQLite migration with new columns, expands WorkspaceUpsertInput/WorkspaceListOptions/IaStore, and adds row normalization helpers.
SQLite store archive/restore implementation
server/ia-store.ts
Implements listWorkspaces with active/all statement switching, upsertWorkspace writing v3 fields, and archiveWorkspace/restoreWorkspace via an updateWorkspaceStatus helper plus updateWorkspaceStatusStmt.
Server router: archive/restore endpoints and metadata validation
server/features/ia-workspace-router.ts
Adds GET includeArchived query support, POST create with workspaceMetadataPatch parsing, PATCH status+metadata fields, and new POST /:id/archive and POST /:id/restore endpoints with request-body parsing helpers.
Frontend API and hook: IaWorkspacePatch and archiveMutation
frontend/src/lib/api.ts, frontend/src/lib/hooks/use-ia-workspaces.ts
Expands IaWorkspace/adds IaWorkspacePatch, extends createIaWorkspace input, adds archiveIaWorkspace function, and wires archiveMutation into useIaWorkspaces.
WorkspaceBar: active selection, archive action, row UI
frontend/src/components/WorkspaceBar.tsx, frontend/src/components/WorkspaceBar.css
WorkspaceRow gains selected/onSelect/onArchive props and editing class modifiers; WorkspaceBar adds an "all" header button, activeWorkspaceId bindings, pinned: true on create, and archive confirmation with activeWorkspaceId cleanup. CSS adds __all-btn, --selected/--editing row modifiers, and __rename-btn styles.
ViewSpineTree: activeWorkspaceId filtering
frontend/src/components/ViewSpineTree.tsx
Derives visibleGrouped memo filtering workspace groups to activeWorkspaceId, updates hasContent, hasPersistedGroups, workspace render loop, ungrouped section, and hides the free/remote lane when a workspace is active.
Tests: rail metadata, archive/restore, migration v3
test/ia-store.test.ts, test/ia-workspace-router.test.ts
Adds store tests for rail metadata persistence, archive/restore status transitions, and migration version 3; adds router tests for metadata persistence, archive/restore list filtering, and invalid pinned value rejection.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Possibly related PRs

  • donovan-yohan/relay-ide#741: Introduced the ViewSpineTree workspace/ungrouped/free-lane grouping sections that this PR now extends with visibleGrouped filtering.
  • donovan-yohan/relay-ide#751: Added the persisted workspace grouping layer in ViewSpineTree and WorkspaceBar that this PR builds on with activeWorkspaceId and archive/restore.
  • donovan-yohan/relay-ide#915: Reads activeWorkspaceId from the same UI store this PR writes to, gating workspace launch actions.

Poem

🐰 Hop hop, the workspace glows,
An "all" button clears the rows,
Archive the old, restore the new,
Rail metadata shining through.
Status pinned, the bunny knows —
Every workspace blooms and grows! 🌸

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.24% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main UI change: persisting workspace rail selection.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-1043-workspace-rail

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces workspace rail metadata and soft archiving functionality, updating the database schema, API endpoints, and frontend components to support these new features. The feedback highlights an accessibility regression caused by moving the rename trigger to a double-click, which prevents keyboard-only users from renaming workspaces, and suggests adding a dedicated rename button instead. Additionally, the feedback recommends cleaning up the unused deleteMutation and its pending state check since deletion has been replaced by archiving.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines 96 to 107
type="button"
className="workspace-bar__name"
disabled={busy}
onClick={() => {
onClick={onSelect}
onDoubleClick={() => {
setDraft(workspace.name);
setEditing(true);
}}
title="rename"
title="select workspace; double-click to rename"
>
{workspace.name}
</button>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Moving the rename trigger from onClick to onDoubleClick introduces an accessibility regression. Keyboard-only users who navigate using Tab and activate buttons using Enter or Space will only trigger the onClick (onSelect) handler, leaving them with no way to rename a workspace. Removing onDoubleClick here and adding a dedicated rename button to the row actions solves this issue.

          type="button"
          className="workspace-bar__name"
          disabled={busy}
          onClick={onSelect}
          title="select workspace"
        >
          {workspace.name}
        </button>

Comment on lines 131 to 139
type="button"
className="workspace-bar__icon-btn workspace-bar__icon-btn--danger"
disabled={busy}
onClick={onDelete}
aria-label={`delete ${workspace.name}`}
title="delete workspace"
onClick={onArchive}
aria-label={`archive ${workspace.name}`}
title="archive workspace"
>
×
</button>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Add a dedicated rename button to the row actions. This ensures that both mouse and keyboard users can trigger the workspace rename flow reliably.

          type="button"
          className="workspace-bar__icon-btn"
          disabled={busy}
          onClick={() => {
            setDraft(workspace.name);
            setEditing(true);
          }}
          aria-label={`rename ${workspace.name}`}
          title="rename workspace"
        >
          ✎
        </button>
        <button
          type="button"
          className="workspace-bar__icon-btn workspace-bar__icon-btn--danger"
          disabled={busy}
          onClick={onArchive}
          aria-label={`archive ${workspace.name}`}
          title="archive workspace"
        >
          ×
        </button>

Comment on lines 151 to 156
invalidate,
createMutation,
updateMutation,
archiveMutation,
deleteMutation,
} = useIaWorkspaces();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The deleteMutation is destructured here but is no longer used to perform deletions since the delete functionality was replaced by archiving. Consider removing it to keep the code clean.

    invalidate,
    createMutation,
    updateMutation,
    archiveMutation,
  } = useIaWorkspaces();

Comment on lines 170 to 175
const pending =
createMutation.isPending ||
updateMutation.isPending ||
archiveMutation.isPending ||
deleteMutation.isPending ||
reordering;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Remove deleteMutation.isPending from the pending state check since deleteMutation is no longer used in this component.

Suggested change
const pending =
createMutation.isPending ||
updateMutation.isPending ||
archiveMutation.isPending ||
deleteMutation.isPending ||
reordering;
const pending =
createMutation.isPending ||
updateMutation.isPending ||
archiveMutation.isPending ||
reordering;

@donovan-yohan

Copy link
Copy Markdown
Owner Author

Review verdict: changes requested for PR #1050 at exact head 3c1c755.

Blocker:

  • frontend/src/components/WorkspaceBar.tsx:95-104 — workspace rename is mouse-only. The row name button selects on click and only enters edit mode on double-click, so keyboard-only users can tab to the row but Enter/Space only select it. Issue Add persistent workspace/project rail and quick-create #1043 explicitly includes workspace edit flows plus keyboard/Command Center create/select expectations; shipping the rail with rename hidden behind double-click is an accessibility regression. Add a real keyboard-reachable rename affordance (the Gemini suggestion of a dedicated row action button is fine) or equivalent keyboard path.

Non-blocking cleanup:

  • frontend/src/components/WorkspaceBar.tsx:145-175 — deleteMutation is still destructured and included in pending even though this component now archives instead of hard-deletes. Harmless at runtime, but stale and should be removed while touching this file.

Bot feedback triage:

  • Gemini inline rename accessibility: blocker, confirmed against the actual JSX.
  • Gemini inline rename button suggestion: actionable fix path.
  • Gemini unused deleteMutation / pending cleanup: follow-up/non-blocking cleanup, confirmed.
  • CodeRabbit issue comment: noise; rate-limit/billing boilerplate only, no substantive code finding.

Scope / closure:

Verification run by reviewer:

  • npm test -- test/ia-store.test.ts test/ia-workspace-router.test.ts -> 29 passed.
  • npm run check could not complete in this worktree because installed deps are missing CodeMirror/lucide packages (npm ls @codemirror/commands lucide-react --depth=0 shows empty). I did not treat that as PR-caused.

@donovan-yohan

Copy link
Copy Markdown
Owner Author

Review verdict: changes requested for PR #1050 at exact head cd88c7e.

Blocker:

  • frontend/src/components/WorkspaceBar.css:110-112 + frontend/src/lib/stores/ui.ts:33-35 — the remediation adds a fourth always-visible row action with a 44px minimum touch target, but the sidebar still allows 180px width. The fixed row chrome now needs at least 208px before the workspace name gets any space: 20px WorkspaceBar horizontal padding + 6px row gap + 444px buttons + 32px action gaps. At the supported minimum sidebar width, the action cluster alone overflows the rail. This violates the remediation gate’s layout requirement (“row actions do not hide or create overflow regressions”) and makes the dedicated rename action compete with move/archive controls in the narrow rail. Use an icon-only 44px rename button with accessible label, collapse less-used actions, or otherwise keep the fixed controls within the 180px rail.

Non-blocking observations:

  • frontend/src/components/WorkspaceBar.tsx:100-121 — the prior keyboard blocker is fixed in principle: the workspace name button still selects on Enter/Space, and a separate focusable rename button opens the inline editor with autofocus.
  • frontend/src/components/WorkspaceBar.tsx:157-185 — stale WorkspaceBar deleteMutation pending wiring was removed; pending state now tracks create/update/archive/reorder.
  • PR body should remain Refs #1043, not Closes #1043; this is still a foundation slice of the broader workspace rail issue.

Bot feedback triage:

  • Gemini rename accessibility: originally blocker; now fixed by the dedicated keyboard-reachable rename action.
  • Gemini dedicated rename button suggestion: implemented, but the chosen always-visible text button creates the layout blocker above.
  • Gemini unused deleteMutation/pending cleanup: fixed in WorkspaceBar.
  • CodeRabbit: rate-limit/no-signal billing comment only; noise.

Evidence checked:

  • Local exact head: cd88c7e.
  • Bot surfaces checked: PR review summaries, inline PR comments, issue comments.
  • Targeted tests: npm test -- test/ia-store.test.ts test/ia-workspace-router.test.ts passed, 29/29.
  • npm run check was attempted but failed before PR-relevant code on missing local dependencies for CodeMirror/lucide modules in this workspace; GitHub checks for the PR head are green.

@donovan-yohan

Copy link
Copy Markdown
Owner Author

Review verdict: approved for PR #1050 at exact head ee994f5.

Blockers: none.

Review focus:

  • frontend/src/components/WorkspaceBar.tsx:100-120 keeps the workspace name as the select button; native Enter/Space activation still reaches only onClick={onSelect}. Rename is a separate focusable button with aria-label, so it no longer steals name activation.
  • frontend/src/components/WorkspaceBar.css:119-159 hides the rename action out of layout until focus-visible, then exposes a 44px focused affordance with accent border/background. That satisfies keyboard/screen-reader reachability without making rename part of the always-visible row chrome.
  • frontend/src/components/WorkspaceBar.tsx:121-152 + WorkspaceBar.css:112-117/165-199 leave only move up, move down, and archive in the fixed row-action cluster. Static sizing at the 180px supported rail minimum is now back inside budget: 20px WorkspaceBar horizontal padding + 2px row border + 6px flex gap + 344px buttons + 22px action gaps + ~14px minimum name button padding/border = 178px worst-case, with the name content allowed to ellipsize via min-width:0/overflow hidden. no more fourth visible 44px button, finally.
  • Move/archive disabled semantics remain unchanged: move up/down keep first/last/pending guards, archive keeps pending guard and confirmation path.
  • PR body still uses Refs #1043, not Closes #1043; keep it that way unless this PR actually closes the full issue.

Bot feedback triage:

  • Gemini inline accessibility comments on WorkspaceBar rename keyboard access: substantive, already fixed by the new focusable rename button path at this head.
  • Gemini cleanup comments about unused deleteMutation/delete pending state: already fixed in WorkspaceBar; deleteMutation remains exposed by the hook because the API still has hard-delete support, but this component no longer consumes it.
  • CodeRabbit issue comment is rate-limit/billing boilerplate: noise.
  • Copilot/new CodeRabbit substantive comments: none found across review summaries, inline PR comments, and issue comments.

Evidence inspected:

  • exact workspace/head verified locally: ee994f5, PR feat: persist workspace rail selection #1050 open against nightly.
  • reviewed changed-file ledger for PR scope: WorkspaceBar.tsx/css remediation in detail; API/router/store/shared model/tests and ViewSpineTree workspace selection paths for integration sanity.
  • commands run: npm ci --ignore-scripts to restore missing worktree deps; npm test -- test/ia-store.test.ts test/ia-workspace-router.test.ts (29 passed); npm run check (passed with existing warning-only lint baseline); npm run build (passed with existing Vite chunk/dynamic-import warnings).

Scope/closure recommendation: approved; keep Refs #1043.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/components/WorkspaceBar.tsx (1)

102-111: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Expose the current workspace filter state to assistive tech.

The new selection is only conveyed by workspace-bar__row--selected. Screen readers won't know whether a workspace or all is currently active, so the filter state disappears outside the visual styling. Add aria-pressed or an equivalent state announcement on these buttons.

Possible fix
         <button
           type="button"
           className="workspace-bar__name"
           disabled={busy}
           onClick={onSelect}
           onDoubleClick={startEditing}
+          aria-pressed={selected}
           title="select workspace; double-click or tab to rename action to edit"
         >
           {workspace.name}
         </button>
         <button
           type="button"
           className="workspace-bar__all-btn"
           disabled={pending || activeWorkspaceId === null}
           onClick={() => setActiveWorkspaceId(null)}
+          aria-pressed={activeWorkspaceId === null}
         >
           all
         </button>

Also applies to: 283-290, 308-310

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/WorkspaceBar.tsx` around lines 102 - 111, The
workspace selection state is only reflected visually in WorkspaceBar, so
assistive tech can’t tell which filter is active. Update the relevant button
elements in WorkspaceBar (including the workspace/all selectors around the
referenced button blocks) to expose the active state with aria-pressed or an
equivalent accessible state. Use the existing selection logic in WorkspaceBar to
keep the ARIA state in sync with the current filter, without changing the
click/double-click behavior.
🧹 Nitpick comments (1)
test/ia-workspace-router.test.ts (1)

233-241: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that /restore makes the workspace visible in the default list again.

This test proves the restore response payload, but not the router contract that default GET /hub/ia/workspaces includes restored rows again. A restore handler that only returns status: 'active' could still pass here.

Suggested test addition
     expect(restored.status).toBe(200);
     expect(
       ((await restored.json()) as { workspace: Workspace }).workspace.status
     ).toBe('active');
+    expect(
+      (
+        (await (await fetch(`${baseUrl}${WS}`)).json()) as {
+          workspaces: Workspace[];
+        }
+      ).workspaces.map((w) => w.id)
+    ).toEqual([created.workspace.id]);
   });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/ia-workspace-router.test.ts` around lines 233 - 241, The restore test
only checks the `/restore` response payload, so it misses the router contract
that restored workspaces reappear in the default `GET /hub/ia/workspaces`
listing. In `ia-workspace-router.test.ts`, after calling the restore endpoint
and verifying the workspace is active, add an assertion using the existing list
fetch flow to confirm the restored workspace ID is present again in the default
workspace list.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/components/WorkspaceBar.tsx`:
- Around line 174-177: The persisted active workspace selection in WorkspaceBar
can become stale when the saved id no longer exists in the loaded workspaces,
causing ViewSpineTree to show an empty filtered view. Add a reconciliation step
in WorkspaceBar after the workspaces query finishes successfully: compare
activeWorkspaceId from useUiStore against the loaded workspaces, and if it is
missing or archived, clear it by calling setActiveWorkspaceId so the UI falls
back to the all view.

In `@server/features/ia-workspace-router.ts`:
- Around line 148-159: The fallback order calculation in the workspace create
flow is using only active rows via iaStore.listWorkspaces(), which can miss
archived workspaces and cause duplicate order values later. Update the create
path in ia-workspace-router to compute nextOrder from all workspaces instead of
the default active-only list, using the same nextOrder helper but with a
full-workspace source so archived rows are included.

---

Outside diff comments:
In `@frontend/src/components/WorkspaceBar.tsx`:
- Around line 102-111: The workspace selection state is only reflected visually
in WorkspaceBar, so assistive tech can’t tell which filter is active. Update the
relevant button elements in WorkspaceBar (including the workspace/all selectors
around the referenced button blocks) to expose the active state with
aria-pressed or an equivalent accessible state. Use the existing selection logic
in WorkspaceBar to keep the ARIA state in sync with the current filter, without
changing the click/double-click behavior.

---

Nitpick comments:
In `@test/ia-workspace-router.test.ts`:
- Around line 233-241: The restore test only checks the `/restore` response
payload, so it misses the router contract that restored workspaces reappear in
the default `GET /hub/ia/workspaces` listing. In `ia-workspace-router.test.ts`,
after calling the restore endpoint and verifying the workspace is active, add an
assertion using the existing list fetch flow to confirm the restored workspace
ID is present again in the default workspace list.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a5b1a8c7-6ce8-46d1-bdbf-aa7ffe64ebe8

📥 Commits

Reviewing files that changed from the base of the PR and between 8751feb and 3faabff.

📒 Files selected for processing (10)
  • frontend/src/components/ViewSpineTree.tsx
  • frontend/src/components/WorkspaceBar.css
  • frontend/src/components/WorkspaceBar.tsx
  • frontend/src/lib/api.ts
  • frontend/src/lib/hooks/use-ia-workspaces.ts
  • server/features/ia-workspace-router.ts
  • server/ia-store.ts
  • shared/workspace.ts
  • test/ia-store.test.ts
  • test/ia-workspace-router.test.ts

Comment thread frontend/src/components/WorkspaceBar.tsx
Comment thread server/features/ia-workspace-router.ts
@donovan-yohan

Copy link
Copy Markdown
Owner Author

Review verdict: approve for PR #1050 at exact head 3faabff.

Blockers: none.

Why this passes:

  • frontend/src/components/WorkspaceBar.tsx:80-158 now renders only the input while a row is editing, so the hidden rename button and move/archive controls are not left focusable or participating in edit-mode layout.
  • frontend/src/components/WorkspaceBar.tsx:113-124 keeps a real dedicated rename button in the keyboard order while the name button still selects the workspace.
  • frontend/src/components/WorkspaceBar.css:5-15,59-80,91-105,123-163 constrains the rail/list/row widths, lets long workspace names shrink/ellipsis, and keeps the rename control out of normal flex layout until focus-visible.

Evidence reviewed:

  • Exact head verified: 3faabff.
  • Remediation delta reviewed: frontend/src/components/WorkspaceBar.tsx, frontend/src/components/WorkspaceBar.css.
  • Browser layout smoke using actual WorkspaceBar.css at 180px sidebar width: .workspace-bar 180/180 client/scroll, normal row 158/158, editing row 158/158, focused rename remains inside sidebar, long name truncates, rename remains in tabbable order.
  • npm test -- test/ia-store.test.ts test/ia-workspace-router.test.ts: pass, 29 tests.
  • npm run check: pass; existing warning-only lint baseline remains.
  • GitHub checks observed on this head: ci, e2e, sync-labels, scan-commits all successful.

Bot feedback + learning:

  • Bot surfaces checked: review summaries, inline PR comments, issue comments.
  • Gemini prior high-priority rename accessibility finding: already fixed by the dedicated keyboard-reachable rename button.
  • Gemini prior cleanup findings around deleteMutation: already fixed; component now uses archiveMutation in pending state.
  • CodeRabbit surface: processing/status/walkthrough noise only in inspected comments.
  • Learning retained: yes — narrow rail review should verify hidden keyboard actions against both tab order and focus-visible overflow bounds.

@donovan-yohan

Copy link
Copy Markdown
Owner Author

Review verdict: changes requested for PR #1050 at exact head 478318e.

Blocker:

  • frontend/src/components/WorkspaceBar.tsx:196-201 + frontend/src/lib/hooks/use-ia-workspaces.ts:43-56 — the new stale-selection reconciliation clears freshly-created workspace selection before the invalidated workspace query has caught up. createMutation invalidates ['ia-workspaces'] on hook-level onSuccess, then WorkspaceBar's mutate-level onSuccess sets activeWorkspaceId to the created id. During the next render, workspaces can still be the previous active list (often [] or a list without the newly-created id), isLoading is false during refetch, and this effect sees the new id as missing and immediately calls setActiveWorkspaceId(null). That regresses the PR's quick-create behavior: create no longer reliably selects the workspace it just made. The stale-id cleanup needs to wait for an authoritative post-mutation/list-settled result or update the query/cache before selecting; do not treat a just-created id as stale against the pre-create list.

Non-blocking concerns:

  • frontend/src/components/WorkspaceBar.tsx:102-111 and 290-295 still expose selected workspace/all state only visually. CodeRabbit's aria-pressed note is valid accessibility debt, but I am not blocking this remediation on it because it predates this exact-head blocker fix and does not break the archive/order correctness path.
  • test/ia-workspace-router.test.ts:179-240 still does not assert that /restore makes the row visible in the default list in that specific metadata test, but the new order regression test does cover default listing after restore at lines 274-283. Fine as non-blocking.

What passes:

  • server/features/ia-workspace-router.ts:150 now computes fallback order from iaStore.listWorkspaces({ includeArchived: true }), so archived max order is not reused.
  • test/ia-workspace-router.test.ts:243-283 reproduces archived-max -> create -> restore and asserts active order values are unique.
  • Restore/archive store semantics remain stable: restore flips status back to active without renumbering, and list sorting is still order then id.

Bot feedback triage:

  • CodeRabbit inline major on fallback order: blocker, fixed at this head.
  • CodeRabbit inline on invalid persisted activeWorkspaceId: attempted but introduced the blocker above.
  • CodeRabbit outside-diff aria-pressed: valid follow-up/non-blocking.
  • CodeRabbit nit on restore-list test: mostly covered by the new order regression; non-blocking.
  • Gemini prior rename/deleteMutation comments: already fixed in earlier heads.
  • CodeRabbit issue comment rate-limit/billing/status boilerplate: noise.

Evidence inspected:

  • Exact head verified locally: 478318e.
  • Changed files reviewed: all 10 PR files accounted for; exact-head delta reviewed in server/features/ia-workspace-router.ts, test/ia-workspace-router.test.ts, frontend/src/components/WorkspaceBar.tsx; hook/query behavior traced through frontend/src/lib/hooks/use-ia-workspaces.ts.
  • npm ci --ignore-scripts to restore missing CodeMirror/lucide deps in this worktree.
  • npm rebuild better-sqlite3 under Node 24, then npm test -- test/ia-workspace-router.test.ts test/ia-store.test.ts -> 30 passed.
  • npm run check -> pass with existing warning-only lint baseline (220 warnings).

Bot feedback + learning:

  • Bot surfaces checked: review summaries, inline PR comments, issue comments.
  • Substantive bot findings: 1 blocker fixed, 1 attempted/found still blocking via Fugu review, 2 follow-up/nits, rate-limit/status noise ignored.
  • Learning retained: no — this is the existing durable UI async/query-state class already covered by review guidance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant