WS-FE-001-01: Canonical React + Vite app foundation#83
Open
jorshimayor wants to merge 2 commits into
Open
Conversation
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 reviews. How do review 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 refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (39)
📝 WalkthroughWalkthroughAdds a canonical React + Vite + TypeScript frontend app under ChangesFrontend Foundation Workstream
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant App
participant apiFetch
participant Backend
Browser->>App: load route
App->>apiFetch: getHealth()
apiFetch->>Backend: GET /api/v1/health
Backend-->>apiFetch: health status
apiFetch-->>App: HealthResponse
App->>apiFetch: getCurrentActor()
apiFetch->>apiFetch: getToken() from token.ts
apiFetch->>Backend: GET /api/v1/auth/me (Bearer token)
alt authenticated
Backend-->>apiFetch: ActorResponse
apiFetch-->>App: actor data
App->>App: render NavShell/BackendStatus with actorName
else unauthorized
Backend-->>apiFetch: 401 error
apiFetch-->>App: ApiError(401)
App->>App: render "signed out" via actorErrorLabel
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
jorshimayor
force-pushed
the
codex/ws-fe-001-01-frontend-foundation
branch
from
9ef030c to
2752ae4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
frontend/src/styles/app.css (1)
88-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding explicit
:focus-visiblestyles for nav links.The
.nav-linkand.nav-wordmarkhave:hoverand.is-activestyles but no:focus-visiblerule. While browsers provide default focus outlines, an explicit style ensures consistent keyboard navigation feedback across browsers and matches the design system's visual language.♿ Suggested focus styles
.nav-link.is-active { color: var(--accent); } +.nav-link:focus-visible, +.nav-wordmark:focus-visible { + outline: 2px solid var(--accent); + outline-offset: 2px; +} + /* Backend status strip — proves the /api/v1 connection */🤖 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/styles/app.css` around lines 88 - 101, Add explicit keyboard focus styling for the nav links so they match the hover/active states and work consistently across browsers. Update the `.nav-link` rule set in the app stylesheet to include a `:focus-visible` state, and apply the same treatment to `.nav-wordmark` if it uses the same navigation pattern. Keep the focus indicator aligned with the existing accent/branding behavior and ensure it remains distinct from hover and `.is-active` styles.frontend/src/styles/tokens.css (1)
29-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStylelint flags font-family keyword casing.
Stylelint's
value-keyword-caserule expects lowercase keywords."Georgia"and"Consolas"are flagged. Font family names are case-insensitive in CSS, so lowercasing is functionally safe and satisfies the linter.♻️ Proposed fix
- --font-serif: "Source Serif 4 Variable", "Source Serif 4", Georgia, "Times New Roman", serif; - --font-mono: "IBM Plex Mono", ui-monospace, "SF Mono", Consolas, monospace; + --font-serif: "Source Serif 4 Variable", "Source Serif 4", georgia, "Times New Roman", serif; + --font-mono: "IBM Plex Mono", ui-monospace, "SF Mono", consolas, monospace;🤖 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/styles/tokens.css` around lines 29 - 30, Stylelint is flagging the font-family keyword casing in the token definitions, specifically the serif and mono font stacks. Update the font family values in the tokens.css declarations for --font-serif and --font-mono to use lowercase keyword casing for the affected family names like Georgia and Consolas while keeping the rest of the stack unchanged. This should satisfy value-keyword-case without changing the intended font fallback behavior.Source: Linters/SAST tools
frontend/tsconfig.json (1)
21-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider type-checking
vite.config.ts.
include: ["src"]excludesvite.config.tsfromtsc --noEmit, so the Vite config file itself is not type-checked. This is a common pattern but typically addressed with a companiontsconfig.node.json. For a foundation PR, this is low risk since Vite parses its own config, but adding a node tsconfig would catch type errors in the config early.🤖 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/tsconfig.json` at line 21, The frontend tsconfig only includes src, so vite.config.ts is not covered by tsc --noEmit. Update the TypeScript setup around tsconfig.json to type-check the Vite config as well, typically by adding a companion node-focused tsconfig (for example, a tsconfig.node.json) and wiring vite.config.ts into it. Keep the existing src checks intact while ensuring the config file is validated during type-checking.frontend/package.json (1)
15-37: 🔒 Security & Privacy | 🔵 TrivialAdd a Node
enginesfloor
frontend/package.jsonshould declare Node 20.19+ / 22.12+ so installs fail fast on unsupported runtimes.🤖 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/package.json` around lines 15 - 37, Add a Node engines floor to the frontend/package.json so unsupported runtimes fail fast during install. Update the package manifest near the existing dependencies/devDependencies block to declare an engines entry for Node 20.19+ and 22.12+, keeping the version constraint explicit and consistent with the project's supported runtime. Ensure the package.json structure remains valid after adding the engines field.frontend/src/api/client.ts (1)
23-49: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a fetch timeout to prevent indefinite loading.
apiFetchhas no timeout. If the backend hangs, the UI stays in "loading" indefinitely with no recovery path. Consider wiring anAbortControllerwith a sensible default timeout, aborting on the cleanup path so callers (e.g.,useAsync) can also cancel in-flight requests on unmount or deps change.⏱️ Proposed timeout via AbortController
export async function apiFetch<T>(path: string, init: RequestInit = {}): Promise<T> { + const controller = new AbortController() + const timeoutId = setTimeout(() => controller.abort(), 15_000) + + const headers = new Headers(init.headers) headers.set('Accept', 'application/json') const token = getToken() if (token) { headers.set('Authorization', `Bearer ${token}`) } - const response = await fetch(`${apiBaseUrl()}${path}`, { ...init, headers }) + const response = await fetch(`${apiBaseUrl()}${path}`, { ...init, headers, signal: controller.signal }) + clearTimeout(timeoutId)🤖 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/api/client.ts` around lines 23 - 49, apiFetch currently waits on fetch indefinitely, so add AbortController-based timeout handling in apiFetch and propagate abort support through the request path. Use a sensible default timeout, merge it with any caller-provided init.signal, and ensure the request is aborted and cleaned up so consumers like useAsync can cancel in-flight calls on unmount or dependency changes. Keep the change localized around apiFetch and the fetch(...) call, preserving existing header/auth behavior and response parsing.
🤖 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
@.agent-loop/initiatives/WS-FE-001-frontend-operations-console/chunks/WS-FE-001-01-canonical-react-vite-foundation.md:
- Around line 57-60: The `INTENT`, `PLAN`, and `CHUNK_MAP` entries in the
canonical React Vite foundation chunk still say “to be created,” but those files
already exist in this PR. Update the chunk description to remove the stale
qualifier and make the references to `INTENT.md`, `PLAN.md`, and `CHUNK_MAP.md`
read as existing artifacts rather than pending ones.
- Around line 119-124: Update the documentation reference in the canonical
React/Vite foundation chunk so it points to website_flow.md instead of
website-flow.md; this is a filename mismatch in the visual-language/fonts note,
and the fix should align the reference with the actual doc path used elsewhere.
Locate the wording that mentions the approved Google Fonts stand-in and adjust
only that filename reference so the docs link stays consistent.
In
@.agent-loop/initiatives/WS-FE-001-frontend-operations-console/reviews/WS-FE-001-01-internal-review-evidence.md:
- Around line 12-15: The unit test summary in the evidence table is internally
inconsistent, so update the tally in the row that reports `npm run test --
--run` to match the parenthetical breakdown exactly. Edit the markdown summary
in the review evidence so the overall pass count, total count, and the
client/shell breakdown are all consistent and easy to audit.
In @.agent-loop/initiatives/WS-FE-001-frontend-operations-console/STATUS.md:
- Around line 7-10: Update the test summary in the STATUS.md entry so it matches
the paired review evidence; the Vitest count is stale. Adjust the status text
that currently reports 5/5 to reflect the final 8/8 result, keeping the
implementation and gate summary consistent with the evidence file and the STATUS
content.
In @.github/workflows/frontend.yml:
- Around line 20-21: The frontend workflow checkout step is leaving the
GITHUB_TOKEN persisted in git credentials by default. Update the
actions/checkout@v4 configuration in the workflow to disable credential
persistence by setting persist-credentials to false, keeping the rest of the
frontend job unchanged.
- Around line 11-13: Add a workflow-level permissions block to the frontend
workflow so GITHUB_TOKEN is least-privileged; in the workflow file near the jobs
section, define permissions with contents: read since this
lint/typecheck/test/build pipeline only needs repository read access. Keep the
change at the workflow root rather than inside individual jobs.
In `@frontend/src/components/actor.ts`:
- Around line 5-7: The actorName helper is using nullish coalescing, so an empty
actor.display_name will be returned instead of falling back to email or
external_subject. Update actorName to treat empty strings as missing by
normalizing display_name before the fallback chain or by using a falsy check,
while keeping the fallback order in place.
In `@frontend/src/components/BackendStatus.tsx`:
- Around line 14-31: The BackendStatus status strip updates dynamically as
health and actor state change, but the current status-strip container does not
announce those transitions to assistive technology. Update the BackendStatus
component so the div with className="status-strip" includes an aria-live value
(preferably polite) to ensure changes from loading to ready/unreachable/signed
out are announced by screen readers.
---
Nitpick comments:
In `@frontend/package.json`:
- Around line 15-37: Add a Node engines floor to the frontend/package.json so
unsupported runtimes fail fast during install. Update the package manifest near
the existing dependencies/devDependencies block to declare an engines entry for
Node 20.19+ and 22.12+, keeping the version constraint explicit and consistent
with the project's supported runtime. Ensure the package.json structure remains
valid after adding the engines field.
In `@frontend/src/api/client.ts`:
- Around line 23-49: apiFetch currently waits on fetch indefinitely, so add
AbortController-based timeout handling in apiFetch and propagate abort support
through the request path. Use a sensible default timeout, merge it with any
caller-provided init.signal, and ensure the request is aborted and cleaned up so
consumers like useAsync can cancel in-flight calls on unmount or dependency
changes. Keep the change localized around apiFetch and the fetch(...) call,
preserving existing header/auth behavior and response parsing.
In `@frontend/src/styles/app.css`:
- Around line 88-101: Add explicit keyboard focus styling for the nav links so
they match the hover/active states and work consistently across browsers. Update
the `.nav-link` rule set in the app stylesheet to include a `:focus-visible`
state, and apply the same treatment to `.nav-wordmark` if it uses the same
navigation pattern. Keep the focus indicator aligned with the existing
accent/branding behavior and ensure it remains distinct from hover and
`.is-active` styles.
In `@frontend/src/styles/tokens.css`:
- Around line 29-30: Stylelint is flagging the font-family keyword casing in the
token definitions, specifically the serif and mono font stacks. Update the font
family values in the tokens.css declarations for --font-serif and --font-mono to
use lowercase keyword casing for the affected family names like Georgia and
Consolas while keeping the rest of the stack unchanged. This should satisfy
value-keyword-case without changing the intended font fallback behavior.
In `@frontend/tsconfig.json`:
- Line 21: The frontend tsconfig only includes src, so vite.config.ts is not
covered by tsc --noEmit. Update the TypeScript setup around tsconfig.json to
type-check the Vite config as well, typically by adding a companion node-focused
tsconfig (for example, a tsconfig.node.json) and wiring vite.config.ts into it.
Keep the existing src checks intact while ensuring the config file is validated
during type-checking.
🪄 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 Plus
Run ID: 00b84123-139b-48ec-9139-8548d6f13368
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
.agent-loop/LOOP_STATE.md.agent-loop/WORK_QUEUE.md.agent-loop/initiatives/WS-FE-001-frontend-operations-console/CHUNK_MAP.md.agent-loop/initiatives/WS-FE-001-frontend-operations-console/INTENT.md.agent-loop/initiatives/WS-FE-001-frontend-operations-console/PLAN.md.agent-loop/initiatives/WS-FE-001-frontend-operations-console/STATUS.md.agent-loop/initiatives/WS-FE-001-frontend-operations-console/chunks/WS-FE-001-01-canonical-react-vite-foundation.md.agent-loop/initiatives/WS-FE-001-frontend-operations-console/reviews/WS-FE-001-01-internal-review-evidence.md.github/workflows/frontend.ymlREADME.mddocs/architecture_system_architecture.mddocs/decision_0012_frontend_stack_and_design_system.mddocs/design_system.mddocs/website_flow.mdfrontend/.gitignorefrontend/eslint.config.jsfrontend/index.htmlfrontend/package.jsonfrontend/src/App.tsxfrontend/src/api/auth.tsfrontend/src/api/client.tsfrontend/src/api/health.tsfrontend/src/api/types.tsfrontend/src/api/useAsync.tsfrontend/src/auth/token.tsfrontend/src/components/BackendStatus.tsxfrontend/src/components/NavShell.tsxfrontend/src/components/actor.tsfrontend/src/main.tsxfrontend/src/routes/NotFound.tsxfrontend/src/routes/Placeholder.tsxfrontend/src/routes/surfaces.tsfrontend/src/styles/app.cssfrontend/src/styles/tokens.cssfrontend/src/test/client.test.tsfrontend/src/test/setup.tsfrontend/src/test/shell.test.tsxfrontend/src/vite-env.d.tsfrontend/tsconfig.jsonfrontend/vite.config.ts
jorshimayor
added a commit
to jorshimayor/workstream
that referenced
this pull request
Jul 9, 2026
- a11y: aria-live="polite" on the backend status strip - actor.ts: treat empty/whitespace display_name as absent in the name fallback; add actor.ts unit tests (empty-name fallback + error labelling) - tests: make token-source tests hermetic (stub VITE_WORKSTREAM_TOKEN) so a local .env.local no longer affects the "no token" case - ci: frontend.yml gets a least-privilege permissions block (contents: read) and persist-credentials: false on checkout - docs: drop stale "(to be created)" plan refs; fix website-flow.md -> website_flow.md; sync test tally to 13/13 in evidence + STATUS Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
|
Thanks @coderabbitai — addressed all eight comments in e09d8c4. Code / CI
Docs
Also made the token-source tests hermetic (stub |
|
I'll take another pass over the updated commit to confirm everything lines up. ヽ(•‿•)ノ ✅ Action performedReview finished.
|
Establishes the single canonical Workstream internal operations console at `frontend/`, built with React + Vite + TypeScript: - Sets up a typed API client with Flow-style bearer token handling for `/api/v1` backend calls - Implements React Router routing with nav placeholders for the five core operations surfaces - Adds a status strip showing backend health and authenticated actor state - Applies the paper-and-ink design system per in-repo design specifications - Adds a full CI workflow for linting, typechecking, unit tests, and production builds - Updates architecture, design system, and README documentation with setup instructions - Passes all automated quality gates and aligns with upstream issue Flow-Research#50 and the WS-FE-001-01 chunk contract Note: The referenced `demos/week1_api_demo_ui` does not exist on upstream/main, so the demo retirement criterion is satisfied via documentation.
- a11y: aria-live="polite" on the backend status strip - actor.ts: treat empty/whitespace display_name as absent in the name fallback; add actor.ts unit tests (empty-name fallback + error labelling) - tests: make token-source tests hermetic (stub VITE_WORKSTREAM_TOKEN) so a local .env.local no longer affects the "no token" case - ci: frontend.yml gets a least-privilege permissions block (contents: read) and persist-credentials: false on checkout - docs: drop stale "(to be created)" plan refs; fix website-flow.md -> website_flow.md; sync test tally to 13/13 in evidence + STATUS Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jorshimayor
force-pushed
the
codex/ws-fe-001-01-frontend-foundation
branch
from
e09d8c4 to
588c170
Compare
Author
|
@Abiorh001 could you review this when you have a chance? 🙏 It's the WS-FE-001-01 canonical React + Vite frontend foundation (implements #50). All CodeRabbit comments are addressed, gates are green (lint/typecheck/tests 13-13/build), and the branch is rebased clean on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the canonical Workstream operations console foundation: a React + Vite +
TypeScript app under
frontend/. This is the first chunk of theWS-FE-001frontend initiative and maps 1:1 to this issue's acceptance criteria.
The chunk is intentionally the foundation only — app shell, design system, routing,
a typed
/api/v1client with Flow bearer-token handling, and navigationplaceholders. Real screens land in follow-up chunks (#49/#51/#52/#56).
Closes #50.
Acceptance criteria
tsc --noEmit && vite build)./api/v1/healthand/api/v1/auth/meagainst the backend (verifiedlive:
health→ 200,auth/me→ 401 without a token, 200 with a Flow-stylebearer token). The status strip renders both.
worker tasks, review queue, dashboards.
demos/week1_api_demo_uidoes not exist on
main; docs recordfrontend/as the one canonical UI andflag the stale reference rather than inventing a retirement.
Design + decisions
docs/design_system.md,docs/website_flow.md.docs/decision_0012_frontend_stack_and_design_system.md(ADR 0012): React Router, single
fetchclient, token-carry only (no sessionownership, per ADR 0006), Vitest + RTL, no UI component kit, no dark theme.
Verification
npm run lint,npm run typecheck,npm run test -- --run(8/8),npm run build— all green.frontendCI job (.github/workflows/frontend.yml) runs the four gates onfrontend/**.Internal review
Ran senior-engineering/architecture, security/auth, QA/test, and product/ops
tracks. All findings addressed (frontend CI job added, request-URL assertions
added, shared actor-error labelling, coverage gaps closed, prod-build token
caveat). Evidence:
.agent-loop/initiatives/WS-FE-001-frontend-operations-console/reviews/WS-FE-001-01-internal-review-evidence.md.Reviewer focus
/health,/auth/meauthed + 401)./api/v1contract change.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests