feat(web,api): tabbed workspace view with connected-work rail + folder-aware file listing#268
Conversation
Batched, unfiltered D1 lookup returning per-object-key metadata maps for a list of keys, chunked at 100 keys per statement to stay under D1/SQLite's bound-parameter limit. Hydrates gh.* metadata onto a workspace file listing (settings overhaul task 2).
GET /me/workspaces/:name/files now reads prefix/delimiter/cursor/limit from the query (mirroring the token-scoped GET /v1/:workspace/files convention) and hydrates each row's D1 gh.* metadata via getMetadataForKeys, so the settings-page file browser gets folder navigation plus connected-work context in one call.
Wraps GET /me/workspaces/:name/files?prefix=&cursor=&limit= (commit 0f9ac65) for the upcoming settings-page files tab. Mirrors searchWorkspaceFiles's credentialed-fetch/timeout idiom but, like the gallery/file list helpers, degrades to an empty listing on any transport failure, non-2xx, or malformed body rather than a Result union, since it backs a folder browser rather than the account overview. Normalizes the API's `cursor: string | null` to `string | undefined` and defaults `prefixes` to [] when omitted.
…test nits Review found WorkspaceFolderFile.url/embedUrl coercing a missing/null API value to "" instead of matching the API's string | null (and the sibling WorkspaceFile/SearchFileItem types in this file), which would have rendered as a dead link rather than letting the files table branch on null. Widened the type and dropped the ?? "" coercion. Also adds a partial-opts querystring test, drops a redundant assertion in the null-cursor test, and adds coverage locking in the null url/embedUrl passthrough.
Reworks the account sidebar nav to the flat 3A style (prompt-prefix active state, uppercase section labels) and adds an optional right-rail slot with a 3-col grid, all in a new account-shell.css scoped entirely under .account-shell so /admin/* (sharing signed-in-shell.css) is unaffected. Drops the workspacePage prop/type from AccountLayout and flattens workspaces-nav.ts to render membership rows without nested Invite sub-items; updates the two current callers accordingly.
…s tab WorkspaceFileTable collapsed a getMyWorkspaces outage and a workspace-not-in-list result (lost access / stale slug) into the same defaults, silently rendering the file table shell with "No files yet." instead of an error. Restores the pre-rewrite [name].astro's distinction (retryable outage vs. non-retryable no-access) via a new pure resolveWorkspaceInfo helper, with unit test coverage for all three outcomes.
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (2)
🚫 Excluded labels (none allowed) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-web | ac9c474 | Commit Preview URL Branch Preview URL |
Jul 19 2026, 07:58 PM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-api | 42ecf54 | Commit Preview URL Branch Preview URL |
Jul 19 2026, 06:42 PM |
- rail: render "Details unavailable." when getMyWorkspaces fails instead
of leaving the details list silently blank, mirroring the usage fetch
- settings.astro: drop a dead requireElement lookup (detailsEl is already
fetched and null-guarded just above)
- WorkspaceFileTable: extract a pluralCount helper in place of five
copy-pasted `${n} x${n === 1 ? "" : "s"}` ternaries (strings unchanged)
- workspace-file-row.test: extract a mkWorkspace fixture for the
resolveWorkspaceInfo cases (assertions unchanged)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/routes/me.ts (1)
189-201: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not exclude the default workspace from file browsing.
The communal early return at Line 201 makes this member-scoped route return no files for the configured default workspace. Remove that special case so every authorized workspace follows the same listing contract.
As per coding guidelines,
apps/api/src/routes/**/*.tsmust “not treat any workspace, includingdefault, as special.”🤖 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 `@apps/api/src/routes/me.ts` around lines 189 - 201, Remove the ws.communal early return from the GET /workspaces/:name/files handler so authorized members, including those in the configured default workspace, proceed through the standard file-listing flow. Keep the existing member authorization and listing contract unchanged, and do not add workspace-specific exceptions in this route.Source: Coding guidelines
🧹 Nitpick comments (1)
apps/web/src/lib/api-client.ts (1)
380-385: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winShared mutable fallback object risks cross-call state leakage.
EMPTY_FOLDER_LISTINGis a single object instance returned by reference from every degraded branch oflistWorkspaceFolder(line 437, 445). Itsfiles/prefixesarrays are shared across every caller and every future call. Today's only consumer (WorkspaceFileTable.tsx'sloadMore) spreads the arrays rather than mutating them, so nothing is broken yet — but any future code that doeslisting.files.push(...)or similar would silently corrupt the fallback for all subsequent calls across the app.♻️ Proposed fix: return a fresh literal per call
-const EMPTY_FOLDER_LISTING: WorkspaceFolderListing = { - files: [], - prefixes: [], - cursor: undefined, - communal: false, -}; +function emptyFolderListing(): WorkspaceFolderListing { + return { files: [], prefixes: [], cursor: undefined, communal: false }; +}- if (result.kind === "unavailable" || !result.response.ok) return EMPTY_FOLDER_LISTING; + if (result.kind === "unavailable" || !result.response.ok) return emptyFolderListing();- if (!body) return EMPTY_FOLDER_LISTING; + if (!body) return emptyFolderListing();Also applies to: 436-437, 445-445, 447-456
🤖 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 `@apps/web/src/lib/api-client.ts` around lines 380 - 385, Remove the shared EMPTY_FOLDER_LISTING fallback object and update each degraded return in listWorkspaceFolder to create a fresh WorkspaceFolderListing literal with new files and prefixes arrays. Preserve the existing cursor and communal values while ensuring callers cannot mutate fallback state shared across calls.
🤖 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 `@apps/web/src/pages/account/workspaces/`[name].astro:
- Around line 68-69: Remove the eager void boot() invocation near
onAstroPageLoad, leaving boot() registered only through onAstroPageLoad so the
initial page load mounts WorkspaceFileTable and fetches data once.
---
Outside diff comments:
In `@apps/api/src/routes/me.ts`:
- Around line 189-201: Remove the ws.communal early return from the GET
/workspaces/:name/files handler so authorized members, including those in the
configured default workspace, proceed through the standard file-listing flow.
Keep the existing member authorization and listing contract unchanged, and do
not add workspace-specific exceptions in this route.
---
Nitpick comments:
In `@apps/web/src/lib/api-client.ts`:
- Around line 380-385: Remove the shared EMPTY_FOLDER_LISTING fallback object
and update each degraded return in listWorkspaceFolder to create a fresh
WorkspaceFolderListing literal with new files and prefixes arrays. Preserve the
existing cursor and communal values while ensuring callers cannot mutate
fallback state shared across calls.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3551fd0c-4717-4b6a-9ae3-2a22d3b921a7
📒 Files selected for processing (26)
apps/api/src/file-metadata.tsapps/api/src/files-core.tsapps/api/src/routes/me.test.tsapps/api/src/routes/me.tsapps/api/test/file-metadata-sqlite.test.tsapps/web/src/components/WorkspaceFileTable.tsxapps/web/src/layouts/AccountLayout.astroapps/web/src/layouts/WorkspaceLayout.astroapps/web/src/lib/api-client.test.tsapps/web/src/lib/api-client.tsapps/web/src/lib/gh-context.test.tsapps/web/src/lib/gh-context.tsapps/web/src/lib/workspace-file-row.test.tsapps/web/src/lib/workspace-file-row.tsapps/web/src/lib/workspace-rail.test.tsapps/web/src/lib/workspace-rail.tsapps/web/src/lib/workspaces-nav.test.tsapps/web/src/lib/workspaces-nav.tsapps/web/src/pages-reachability.test.tsapps/web/src/pages/account/workspaces/[name].astroapps/web/src/pages/account/workspaces/[name]/galleries.astroapps/web/src/pages/account/workspaces/[name]/invite.astroapps/web/src/pages/account/workspaces/[name]/people.astroapps/web/src/pages/account/workspaces/[name]/settings.astroapps/web/src/styles/account-content.cssapps/web/src/styles/account-shell.css
| void boot(); | ||
| onAstroPageLoad(() => void boot()); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect onAstroPageLoad / onSession semantics to confirm initial-load behavior.
fd -a 'account-shell.ts' apps/web/src/lib | head -n1 | xargs -I{} \
ast-grep outline {} --items all
rg -nP -C4 '\bexport function (onAstroPageLoad|onSession)\b' apps/web/src/lib/account-shell.tsRepository: buildinternet/uploads
Length of output: 2148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='apps/web/src/lib/account-shell.ts'
page='apps/web/src/pages/account/workspaces/[name].astro'
echo '--- account-shell.ts excerpt ---'
sed -n '118,210p' "$file" | cat -n
echo
echo '--- workspace page excerpt ---'
sed -n '1,130p' "$page" | cat -n
echo
echo '--- onAstroPageLoad call sites ---'
rg -n '\bonAstroPageLoad\(' apps/web/src/pages apps/web/src/libRepository: buildinternet/uploads
Length of output: 8036
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the helper’s exact behavior and nearby page patterns for initial-load mounting.
file='apps/web/src/lib/account-shell.ts'
sed -n '118,170p' "$file" | cat -nRepository: buildinternet/uploads
Length of output: 2374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether other pages use the same pattern without a direct boot() call.
rg -n -C2 '\bvoid boot\(\);\s*$|\bonAstroPageLoad\(\(\) => void boot\(\)\)' apps/web/src/pagesRepository: buildinternet/uploads
Length of output: 578
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '118,145p' apps/web/src/lib/account-shell.ts | cat -nRepository: buildinternet/uploads
Length of output: 1238
🌐 Web query:
Astro client router event "astro:page-load" initial page load semantics documentation
💡 Result:
The astro:page-load event is a core lifecycle event provided by the Astro View Transitions API [1][2]. Its primary purpose is to act as a replacement for the standard browser DOMContentLoaded event when View Transitions are enabled, ensuring that client-side scripts execute correctly across both initial page loads and subsequent client-side navigations [3][1]. Semantics and Behavior The astro:page-load event fires at the very end of the navigation process [1][2]. According to official documentation, it is dispatched once the following conditions are met: 1. The new page is visible to the user [1][2]. 2. Blocking styles and scripts have finished loading [1][2]. This event is specifically designed to work for both the initial page load (for pre-rendered pages) and all subsequent navigations, whether they are forward or backward in history [1][2]. When using Astro's ClientRouter (or the earlier ViewTransitions component), developers should wrap code that needs to run on every page navigation—such as initializing event listeners or setting up UI components—within an event listener for astro:page-load on the document [1][4]. Technical Considerations - Event Dispatch: The client-side router waits for external scripts to execute before dispatching the event [5][6]. It generally does not wait for inline scripts, as these are expected to execute immediately [5]. - Initial Load Consistency: While the event is designed to fire on the initial load, there have been historical technical nuances regarding its behavior. For example, issues related to cyclic module dependencies or specific script loading patterns have been addressed to ensure consistent dispatch during initial page loads [7]. - Implementation Tip: Because the router manages the lifecycle of the DOM and scripts, developers should add listeners for astro:page-load in the synchronous part of their scripts [6]. This ensures the listener is attached before the event is dispatched [5][6]. Usage Example // Add an event listener to the document to handle setup tasks on every page load document.addEventListener("astro:page-load", => { // This runs on the initial page load and after every navigation setupUI; });
Citations:
- 1: https://docs.astro.build/en/guides/view-transitions/
- 2: https://v5.docs.astro.build/en/guides/view-transitions/
- 3: https://docs.astro.build/en/reference/modules/astro-transitions/
- 4: https://v7.docs.astro.build/en/guides/view-transitions/
- 5:
astro:page-loadbroken and inconsistent, breaks scripts withastro/astro#12858 - 6: Top-level awaits in imported modules break View Transitions events withastro/astro#13047
- 7: Ensure that the astro:page-load event gets dispatched on initial page loads withastro/astro#15205
Drop the eager boot() call
onAstroPageLoad already covers the initial page load, so void boot() makes this run twice on first paint. That can mount WorkspaceFileTable twice on the same container and issue duplicate fetches; remove the direct call or make boot() idempotent.
🤖 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 `@apps/web/src/pages/account/workspaces/`[name].astro around lines 68 - 69,
Remove the eager void boot() invocation near onAstroPageLoad, leaving boot()
registered only through onAstroPageLoad so the initial page load mounts
WorkspaceFileTable and fetches data once.
- files tab: remove the eager void boot() next to the onAstroPageLoad registration. astro:page-load fires on the initial load too (as the sibling tabs and the rail already rely on), so the eager call only double-mounted WorkspaceFileTable and double-fetched on first load. - api-client: replace the module-level EMPTY_FOLDER_LISTING singleton with an emptyFolderListing() factory so each degraded return in listWorkspaceFolder gets fresh files/prefixes arrays, removing the shared-mutable-state aliasing hazard.
…llow-ups (#274) The 3A files tab (#268) listed every key flat: the new client never sent delimiter=/, so the API returned no folder prefixes. Send it always, and lock the contract in the client tests. Stacked follow-ups from the same review pass: - responsive shell: mid-width (681-1080px) mode moves the rail below the content instead of letting the file grid paint over it; <=560px drops the size/type columns so filenames keep room - rail details: /me/workspaces now passes publicBaseUrl through (boolean hasPublicUrl kept for existing consumers) and the rail links the real public URL instead of the "configured" label - people tab: member-gated GET /me/workspaces/:name/members (sanitized via the new shared membersForOrg helper, also adopted by admin-ui) and a people list every member can see; invites stay admin-only - copy: bracketed [button] labels ([add], [copy], [invite teammate], [MEMBER]) flattened to the plain lowercase style used elsewhere
1 participant
Reworks the workspace view (
/account/workspaces/:name) into the converged "3A" tabbed design and makes the account file listing folder-aware with GitHub-metadata hydration.What changed
Workspace view → 4 tabs + summary rail
WorkspaceLayout(header +[data-tabrow]nav + right-hand summary rail) wrapping four routes:WorkspaceFileTable) with thumbnails, metadata filter chips ↔ folder breadcrumbs, a PR banner on an exact folder↔PR match, and a⋯menu (copy link / make public·private — no delete)./invite(old path 301-redirects to/people).gh.*file metadata), usage, details, and quick actions. The files tab pushes connected-work items into the rail via a documentedwindow.__uploadsSetConnectedWorkhook..account-shell.Backend (additive, backward-compatible)
GET /me/workspaces/:name/filesis now folder-aware (prefix/delimiter/cursor) and hydratesgh.*metadata from D1 in batchedIN(...)queries (getMetadataForKeys).ListedObject/listObjectsgained only optional fields; existing callers (files.ts,mcp/tools.ts) are unchanged. No new endpoint, no GraphQL — the Files SDK genuinely lacks metadata query, so this is the minimal additive REST change.Verification
aria-current;/invite→/people301 redirect; desktop 3-column grid (180px minmax(0,1fr) 272px, sticky rail); mobile 375px collapses to a single column with no horizontal overflow; no console errors.signed-in-shell.css,AdminLayout.astro,pages/admin/, andadmin-workspaces.csshave an empty diff across the whole branch;/adminrenders#dashboardwith zero.account-shell/.wft-*leakage (verified live).Screenshots
The populated files-tab visuals (thumbnails from real R2 files, gh-metadata chips, connected-work rows) require the full local stack (auth + api + web + a seeded workspace); they're not captured here. The shell/layout, routing, responsiveness, and admin isolation were verified via the dev server as noted above.
Known follow-ups (non-blocking)
ref(owner/repo#123), not the real PR/issue title (shipped intentionally title-less)..side-nested*rules insigned-in-shell.css— a separate PR, since this branch requires that file's diff to stay empty for admin parity.getMyWorkspacesfetch between the files island and the rail — unifying it would need a shared store / React↔vanilla coupling that doesn't exist today, so it's left as-is.(The earlier follow-ups for the rail's silent-blank outage state and a dead
requireElementcall were addressed inec02d12.)Notes
No changeset: this touches only
apps/web+apps/api, both on the changesetignorelist (Workers-deployed, not npm-published) — a changeset would block the CLI publish pipeline.Design:
.context/2026-07-19-settings-overhaul-design.md· Plan:.context/2026-07-19-settings-overhaul-plan.mdSummary by CodeRabbit
New Features
Bug Fixes
Tests