Integrate Supabase authentication and enhance store management#32
Closed
DDTTYYSS wants to merge 25 commits into
Closed
Integrate Supabase authentication and enhance store management#32DDTTYYSS wants to merge 25 commits into
DDTTYYSS wants to merge 25 commits into
Conversation
- Added Supabase client setup and authentication context to manage user sessions. - Implemented ProtectedRoute component to restrict access to authenticated users. - Updated frontend routing to include a login page and handle session state. - Enhanced API client to include authorization tokens for requests. - Updated backend dependencies to include PyJWT for token handling. - Added new environment variables for Supabase URL and anon key in settings.
- Added Supabase authentication configuration to .env.example. - Implemented authentication logic in auth.py to link users to their respective stores. - Updated Store model to include unique constraints for owner_email and nullable owner_auth_user_id. - Enhanced order field configuration routes to ensure users can only access their own stores. - Modified statistics route to utilize the current store context for fetching stats.
- Added support for LINE channel credentials in the Store model. - Implemented provisioning script to automate store creation and updates from a JSON configuration. - Updated routes and services to ensure store-specific data handling for chat rooms, orders, and payment methods. - Enhanced error handling for missing store configurations in webhook processing. - Introduced new utility functions for managing database connections to Supabase, optimizing for async operations. - Updated README and .env.example to reflect new configuration requirements for LINE integration.
- Added `display_config` JSON field to `store_order_field_config` for customizable order field visibility and order. - Introduced new API endpoints for managing store-specific order field configurations. - Updated order export functionality to utilize the new field configuration settings. - Enhanced README documentation to reflect changes in order field settings and API usage. - Added utility functions for building and formatting order field values for DOCX exports.
- Updated backend API to support OAuth-bound store access, removing the need for `X-Store-Id` in requests. - Introduced new endpoint `GET /stores/me` to retrieve the store associated with the logged-in owner. - Refactored frontend components to display the bound store name and adjust API calls accordingly. - Enhanced README documentation to reflect the new authentication and store selection process. - Added tests for the new store retrieval functionality to ensure proper access control and response structure.
- Added detailed steps for changing the store owner Gmail, including scenarios for stores with and without existing logins. - Included important notes regarding the implications of changing the owner email and the necessity of using the `--reset-owner-binding` flag. - Enhanced clarity on the relationship between `owner_email` and `owner_auth_user_id` for better user understanding.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
|
Worried about impact? Review this PR in Change Stack to explore blast radius before you approve or request changes. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughBack end: Supabase JWT resolves a single owner-bound store; routes, services, and LINE handling are store-scoped. Added migrations, provisioning CLI, and per-store LINE clients. Front end: Supabase auth, onboarding flow, protected routes, and /stores/me integration; API interceptor injects bearer token. ChangesSingle-store auth, per-store LINE, provisioning, and frontend onboarding
Sequence Diagram(s)sequenceDiagram
rect rgba(0, 150, 255, 0.5)
participant Browser
participant Frontend
participant Supabase
end
participant Backend
participant DB
participant LINE
Browser->>Frontend: request (with stored session)
Frontend->>Supabase: supabase.auth.getSession()
Frontend->>Backend: API request Authorization: Bearer <token>
Backend->>Supabase: GET /auth/user (token)
Supabase-->>Backend: user(id,email)
Backend->>DB: find/claim Store by owner_auth_user_id or owner_email
DB-->>Backend: Store
Backend-->>Frontend: Store-scoped response
LINE->>Backend: POST /callback (events + destination)
Backend->>DB: resolve_store_for_webhook(destination)
DB-->>Backend: Store
Backend->>Backend: webhook_handler_for_store(store).parse + dispatch
Backend->>LINE: reply/push via LineBotApi(store)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR upgrades the project to support per-store LINE channel credentials and routing (via LINE webhook destination → store.slug), and introduces Supabase OAuth/JWT-based authentication to scope staff dashboard access to a single, authenticated store. It also adds a provisioning workflow (JSON → DB) and updates frontend routing/UI to include login + onboarding flows.
Changes:
- Add per-store LINE credentials, multi-store webhook routing, and a store provisioning script/workflow.
- Introduce Supabase-authenticated store binding (
get_current_store) and scope staff APIs to the bound store. - Update frontend to use Supabase OAuth + onboarding routes, and remove the
X-Store-Idmodel in favor of Bearer JWT.
Reviewed changes
Copilot reviewed 82 out of 84 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updated docs for provisioning, multi-store LINE, OAuth single-store model |
| docs/CONTRACT.md | Contract updates for destination routing + auth requirements |
| docker-compose.yml | Frontend env wiring for Supabase config |
| .gitignore | Ignore provisioning secrets JSON |
| backend/.env.example | New Supabase/Auth + DB URL variables and guidance |
| backend/requirements.txt | Add PyJWT dependency |
| backend/Makefile | Add venv-safe targets for migrate/provision/test/run |
| backend/scripts/provision_stores.py | Provision stores from JSON and resolve LINE slug via bot info |
| backend/config/stores.provision.example.json | Example provisioning config |
| backend/docs/manual_migration_f1a2b3c4d5e6.sql | Manual SQL fallback for LINE credential columns |
| backend/alembic/versions/a9f3c2d1e4b7_supabase_bridge_revision.py | Bridge revision to align Alembic history (see review notes) |
| backend/alembic/versions/f1a2b3c4d5e6_add_store_line_credentials.py | Migration adding per-store LINE credential columns |
| backend/app/core/supabase_db_url.py | Helpers to rewrite Supabase pooler/direct DB URLs |
| backend/app/core/settings.py | Settings extended for Supabase Auth + DB URL resolution |
| backend/app/core/deps.py | Add Supabase user lookup for Bearer token validation |
| backend/app/core/auth.py | Implement store binding + store-scoped resource guards |
| backend/app/core/line_client.py | Per-store LINE API/WebhookHandler factory with env fallback |
| backend/app/models/store.py | Store model updated for LINE creds + owner binding fields |
| backend/app/schemas/store_provision.py | Pydantic schema for provisioning input |
| backend/app/utils/line_bot_info.py | LINE bot info helper (GET /v2/bot/info) for provisioning |
| backend/app/utils/line_send_message.py | Require passing per-store LineBotApi into send helpers |
| backend/app/utils/line_inbound_media.py | Require passing LineBotApi for media fetch |
| backend/app/utils/line_get_profile.py | Require passing LineBotApi for profile fetch |
| backend/app/repositories/store_repository.py | Destination parsing + store resolution + provisioning upsert |
| backend/app/repositories/user_repository.py | Scope user lookup by (line_uid, store_id) |
| backend/app/repositories/chat_repository.py | Add optional store filter to chat room listing |
| backend/app/repositories/order_repository.py | Add optional store scoping for order list queries |
| backend/app/repositories/payment_repository.py | Add optional store scoping for payment method list |
| backend/app/services/user_service.py | Propagate store_id into user lookup |
| backend/app/services/message_service.py | Use per-store LINE API for staff push + store checks |
| backend/app/services/order_service.py | Store-scoped list + per-store LINE push on order create |
| backend/app/services/payment_service.py | Store-scoped payment method listing |
| backend/app/usecases/linebot_flow.py | Route inbound LINE events through resolved store + per-store LINE API |
| backend/app/usecases/organize_order_draft.py | Use per-store LINE API for warning push |
| backend/app/routes/linebot.py | Resolve store via destination before dispatching handlers |
| backend/app/routes/stores.py | Add GET /stores/me using get_current_store |
| backend/app/routes/orders.py | Add store binding/scoping to several order endpoints |
| backend/app/routes/messages.py | Enforce store scoping on chat/message endpoints |
| backend/app/routes/order_field_config.py | Add OAuth-bound and store-id guarded config endpoints |
| backend/app/routes/payment.py | Enforce store scoping for payment method endpoints |
| backend/app/routes/statistics.py | Update stats endpoint to use OAuth-bound store |
| backend/app/routes/organize_data.py | Enforce store scoping for organize endpoint |
| backend/app/routes/export_docx.py | Enforce store scoping for docx export |
| backend/app/seeds/seed_user.py | Update seed error message for new provisioning flow |
| backend/tests/test_stores_me.py | Tests for /stores/me auth + binding behavior |
| backend/tests/test_store_provision.py | Tests for provisioning schema + LINE bot info helper |
| backend/tests/test_multi_store_line.py | Tests for destination parsing + webhook store resolution |
| backend/tests/test_models_pr_changes.py | Update Store column expectations (LINE creds) |
| frontend/package.json | Add @supabase/supabase-js + tslib |
| frontend/package-lock.json | Lockfile updates for Supabase deps |
| frontend/src/lib/supabase.ts | Supabase client initialization |
| frontend/src/api/client.ts | Attach Bearer JWT to API requests + 401 handling |
| frontend/src/contexts/AuthContext.tsx | Auth provider bridging Supabase session to app session |
| frontend/src/hooks/useAuth.ts | Auth hook for context access |
| frontend/src/router.tsx | Add /login + /onboarding routes and guards |
| frontend/src/pages/LoginPage.tsx | Supabase Google OAuth entry + redirects |
| frontend/src/layouts/OnboardingLayout.tsx | Onboarding shell layout |
| frontend/src/pages/onboarding/OnboardingNamePage.tsx | Onboarding step 1 (name) |
| frontend/src/pages/onboarding/OnboardingLineOfficialPage.tsx | Onboarding step 2 (LINE OA confirm) |
| frontend/src/components/onboarding/OnboardingCard.tsx | Shared onboarding card UI |
| frontend/src/components/onboarding/StepIndicator.tsx | Wizard step indicator |
| frontend/src/components/auth/RequireAuth.tsx | Guard for authenticated routes |
| frontend/src/components/auth/RequireOnboardingDone.tsx | Guard for onboarding completion |
| frontend/src/components/auth/OnboardingStepGuard.tsx | Guard for correct onboarding step |
| frontend/src/components/auth/OnboardingIndexRedirect.tsx | Redirect /onboarding → current step |
| frontend/src/components/auth/AuthLoading.tsx | Loading placeholder |
| frontend/src/components/auth/ProtectedRoute.tsx | Generic protected outlet (added) |
| frontend/src/layouts/AppLayout.tsx | App shell extracted (Navbar + Outlet) |
| frontend/src/App.tsx | Delegate to AppLayout |
| frontend/src/main.tsx | Wrap app in AuthProvider |
| frontend/src/context/StoreContext.tsx | Replace store picker list with OAuth-bound /stores/me |
| frontend/src/components/layout/StorePicker.tsx | Replace selector with single store name display |
| frontend/src/api/stores.ts | Add fetchMyStore (/stores/me) |
| frontend/src/api/orderFieldConfig.ts | Switch to OAuth-bound /store/order-field-config/default |
| frontend/src/context/OrderDisplayConfigContext.tsx | Remove store picker dependency and fetch OAuth-bound config |
| frontend/src/pages/settings/OrderFieldsPage.tsx | Placeholder onboarding finale route |
| frontend/src/lib/onboardingPaths.ts | Map onboarding step → route |
| frontend/src/lib/authStorage.ts | Local mock staff session storage |
| frontend/src/lib/auth.ts | Router helpers for auth redirects (future use) |
| frontend/src/api/auth.ts | Mock auth API surface for onboarding (future backend wiring) |
| frontend/src/types/auth.ts | Staff session/onboarding types |
| frontend/src/types/authApi.ts | Future auth API response/body types |
| frontend/src/config/support.ts | Support contact info for onboarding mismatch |
| frontend/src/config/mockLineOfficial.ts | Mock LINE OA display for onboarding |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)
backend/app/routes/orders.py:42
GET /orders/room/{room_id}is currently unauthenticated and not scoped to the OAuth-bound store. This allows any caller to enumerate orders for any room_id (cross-tenant data exposure). It should requireget_current_storeand verify the chat room belongs to that store (e.g.,get_chat_room_for_store) before returning data.
@api_router.get("/orders/room/{room_id}", response_model=List[OrderOut])
async def get_orders_by_room(room_id: int, db: AsyncSession = Depends(get_db)):
return await get_orders_by_room_id(db, room_id)
backend/app/routes/orders.py:62
PATCH /orders/{order_id}does not require auth or enforce store ownership, so any caller can modify orders across tenants if they know anorder_id. Please requireget_current_storeand callget_order_for_storebefore applying updates.
@api_router.patch("/orders/{order_id}", response_model=OrderOut)
async def patch_order(
order_id: int,
body: OrderPatchUpdate,
db: AsyncSession = Depends(get_db),
):
return await update_order_fields_by_id(db, order_id, body)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+99
to
+104
| if store is not None: | ||
| if not store.active: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_503_FORBIDDEN, | ||
| detail="Store is inactive", | ||
| ) |
Comment on lines
+26
to
35
| async def list_chat_rooms(db: AsyncSession, store_id: int | None = None) -> list[ChatRoom]: | ||
| stmt = ( | ||
| select(ChatRoom) | ||
| .options(joinedload(ChatRoom.customer)) | ||
| .where(ChatRoom.store_id == store_id) | ||
| .order_by(ChatRoom.updated_at.desc()) | ||
| ) | ||
| if store_id is not None: | ||
| stmt = stmt.where(ChatRoom.store_id == store_id) | ||
| result = await db.execute(stmt) |
Comment on lines
+13
to
+16
| revision = "a9f3c2d1e4b7" | ||
| down_revision = "e2f1a4b5c6d7" | ||
| branch_labels = None | ||
| depends_on = None |
Comment on lines
+16
to
+33
| __table_args__ = (UniqueConstraint("owner_email", name="uq_store_owner_email"),) | ||
|
|
||
| id: Mapped[int] = mapped_column(primary_key=True) | ||
| name: Mapped[str] = mapped_column(String, nullable=False) | ||
| # LINE webhook destination (channel user id, e.g. U4b…); must match JSON "destination" | ||
| slug: Mapped[str | None] = mapped_column(String, unique=True, nullable=True) | ||
| line_channel_access_token: Mapped[str | None] = mapped_column(String, nullable=True) | ||
| line_channel_secret: Mapped[str | None] = mapped_column(String, nullable=True) | ||
| timezone: Mapped[str] = mapped_column(String, nullable=False, default="Asia/Taipei") | ||
| active: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True) | ||
| created_at: Mapped[datetime] = mapped_column(DateTime, default=now_taipei_naive) | ||
| updated_at: Mapped[datetime] = mapped_column( | ||
| DateTime, default=now_taipei_naive, onupdate=now_taipei_naive | ||
| ) | ||
| owner_auth_user_id: Mapped[uuid.UUID] = mapped_column(UUID(as_uuid=True), nullable=False) | ||
| # 管理者指派的店主 Gmail（小寫）；首次登入時用來認領 store | ||
| owner_email: Mapped[str] = mapped_column(String, nullable=False) | ||
| # 首次登入時由 Supabase auth user id 綁定；未綁定前為 NULL（partial-unique 由 DB 端建立） | ||
| owner_auth_user_id: Mapped[uuid.UUID | None] = mapped_column(UUID(as_uuid=True), nullable=True) |
Comment on lines
+1
to
+8
| """add store LINE channel credentials | ||
|
|
||
| Revision ID: f1a2b3c4d5e6 | ||
| Revises: e2f1a4b5c6d7 | ||
| Create Date: 2026-06-04 | ||
|
|
||
| store.slug holds LINE webhook destination (channel user id). | ||
| """ |
Comment on lines
+17
to
+24
| <header className="mb-8 text-center"> | ||
| <p className="m-0 text-sm font-medium tracking-[2px] text-white/80"> | ||
| 奇美花店 | ||
| </p> | ||
| <h1 className="m-0 mt-1 text-2xl font-bold tracking-wide text-white"> | ||
| Chi-Mei Floral | ||
| </h1> | ||
| <p className="m-0 mt-2 text-sm text-white/75">帳號設定</p> |
Comment on lines
+30
to
+31
| env_file: | ||
| - ./frontend/.env.local |
Comment on lines
72
to
81
| # 更新 order 狀態（店家手動標示） | ||
| @api_router.patch("/order/{order_id}/status", response_model=OrderOut) | ||
| async def update_order_status( | ||
| order_id: int, | ||
| body: OrderStatusUpdate, | ||
| store: Store = Depends(get_current_store), | ||
| db: AsyncSession = Depends(get_db), | ||
| ): | ||
| await get_order_for_store(db, order_id, store) | ||
| return await update_order_status_by_id(db, order_id, body.status) |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/routes/orders.py (2)
40-42:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
GET /orders/room/{room_id}still bypasses store authorization.Lines 40-42 never resolve
get_current_storeor callget_chat_room_for_store(...), so a user who knows another store’sroom_idcan read its orders even though the rest of this router was moved to single-store scoping.Proposed fix
`@api_router.get`("/orders/room/{room_id}", response_model=List[OrderOut]) -async def get_orders_by_room(room_id: int, db: AsyncSession = Depends(get_db)): +async def get_orders_by_room( + room_id: int, + store: Store = Depends(get_current_store), + db: AsyncSession = Depends(get_db), +): + await get_chat_room_for_store(db, room_id, store) return await get_orders_by_room_id(db, room_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 `@backend/app/routes/orders.py` around lines 40 - 42, The GET /orders/room/{room_id} endpoint currently calls get_orders_by_room_id directly and skips store authorization; update the route handler get_orders_by_room to accept the current store (via Depends(get_current_store)) and before returning orders call get_chat_room_for_store(db, current_store.id, room_id) to validate the room belongs to that store (raise/propagate the same not-found/forbidden behavior if it doesn't), then call and return get_orders_by_room_id(db, room_id) only after validation.
55-69:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winThe remaining order-by-id endpoints still miss the ownership check.
patch_order(...)andsuggest_order_from_chat_route(...)call downstream services with a raworder_idbut never bind that ID to the authenticated store. That leaves both an unauthorized write path and an unauthorized read path after the rest of the file was hardened.Proposed fix
`@api_router.patch`("/orders/{order_id}", response_model=OrderOut) async def patch_order( order_id: int, body: OrderPatchUpdate, + store: Store = Depends(get_current_store), db: AsyncSession = Depends(get_db), ): + await get_order_for_store(db, order_id, store) return await update_order_fields_by_id(db, order_id, body) `@api_router.post`("/orders/{order_id}/suggest-from-chat", response_model=OrderSuggestFromChatOut) async def suggest_order_from_chat_route( order_id: int, + store: Store = Depends(get_current_store), db: AsyncSession = Depends(get_db), ): + await get_order_for_store(db, order_id, store) return await suggest_order_from_chat(db, order_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 `@backend/app/routes/orders.py` around lines 55 - 69, Both patch_order and suggest_order_from_chat_route call downstream helpers with an unvalidated raw order_id; add the same ownership check used elsewhere by fetching the authenticated store/user (e.g., via the existing dependency used in other routes) and either validate that the order belongs to that store or pass the bound store_id into update_order_fields_by_id and suggest_order_from_chat so the downstream functions enforce/store-scope the operation; specifically locate patch_order and suggest_order_from_chat_route and modify their call sites to include the authenticated store identifier (or perform a quick DB lookup to assert ownership) before calling update_order_fields_by_id(order_id, ...) or suggest_order_from_chat(order_id, ...).
🟠 Major comments (20)
frontend/src/config/support.ts-9-11 (1)
9-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace placeholder support channels before release.
These values are currently placeholder-only and will route users to non-functional contacts in a critical onboarding recovery path.
🤖 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/config/support.ts` around lines 9 - 11, The support config currently contains placeholder values—update the email, lineId, and hours fields in the support object (keys: email, lineId, hours) in frontend/src/config/support.ts to real, production-ready support contacts and business hours; replace 'support@example.com' with the official support email, '`@example-support`' with the real Line/IM identifier or remove if not used, and set accurate operating hours (localized string) so the onboarding recovery path routes to functional contacts.frontend/src/api/auth.ts-92-98 (1)
92-98:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
VITE_USE_MOCK_AUTHtoggle is currently ignored.Line 98 always exports
mockAuthApi, so settingVITE_USE_MOCK_AUTH='false'has no effect. This can silently ship mock auth behavior to non-mock environments.🔧 Proposed fix (minimum safe behavior)
/** Active auth API (mock for now). */ -export const authApi: AuthApi = mockAuthApi +if (!isMockAuthEnabled()) { + throw new Error('Real Auth API is not implemented yet, but VITE_USE_MOCK_AUTH=false') +} +export const authApi: AuthApi = mockAuthApi🤖 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/auth.ts` around lines 92 - 98, The export always uses mockAuthApi; change authApi to be conditionally assigned using isMockAuthEnabled(): export authApi should reference mockAuthApi when isMockAuthEnabled() is true and fall back to the real implementation (e.g., realAuthApi or productionAuthApi) when false. Update the declaration of authApi (symbol: authApi) to perform the runtime check and ensure it conforms to the AuthApi type and that the real implementation symbol (e.g., realAuthApi) is imported/available.frontend/src/contexts/AuthContext.tsx-64-67 (1)
64-67:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftBind cached staff session to the Supabase user identity.
Line 64 reuses cached local session for any authenticated user. If a different Google account signs in on the same browser, previous onboarding/store state is reused incorrectly. Session cache must be keyed to
supabaseSession.user.id(or cleared/recreated on mismatch).🔧 Proposed fix (identity-bound cache)
+const STAFF_SESSION_OWNER_KEY = 'flower_auth_session_owner' + function createStaffSessionFromUser(user: User): StaffSession { @@ setStaffSession(session) + localStorage.setItem(STAFF_SESSION_OWNER_KEY, user.id) return session } @@ function bridgeStaffSession(supabaseSession: Session | null): StaffSession | null { @@ const existing = getStaffSession() - if (existing) { + const ownerId = localStorage.getItem(STAFF_SESSION_OWNER_KEY) + if (existing && ownerId === supabaseSession.user.id) { return existing } + clearStaffSession() + localStorage.removeItem(STAFF_SESSION_OWNER_KEY) // TODO: replace with GET /auth/me when backend Google OAuth is ready. return createStaffSessionFromUser(supabaseSession.user) }🤖 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/contexts/AuthContext.tsx` around lines 64 - 67, The cached staff session returned by getStaffSession() is not bound to the current Supabase user, causing state bleed between different Google accounts; update the cache logic where getStaffSession() is used so it is keyed by supabaseSession.user.id (or verify supabaseSession.user.id against the cached session and clear/recreate the cache on mismatch). Specifically, when reading or writing the cached session in AuthContext (the existing variable and getStaffSession() call), include the supabaseSession.user.id in the cache key or add a check that compares cached.userId (or similar) to supabaseSession.user.id and reset the cached session if they differ. Ensure all places that persist or read the staff session (getStaffSession(), any setStaffSession/store writes) follow the same identity-bound keying and that onboarding/store state is cleared/recreated on user id mismatch.frontend/src/lib/supabase.ts-8-15 (1)
8-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast instead of using placeholder Supabase credentials.
Line 12 currently creates a real client even when config is missing (Line 8), which can silently route auth calls to a placeholder host and hide deployment misconfiguration. Make missing credentials a hard failure.
🔧 Proposed fix
if (!isSupabaseConfigured) { - console.error('Missing VITE_SUPABASE_URL or VITE_SUPABASE_ANON_KEY') + throw new Error('Missing VITE_SUPABASE_URL or VITE_SUPABASE_ANON_KEY') } export const supabase = createClient( - supabaseUrl ?? 'https://placeholder.supabase.co', - supabaseAnonKey ?? 'placeholder', + supabaseUrl!, + supabaseAnonKey!, { auth: {🤖 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/lib/supabase.ts` around lines 8 - 15, The code currently logs missing Supabase config but still instantiates supabase with placeholders; change this to fail fast by throwing an error (or exiting) when isSupabaseConfigured is false so no createClient call runs with superset placeholders. Locate the isSupabaseConfigured check and the export const supabase = createClient(...) and replace the placeholder-instantiation path with a hard failure that includes supabaseUrl/supabaseAnonKey context (or a clear message) so createClient is only called when both supabaseUrl and supabaseAnonKey are present.frontend/src/context/OrderDisplayConfigContext.tsx-122-123 (1)
122-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFallback key misses existing per-store local cache on fetch failure.
When the first authenticated fetch fails,
fallbackKeybecomesorder-display-config:unknown, which cannot match keys written viabuildOrderDisplayStorageKey(remote.store_id). This makes the “use local cache” branch load defaults instead of the previously cached store config.Suggested fix
- const fallbackKey = storageKey ?? 'order-display-config:unknown' - const fromLocal = mergeWithRegistry(loadConfig(fallbackKey)) + const fromLocal = mergeWithRegistry( + storageKey ? loadConfig(storageKey) : loadConfig(), + )🤖 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/context/OrderDisplayConfigContext.tsx` around lines 122 - 123, The fallbackKey currently defaults to 'order-display-config:unknown' which prevents matching per-store cached keys; change the fallbackKey computation to prefer an actual per-store key by using buildOrderDisplayStorageKey(remote?.store_id) when storageKey is undefined (e.g. storageKey ?? buildOrderDisplayStorageKey(remote?.store_id) ?? 'order-display-config:unknown'), then pass that key into loadConfig/mergeWithRegistry so loadConfig can find the store-specific local cache; ensure remote is referenced safely (optional chaining) so this still falls back to 'unknown' if no store_id is available.frontend/src/router.tsx-42-44 (1)
42-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestrict
/settings/order-fieldsto the intended onboarding step only.Line 42 currently treats every non-
DONEstate the same, so a user inNAMEcan jump directly to/settings/order-fieldsand bypass required onboarding steps. Gate onlyLINE_OAhere, and redirect earlier steps to their canonical onboarding route.💡 Proposed fix
import { useAuth } from '`@/hooks/useAuth`' +import { getOnboardingPath } from '`@/lib/onboardingPaths`' @@ function OrderFieldsRoute() { const { session, isLoading } = useAuth() @@ - if (session.onboardingStep !== 'DONE') { + if (session.onboardingStep === 'LINE_OA') { return <OrderFieldsPage /> } + if (session.onboardingStep !== 'DONE') { + return <Navigate to={getOnboardingPath(session.onboardingStep)} replace /> + } + return <OrderFieldSettingsPage /> }🤖 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/router.tsx` around lines 42 - 44, The current guard in the router checks session.onboardingStep !== 'DONE' and returns OrderFieldsPage, which lets earlier steps (e.g., 'NAME') access /settings/order-fields; change this to only allow OrderFieldsPage when session.onboardingStep === 'LINE_OA' and otherwise redirect users in earlier states to their canonical onboarding route (map other onboardingStep values to their respective onboarding pages and return that route/component instead). Locate and update the condition around session.onboardingStep and OrderFieldsPage so only 'LINE_OA' shows OrderFieldsPage, and implement redirects for other steps.docker-compose.yml-30-31 (1)
30-31:⚠️ Potential issue | 🟠 MajorAvoid startup dependency on uncommitted
./frontend/.env.local.
docker-compose.ymlconfigures thefrontendservice with:env_file: - ./frontend/.env.localThe repo doesn’t include
frontend/.env.local, and.gitignoreignores**/.env*/*.local, so fresh clones/CI commonly fail duringdocker compose up. Use a committedfrontend/.env.example+ copy/init step in the image/entrypoint, or pass required values viaenvironment(with sensible defaults).🤖 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 `@docker-compose.yml` around lines 30 - 31, The frontend service in docker-compose.yml is configured to load an uncommitted .env.local file which causes startup failures in fresh clones and CI environments. Replace the env_file directive that references ./frontend/.env.local with one of two approaches: either create a committed ./frontend/.env.example file and add an entrypoint or Dockerfile step that copies and initializes it for the frontend service, or move the environment variable declarations directly into the frontend service's environment section in docker-compose.yml with sensible defaults so the service can start without external file dependencies.frontend/src/pages/onboarding/OnboardingNamePage.tsx-37-40 (1)
37-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAwait profile update before routing to next onboarding step.
At Line 38,
updateDisplayName(trimmed)is not awaited, so async failures won’t be caught by thistry/catch, and navigation can occur before persistence completes.Suggested fix
- function handleSubmit(event: FormEvent<HTMLFormElement>) { + async function handleSubmit(event: FormEvent<HTMLFormElement>) { event.preventDefault() setError(null) @@ try { - updateDisplayName(trimmed) + await updateDisplayName(trimmed) navigate('/onboarding/line-official', { replace: true }) } catch (err) { setError(err instanceof Error ? err.message : '無法儲存名稱') } }🤖 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/pages/onboarding/OnboardingNamePage.tsx` around lines 37 - 40, The call to updateDisplayName(trimmed) is missing an await so async errors escape the try/catch and navigation via navigate('/onboarding/line-official', { replace: true }) can happen before the update completes; change the flow in OnboardingNamePage by awaiting updateDisplayName(trimmed) inside the try block (or handling its returned Promise with .then/.catch) and only call navigate after the awaited update resolves, keeping the existing try/catch to surface errors from updateDisplayName.backend/app/models/store.py-33-33 (1)
33-33:⚠️ Potential issue | 🟠 MajorAdd DB-level unique/partial-unique enforcement for
store.owner_auth_user_id(auth flow currently assumes nullable).Alembic history defines the
owner_auth_user_idcolumn (e.g.,backend/alembic/versions/f4e8bb2a9031_multitenant_schema.py) but does not create any unique/partial-unique index/constraint for non-nullstore.owner_auth_user_id; the only explicit uniqueness in the model isuq_store_owner_email. Also, the migration setsowner_auth_user_idasnullable=False, whilebackend/app/models/store.pymarks itnullable=Trueandbackend/app/core/auth.pyrelies onStore.owner_auth_user_id IS NULLto bind the first-login store.🤖 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 `@backend/app/models/store.py` at line 33, Model and migrations are inconsistent: owner_auth_user_id is nullable in Store (owner_auth_user_id) but migration made it non-null and no DB uniqueness exists for non-null owners; add a DB-level unique constraint/index for non-null owner_auth_user_id and make migrations and model consistent with the auth flow. Update the model (Store) to keep owner_auth_user_id nullable=True, then add a new Alembic migration that creates a partial unique index (e.g., name it uq_store_owner_auth_user_id) on owner_auth_user_id WHERE owner_auth_user_id IS NOT NULL (or the DB-specific equivalent), and ensure the migration does not set nullable=False; finally reference uq_store_owner_email to avoid name collisions and run the migration to enforce uniqueness at the DB level used by auth.py checks.backend/app/core/deps.py-39-54 (1)
39-54:⚠️ Potential issue | 🟠 MajorDifferentiate Supabase upstream failures from invalid-token responses; narrow the
httpxexception scope.
backend/app/core/deps.pyget_current_usercurrently maps any non-200 Supabase response to401 "Invalid or expired token", so upstream429/5xxbecome invalid-token errors. It also catches all exceptions (except Exception), which can mask non-network defects as503.🔧 Suggested fix
async def get_current_user(token: str = Depends(oauth2_scheme)) -> dict: settings = get_settings() + if not settings.supabase_url or not settings.supabase_anon_key: + raise HTTPException(status_code=503, detail="Auth service misconfigured") try: - async with httpx.AsyncClient() as client: + async with httpx.AsyncClient(timeout=5.0) as client: resp = await client.get( f"{settings.supabase_url}/auth/v1/user", headers={ "Authorization": f"Bearer {token}", "apikey": settings.supabase_anon_key, }, - timeout=5.0, ) - except Exception: - raise HTTPException(status_code=503, detail="Auth service unavailable") - if resp.status_code != 200: + except httpx.RequestError as err: + raise HTTPException(status_code=503, detail="Auth service unavailable") from err + + if resp.status_code in (401, 403): raise HTTPException(status_code=401, detail="Invalid or expired token") + if resp.status_code >= 500: + raise HTTPException(status_code=503, detail="Auth service unavailable") + if resp.status_code != 200: + raise HTTPException(status_code=502, detail="Auth service error") return resp.json()🤖 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 `@backend/app/core/deps.py` around lines 39 - 54, In get_current_user, narrow the broad except and distinguish upstream failures from actual invalid tokens: catch httpx.RequestError (or httpx.HTTPError subclasses related to network/timeouts) and raise HTTPException(status_code=503, detail="Auth service unavailable"), then after the client.get examine resp.status_code and only raise HTTPException(status_code=401, detail="Invalid or expired token") when resp.status_code == 401; for 429 or any 5xx return raise HTTPException(status_code=503, detail=f"Auth service error ({resp.status_code})") so upstream rate-limit/server errors are surfaced correctly.Source: Linters/SAST tools
backend/config/stores.provision.example.json-5-5 (1)
5-5:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace real email with a neutral placeholder in sample config.
The example file currently includes a specific personal-looking email address; sample provisioning templates should avoid embedding identifiable real addresses.
💡 Suggested fix
- "owner_email": "1ddttyyss@gmail.com", + "owner_email": "owner@example.com",🤖 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 `@backend/config/stores.provision.example.json` at line 5, The sample config sets owner_email to a real-looking address ("1ddttyyss@gmail.com"); replace the value of the "owner_email" key in the example provisioning JSON with a neutral placeholder such as "owner@example.com" or "user@example.com" to avoid embedding identifiable personal data in sample templates.backend/app/schemas/store_provision.py-15-25 (1)
15-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject blank required fields after trimming.
Whitespace-only values currently validate to
""for required fields, so invalidowner_email/ LINE credentials can pass schema validation and fail later in provisioning/runtime.💡 Suggested fix
class StoreProvisionEntry(BaseModel): @@ - `@field_validator`("name", "owner_email", "line_channel_access_token", "line_channel_secret", "slug") + `@field_validator`("name", "owner_email", "line_channel_access_token", "line_channel_secret") `@classmethod` - def strip_strings(cls, v: str | None) -> str | None: - if v is None: - return None - return v.strip() + def normalize_required_strings(cls, v: str) -> str: + value = v.strip() + if not value: + raise ValueError("must not be blank") + return value + + `@field_validator`("slug") + `@classmethod` + def normalize_slug(cls, v: str | None) -> str | None: + if v is None: + return None + value = v.strip() + return value or None @@ `@field_validator`("owner_email") `@classmethod` def lowercase_email(cls, v: str) -> str: - return v.strip().lower() + return v.lower()🤖 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 `@backend/app/schemas/store_provision.py` around lines 15 - 25, The current validators strip_strings and lowercase_email allow whitespace-only inputs to become empty strings which then pass validation; update these field validators (registering on fields name, owner_email, line_channel_access_token, line_channel_secret, slug and owner_email specifically) to trim the input and raise a ValueError when the resulting string is empty (e.g., after v.strip() if not v: raise ValueError("...")), and ensure lowercase_email still returns a lowercased, trimmed value (and raises if empty) so blank/whitespace-only required fields are rejected during schema validation.backend/app/services/order_service.py-181-186 (1)
181-186:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t let LINE push failures break order creation response.
Lines 182-185 can raise (missing LINE creds/API failure) after the order commit at Line 176, causing a 500 on a partially successful flow.
Suggested fix
if line_uid: - store = await get_store_by_id(db, room.store_id) - if store: - line_api = line_bot_api_for_store(store) - LINE_push_message(line_api, line_uid, ChatMessagePayload(text=msg)) + try: + store = await get_store_by_id(db, room.store_id) + if store: + line_api = line_bot_api_for_store(store) + LINE_push_message(line_api, line_uid, ChatMessagePayload(text=msg)) + except Exception: + # Keep order flow successful even if notification fails. + pass🤖 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 `@backend/app/services/order_service.py` around lines 181 - 186, The LINE push call after order commit (the block using get_store_by_id, line_bot_api_for_store, and LINE_push_message with ChatMessagePayload) can raise and currently will turn a successful order into a 500; wrap the entire post-commit LINE notification sequence in a try/except (or equivalent error-handling for async) so exceptions are caught, log the failure with context (store_id, line_uid, exception) and do not re-raise or affect the HTTP response; alternatively, dispatch the notification to a background task/worker to decouple failures from order creation.backend/app/repositories/store_repository.py-114-121 (1)
114-121:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove destination→first-store fallback to prevent cross-store webhook routing.
Lines 114-121 route unknown destinations to the first store whenever an env secret exists. That can mis-attribute events to the wrong tenant.
Suggested fix
- settings: Settings = load_settings() - secret = (settings.line_channel_secret or "").strip() - if secret: - store_id = await get_first_store_id(db) - if store_id is not None: - fallback = await get_store_by_id(db, store_id) - if fallback is not None: - return fallback - raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail=f"No store for LINE destination: {destination}", )🤖 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 `@backend/app/repositories/store_repository.py` around lines 114 - 121, The code is returning the first store when a line_channel_secret exists (using load_settings, secret, get_first_store_id, get_store_by_id) which enables cross-store webhook routing; remove the fallback branch that fetches get_first_store_id and returns fallback (i.e. delete the if secret → get_first_store_id → get_store_by_id → return fallback block) so that unknown destinations are not silently mapped to the first store and the function returns None (or propagates the original no-match behavior) instead; ensure any callers handle a None result and update tests that assumed the fallback.backend/app/usecases/organize_order_draft.py-176-180 (1)
176-180:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard warning push so organize flow doesn’t fail on notification errors.
Lines 177-180 can throw on missing store LINE credentials or LINE API errors, which aborts the organize flow instead of just skipping notification.
Suggested fix
line_uid = await get_line_uid_by_chatroom_id(db, chat_room.id) if line_uid: - store = await get_store_by_id(db, chat_room.store_id) - if store: - line_api = line_bot_api_for_store(store) - LINE_push_message(line_api, line_uid, ChatMessagePayload(text=warning_msg)) + try: + store = await get_store_by_id(db, chat_room.store_id) + if store: + line_api = line_bot_api_for_store(store) + LINE_push_message(line_api, line_uid, ChatMessagePayload(text=warning_msg)) + except Exception: + pass🤖 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 `@backend/app/usecases/organize_order_draft.py` around lines 176 - 180, The notification call can raise and abort the organize flow; wrap the LINE notification in a safe guard: after retrieving store via get_store_by_id(db, chat_room.store_id) and constructing line_api with line_bot_api_for_store(store), verify the store has valid LINE credentials (or that line_api is truthy) and invoke LINE_push_message(line_api, line_uid, ...) inside a try/except block that catches API/connection errors, logs them, and suppresses them so the organize flow continues. Ensure you reference get_store_by_id, line_bot_api_for_store, and LINE_push_message when applying the change.backend/app/repositories/store_repository.py-81-81 (1)
81-81:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate webhook payload shape before accessing
destination.Line 81 assumes
json.loadsreturns a dict. A valid JSON array/string causesAttributeErrorand a 500 instead of a 400.Suggested fix
def parse_webhook_destination(body_str: str) -> str: try: payload = json.loads(body_str) except json.JSONDecodeError as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid webhook JSON", ) from e + if not isinstance(payload, dict): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Invalid webhook JSON", + ) destination = (payload.get("destination") or "").strip()🤖 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 `@backend/app/repositories/store_repository.py` at line 81, After json.loads parses the webhook body, ensure the resulting payload is a dict before calling payload.get(...)—replace the blind access at the line with "destination = (payload.get('destination') or '').strip()" by first checking isinstance(payload, dict); if it's not a dict, return/raise a 400 Bad Request (or the repository's equivalent) with a clear message about expecting an object, log the malformed payload, and only then extract destination. Reference the payload variable and the exact assignment "destination = (payload.get('destination') or '').strip()" when making the change.backend/app/repositories/payment_repository.py-9-14 (1)
9-14:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
store_idmandatory to prevent accidental cross-store reads.Allowing
store_id=Nonereturns all payment methods. That creates a tenant-isolation gap if any caller omits the argument. Make this repository API store-scoped by default, and keep global listing in a separate explicit admin-only method if needed.Proposed fix
-async def list_payment_methods( - db: AsyncSession, store_id: int | None = None -) -> list[PaymentMethod]: +async def list_payment_methods( + db: AsyncSession, store_id: int +) -> list[PaymentMethod]: stmt = select(PaymentMethod) - if store_id is not None: - stmt = stmt.where(PaymentMethod.store_id == store_id) + stmt = stmt.where(PaymentMethod.store_id == store_id) result = await db.execute(stmt) return result.scalars().all()# Also align the service contract: # backend/app/services/payment_service.py # async def get_all_payment_methods(db: AsyncSession, store_id: int) -> list[PaymentMethodBase]: # ...🤖 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 `@backend/app/repositories/payment_repository.py` around lines 9 - 14, The repository function list_payment_methods currently allows store_id to be omitted which can return all payment methods; change its API to require a store_id (make the parameter non-optional in list_payment_methods) and remove the default None handling (drop the if store_id is not None branch), update any callers and the service contract (e.g., payment_service.get_all_payment_methods → get_all_payment_methods(db: AsyncSession, store_id: int) or similar) to pass a store_id, and create a separate explicit admin-only method if global listing is needed; ensure types/signatures and any docstrings/tests are updated to reflect the required store-scoped parameter.backend/app/utils/line_send_message.py-62-94 (1)
62-94:⚠️ Potential issue | 🟠 MajorHandle LINE reply failures in
send_quick_reply_message/send_confirmto prevent webhook 500s
send_quick_reply_messageandsend_confirmcallline_bot_api.reply_message(...)without catchingLineBotApiError, and/callbackinbackend/app/routes/linebot.pydoesn’t intercept these exceptions—so transient LINE API errors can propagate and fail the request path. Add localized handling aroundreply_message(mirroringLINE_push_message’sLineBotApiErrorhandling) and return a success boolean.Suggested fix
-def send_quick_reply_message( +def send_quick_reply_message( line_bot_api: LineBotApi, reply_token: str, text: str, options: list[str] -): +) -> bool: items = [ QuickReplyButton(action=MessageAction(label=opt, text=opt)) for opt in options ] - line_bot_api.reply_message( - reply_token, - TextSendMessage(text=text, quick_reply=QuickReply(items=items)), - ) + try: + line_bot_api.reply_message( + reply_token, + TextSendMessage(text=text, quick_reply=QuickReply(items=items)), + ) + return True + except LineBotApiError as e: + logging.error(f"[LINE REPLY] quick reply failed: {e.status_code} - {e.error.message}") + return False def send_confirm( @@ -): +) -> bool: @@ - line_bot_api.reply_message( - reply_token, - TemplateSendMessage(alt_text=text, template=tpl), - ) + try: + line_bot_api.reply_message( + reply_token, + TemplateSendMessage(alt_text=text, template=tpl), + ) + return True + except LineBotApiError as e: + logging.error(f"[LINE REPLY] confirm failed: {e.status_code} - {e.error.message}") + return False🤖 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 `@backend/app/utils/line_send_message.py` around lines 62 - 94, Wrap the calls to line_bot_api.reply_message in both send_quick_reply_message and send_confirm with a try/except that catches LineBotApiError (same handling as LINE_push_message): log the error (including exception details) and return False on exception, otherwise return True on success; ensure you import LineBotApiError if not present and preserve existing logging behavior/format used by LINE_push_message so webhook `/callback` won't raise on transient LINE API failures.backend/app/utils/line_get_profile.py-5-8 (1)
5-8:⚠️ Potential issue | 🟠 MajorAvoid blocking LINE SDK call inside async function
backend/app/utils/line_get_profile.pydefinesasync fetch_user_profile, but it calls the synchronousline_bot_api.get_profile(...)directly (network I/O). Sincebackend/app/usecases/linebot_flow.pyawaits this helper, it can block the event loop under load. Offload it (or use the SDK’s async client).Suggested fix
+import asyncio from linebot import LineBotApi from linebot.exceptions import LineBotApiError async def fetch_user_profile(line_bot_api: LineBotApi, user_id: str): try: - profile = line_bot_api.get_profile(user_id) + profile = await asyncio.to_thread(line_bot_api.get_profile, user_id) return profile except LineBotApiError as e: print(f"Error: {e.status_code} {e.error.message}") return None🤖 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 `@backend/app/utils/line_get_profile.py` around lines 5 - 8, The async function fetch_user_profile currently calls the synchronous line_bot_api.get_profile which performs network I/O and can block the event loop; change fetch_user_profile to offload that blocking call to a thread (e.g., use asyncio.to_thread or loop.run_in_executor) or switch to the SDK’s async client, so the call to line_bot_api.get_profile (or its async equivalent) does not run on the event loop thread; update callers (e.g., those in linebot_flow.py) to continue awaiting the async fetch_user_profile as before.backend/app/repositories/chat_repository.py-27-34 (1)
27-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the unconditional store filter.
Line 30 already applies
ChatRoom.store_id == store_id, sostore_id=NonebecomesWHERE store_id IS NULLinstead of “no store filter.” That breaks the new optional contract now exposed byget_chat_room_list(...)and will return the wrong rooms whenever a caller omitsstore_id.Proposed fix
async def list_chat_rooms(db: AsyncSession, store_id: int | None = None) -> list[ChatRoom]: stmt = ( select(ChatRoom) .options(joinedload(ChatRoom.customer)) - .where(ChatRoom.store_id == store_id) .order_by(ChatRoom.updated_at.desc()) ) if store_id is not None: stmt = stmt.where(ChatRoom.store_id == store_id) result = await db.execute(stmt) return result.scalars().all()🤖 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 `@backend/app/repositories/chat_repository.py` around lines 27 - 34, The query always applies ChatRoom.store_id == store_id when building stmt, causing store_id=None to become WHERE store_id IS NULL; remove that unconditional .where(...) so the base stmt uses only select(ChatRoom).options(joinedload(ChatRoom.customer)).order_by(ChatRoom.updated_at.desc()), and keep the conditional filter inside the if store_id is not None block (so only apply ChatRoom.store_id == store_id when a store_id is passed); update the code in get_chat_room_list where stmt is constructed and the stmt variable is modified accordingly.
🟡 Minor comments (11)
frontend/src/pages/onboarding/OnboardingLineOfficialPage.tsx-30-37 (1)
30-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle auth action exceptions before routing.
confirmLineOfficial()can throw when no active session; this click path currently has no fallback and can fail with an unhandled error instead of redirecting cleanly.Proposed patch
function handleConfirmCorrect() { - confirmLineOfficial() - navigate('/settings/order-fields', { replace: true }) + try { + confirmLineOfficial() + navigate('/settings/order-fields', { replace: true }) + } catch { + navigate('/login?error=session_expired', { replace: true }) + } } function handleRejectWrongAccount() { - rejectWrongAccount() - navigate('/login?error=wrong_account', { replace: true }) + try { + rejectWrongAccount() + } finally { + navigate('/login?error=wrong_account', { replace: true }) + } }🤖 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/pages/onboarding/OnboardingLineOfficialPage.tsx` around lines 30 - 37, The click handlers call confirmLineOfficial() and rejectWrongAccount() without guarding against exceptions which can abort navigation; wrap the calls in try/catch (and await them if they return a Promise) inside handleConfirmCorrect and handleRejectWrongAccount so any thrown errors are caught/logged and navigation via navigate('/settings/order-fields', { replace: true }) and navigate('/login?error=wrong_account', { replace: true }) always runs as the fallback; reference the functions handleConfirmCorrect, confirmLineOfficial, handleRejectWrongAccount, rejectWrongAccount, and navigate when making this change.frontend/src/lib/authStorage.ts-12-19 (1)
12-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
rolein the runtime session guard.
isStaffSessioncurrently accepts objects withoutrole, so malformed localStorage payloads can be treated as valid sessions.🔧 Proposed fix
return ( typeof s.staffId === 'number' && typeof s.storeKey === 'string' && typeof s.displayName === 'string' && + typeof s.role === 'string' && (s.onboardingStep === 'NAME' || s.onboardingStep === 'LINE_OA' || s.onboardingStep === 'DONE') )🤖 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/lib/authStorage.ts` around lines 12 - 19, The runtime session guard is missing validation for the role property, so isStaffSession can accept objects without role; update isStaffSession to also check that s.role exists and is one of the allowed staff roles (e.g., a non-empty string or specific enum values your app uses) along with existing checks for staffId, storeKey, displayName, and onboardingStep; locate the isStaffSession function in authStorage.ts and add the role validation to the boolean expression to reject malformed localStorage payloads.frontend/src/contexts/AuthContext.tsx-160-163 (1)
160-163:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep auth flags consistent in
resetMockAuth.Line 160 clears session but leaves
isAuthenticatedunchanged. This can leave guards in an inconsistent state (isAuthenticated === truewithsession === null).🔧 Proposed fix
const resetMockAuth = useCallback(() => { resetMockAuthStorage() setSession(null) + setIsAuthenticated(false) }, [])🤖 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/contexts/AuthContext.tsx` around lines 160 - 163, The resetMockAuth function clears storage and session but does not update the authentication flag, leaving isAuthenticated inconsistent; update resetMockAuth (which calls resetMockAuthStorage and setSession) to also call setIsAuthenticated(false) (and clear any related user/token state if present) so that isAuthenticated and session remain consistent after reset.frontend/src/components/auth/OnboardingStepGuard.tsx-27-29 (1)
27-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid blank-screen fallback for unauthenticated users.
At Line 27, returning
nullcan strand users on an empty page when auth resolves to unauthenticated. Redirect to/logininstead for deterministic behavior.Suggested fix
- if (!session) { - return null - } + if (!session) { + return <Navigate to="/login" replace /> + }🤖 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/auth/OnboardingStepGuard.tsx` around lines 27 - 29, The component's session check in OnboardingStepGuard currently returns null when no session, causing a blank screen; replace that fallback with a deterministic redirect to the login page. Update the check in the OnboardingStepGuard component (where `if (!session) { return null }` appears) to trigger a client-side redirect to "/login" (e.g., using your router's replace/push or a <Navigate/> equivalent) and return null only after initiating the redirect to avoid rendering a blank page; ensure you import and use the app's routing helper consistently with the rest of the app.README.md-287-287 (1)
287-287:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix TOC anchor drift after heading rename.
The section title changed, but the existing TOC link still points to the old anchor text, so in-page navigation will break.
Suggested fix
-## 後台 API 與欄位設定（OAuth 單店） +## 多店後台 API 與欄位設定（OAuth 單店）or update the TOC entry to the new anchor generated from this heading.
🤖 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 `@README.md` at line 287, TOC link pointing to the old anchor no longer matches the renamed heading "## 後台 API 與欄位設定（OAuth 單店）"; update the corresponding entry in the table of contents to the new anchor generated from this heading (convert to lowercase, remove/normalize punctuation and full-width characters, replace spaces with hyphens) so the in-page link matches the heading text and navigation works; locate the TOC entry referencing the old anchor and replace it with the new anchor derived from "後台 API 與欄位設定（OAuth 單店）".backend/app/models/store.py-30-32 (1)
30-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winResolve Ruff RUF003 ambiguous Unicode punctuation in comments.
These comment characters are flagged and can cause lint noise/failures in stricter CI.
Suggested fix
- # 管理者指派的店主 Gmail（小寫）；首次登入時用來認領 store + # 管理者指派的店主 Gmail (小寫); 首次登入時用來認領 store @@ - # 首次登入時由 Supabase auth user id 綁定；未綁定前為 NULL（partial-unique 由 DB 端建立） + # 首次登入時由 Supabase auth user id 綁定; 未綁定前為 NULL (partial-unique 由 DB 端建立)🤖 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 `@backend/app/models/store.py` around lines 30 - 32, The comments containing Chinese text in the owner_email field definition use full-width Unicode punctuation characters (such as （）for parentheses and ；for semicolon) which trigger the Ruff RUF003 linting rule for ambiguous Unicode punctuation. Replace all full-width punctuation marks in these comments with their standard ASCII equivalents: replace （ with (, replace ） with ), and replace ； with ; to resolve the linting issue.Source: Linters/SAST tools
backend/alembic/versions/f1a2b3c4d5e6_add_store_line_credentials.py-4-4 (1)
4-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix migration header revision mismatch.
Revises:in the docstring does not matchdown_revision, which can mislead manual migration troubleshooting.Suggested fix
-Revises: e2f1a4b5c6d7 +Revises: a9f3c2d1e4b7🤖 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 `@backend/alembic/versions/f1a2b3c4d5e6_add_store_line_credentials.py` at line 4, The migration header's "Revises:" line must match the Alembic down_revision value to avoid confusion; open the migration module (f1a2b3c4d5e6_add_store_line_credentials.py), locate the module docstring "Revises:" entry and the down_revision variable, and update the "Revises:" text to exactly match the down_revision identifier (and ensure the top-level revision value is correct if needed) so both identifiers are consistent.backend/scripts/provision_stores.py-52-102 (1)
52-102:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlways dispose the engine, including early-failure paths.
Current early returns inside the loop skip cleanup at the end of the function, so engine disposal is not guaranteed.
💡 Suggested fix
engine = create_async_engine( db_url, poolclass=NullPool, pool_pre_ping=True, connect_args=asyncpg_connect_args_for_url(db_url), ) session_factory = async_sessionmaker(engine, expire_on_commit=False) - - async with session_factory() as db: - for i, entry in enumerate(payload.stores, start=1): - try: - slug = await _resolve_slug(entry) - except ValueError as e: - print(f"[{i}] {entry.name}: ERROR — {e}", file=sys.stderr) - return 1 + try: + async with session_factory() as db: + for i, entry in enumerate(payload.stores, start=1): + try: + slug = await _resolve_slug(entry) + except ValueError as e: + print(f"[{i}] {entry.name}: ERROR — {e}", file=sys.stderr) + return 1 @@ - if not dry_run: - await db.commit() - print("Done. Owners can log in with owner_email (Google) to claim the store.") - - await engine.dispose() + if not dry_run: + await db.commit() + print("Done. Owners can log in with owner_email (Google) to claim the store.") + finally: + await engine.dispose() return 0🤖 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 `@backend/scripts/provision_stores.py` around lines 52 - 102, The engine disposal is skipped by early returns inside the loop; wrap the main work so engine.dispose() always runs (e.g., create a variable exit_code=0, move the async with session_factory() block into a try/finally where the finally does await engine.dispose(), and replace the in-loop return 1 occurrences with setting exit_code=1 and breaking out of the loop), keeping the commit/rollback logic around db and preserving calls to _resolve_slug and upsert_store_from_provision; after the try/finally return exit_code.backend/tests/test_store_provision.py-18-31 (1)
18-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the missing negative assertion for empty
stores.This test name says “requires stores” but only checks a valid payload. It should assert validation failure for
{"stores": []}.💡 Suggested fix
import pytest +from pydantic import ValidationError @@ def test_provision_file_requires_stores(): - payload = StoreProvisionFile.model_validate( - { - "stores": [ - { - "name": "A", - "owner_email": "a@b.com", - "line_channel_access_token": "t", - "line_channel_secret": "s", - } - ] - } - ) - assert len(payload.stores) == 1 + with pytest.raises(ValidationError): + StoreProvisionFile.model_validate({"stores": []})🤖 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 `@backend/tests/test_store_provision.py` around lines 18 - 31, Update test_provision_file_requires_stores to include a negative assertion that an empty "stores" list fails validation: call StoreProvisionFile.model_validate({"stores": []}) inside a pytest.raises(ValidationError) (import ValidationError from pydantic) and optionally assert the raised error mentions "stores" or "must contain at least one" to ensure the validator on StoreProvisionFile enforces non-empty stores.backend/tests/test_multi_store_line.py-20-21 (1)
20-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert HTTP status/detail in missing-destination test.
The current assertion passes for any
HTTPException. Please assertstatus_code == 400(and ideallydetail) to lock behavior.Suggested fix
def test_parse_webhook_destination_missing(): from fastapi import HTTPException - with pytest.raises(HTTPException): + with pytest.raises(HTTPException) as exc_info: parse_webhook_destination("{}") + assert exc_info.value.status_code == 400 + assert exc_info.value.detail == "Missing destination in webhook body"🤖 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 `@backend/tests/test_multi_store_line.py` around lines 20 - 21, Update the test that calls parse_webhook_destination("{}") so it doesn't accept any HTTPException: catch the exception and assert its status_code is 400 (and assert the detail message matches the expected missing-destination text). Locate the assertion around parse_webhook_destination in test_multi_store_line.py, capture the raised HTTPException from pytest.raises and add assertions for exc.value.status_code == 400 and exc.value.detail == "<expected detail string>" (replace with the actual expected detail).backend/app/services/user_service.py-36-37 (1)
36-37:⚠️ Potential issue | 🟡 MinorAlign
get_user_by_line_uidreturn annotation with repositoryNonecase
repo_get_user_by_line_uid()usesscalar_one_or_none()and is typed asOptional[Customer](withUser = Customer), and callers already handleif not user:—so the service signature should also return an optional type.Suggested fix
-async def get_user_by_line_uid(db: AsyncSession, line_uid: str, store_id: int) -> User: +async def get_user_by_line_uid( + db: AsyncSession, line_uid: str, store_id: int +) -> Optional[User]: return await repo_get_user_by_line_uid(db, line_uid, store_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 `@backend/app/services/user_service.py` around lines 36 - 37, The service function get_user_by_line_uid currently declares a non-optional User return but calls repo_get_user_by_line_uid which returns Optional[Customer] (User = Customer); update get_user_by_line_uid's return annotation to Optional[User] (or Optional[Customer]) and import Optional from typing so the signature reflects the repository's scalar_one_or_none() behavior and matches callers that check "if not user:".
🧹 Nitpick comments (2)
frontend/package.json (1)
32-33: Align React 19 major witheslint-plugin-react-hooksmajor
frontend/package.jsonusesreact/react-dom^19.0.0andfrontend/package-lock.jsonresolves toreact@19.2.6/react-dom@19.2.6, whileeslint-plugin-react-hooksresolves to5.2.0(^5.0.0). Bumpeslint-plugin-react-hooksto v6+ to pick up the newer hook lint behavior (e.g.,React.useEffecthandling) and updated preset expectations (also applies to lines 46-47).🤖 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 32 - 33, Update the eslint-plugin-react-hooks dependency in frontend/package.json to a v6+ range to align with React 19 (react and react-dom are ^19.0.0); specifically change the "eslint-plugin-react-hooks" version to "^6.0.0" (and likewise update any duplicate entries on the other dependency lines referenced) and then run npm install to regenerate frontend/package-lock.json so the lockfile resolves the plugin to v6+.backend/tests/test_models_pr_changes.py (1)
274-283: ⚡ Quick winInclude
owner_emailin the required Store column assertions.This PR relies on owner-based binding; the model includes
owner_email, but this structural test doesn’t assert it.Suggested fix
for c in ( "id", "name", "slug", "line_channel_access_token", "line_channel_secret", "timezone", "active", + "owner_email", "owner_auth_user_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 `@backend/tests/test_models_pr_changes.py` around lines 274 - 283, The test iterating over required Store columns is missing the "owner_email" field; update the tuple in backend/tests/test_models_pr_changes.py (the for c in ( ... ) block) to include "owner_email" alongside "owner_auth_user_id" so the structural assertion covers the owner_email column used by the model.
Comment on lines
+102
to
+103
| status_code=status.HTTP_503_FORBIDDEN, | ||
| detail="Store is inactive", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from fastapi import status
print("HTTP_403_FORBIDDEN:", hasattr(status, "HTTP_403_FORBIDDEN"))
print("HTTP_503_FORBIDDEN:", hasattr(status, "HTTP_503_FORBIDDEN"))
PYRepository: chennjustin/Flower_Order_Bot
Length of output: 192
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the reported code region
sed -n '80,140p' backend/app/repositories/store_repository.py
# See how `status` is imported/defined in this file
rg -n "from fastapi import status|import status|HTTP_503_FORBIDDEN|HTTP_403_FORBIDDEN" backend/app/repositories/store_repository.py
# Check whether `HTTP_503_FORBIDDEN` appears anywhere else in the repo
rg -n "HTTP_503_FORBIDDEN" .Repository: chennjustin/Flower_Order_Bot
Length of output: 2249
🌐 Web query:
FastAPI status constant HTTP_503_FORBIDDEN exists?
💡 Result:
No, the constant HTTP_503_FORBIDDEN does not exist in FastAPI [1][2]. FastAPI provides HTTP status code constants through its status module (which re-exports them from Starlette) [3][4]. The correct constant for a 503 status code is HTTP_503_SERVICE_UNAVAILABLE [1][5]. Similarly, the 403 status code is represented by the constant HTTP_403_FORBIDDEN [1][5]. You cannot combine these labels, as they refer to distinct HTTP status codes (503 for "Service Unavailable" and 403 for "Forbidden") [5].
Citations:
- 1: https://fastapi.tiangolo.com/reference/status/
- 2: https://fastapi.tiangolo.com/uk/reference/status/
- 3: https://github.com/tiangolo/fastapi/blob/master/docs/en/docs/reference/status.md
- 4: https://fastapi.tiangolo.com/tutorial/response-status-code/
- 5: https://github.com/encode/starlette/blob/5dd8ff1377fe16ec8adf610b23c5858c42ce4f66/starlette/status.py
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check imports + context around the status usage
sed -n '1,80p' backend/app/repositories/store_repository.py
sed -n '90,120p' backend/app/repositories/store_repository.py
# Confirm the constant is only used here (and capture exact import of `status`)
rg -n "HTTP_503_FORBIDDEN|HTTP_403_FORBIDDEN|HTTP_503_SERVICE_UNAVAILABLE|HTTP_403_FORBIDDEN|from fastapi import status|import status" backend/app/repositories/store_repository.py
# Repo-wide occurrences (small)
rg -n "HTTP_503_FORBIDDEN" .Repository: chennjustin/Flower_Order_Bot
Length of output: 4155
Fix invalid FastAPI/Starlette status constant for inactive stores
File: backend/app/repositories/store_repository.py (lines 102-103)
The branch for inactive stores uses status.HTTP_503_FORBIDDEN, which is not a valid FastAPI/Starlette status constant and will raise at runtime when reached.
status_code=status.HTTP_503_FORBIDDEN,
detail="Store is inactive",Suggested fix
- status_code=status.HTTP_503_FORBIDDEN,
+ status_code=status.HTTP_403_FORBIDDEN,🤖 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 `@backend/app/repositories/store_repository.py` around lines 102 - 103, Replace
the invalid FastAPI/Starlette constant status.HTTP_503_FORBIDDEN in the "Store
is inactive" branch inside backend/app/repositories/store_repository.py with the
correct constant status.HTTP_503_SERVICE_UNAVAILABLE (or use
status.HTTP_403_FORBIDDEN if you intend a permission denial) so the raised
HTTPException uses a valid status code; locate the branch checking for inactive
stores (where the message "Store is inactive" is used) and update the
status_code to status.HTTP_503_SERVICE_UNAVAILABLE.
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
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.
This pull request introduces major improvements to multi-store (多店家) support, per-store LINE channel configuration, and secure OAuth-based authentication for the backend. The documentation (
README.md) has been extensively updated to guide users through new workflows for store provisioning, authentication, and per-store LINE webhook handling. Several backend modules, environment variables, and migration scripts have been added or updated to support these changes.Key changes:
Multi-store & LINE channel configuration
line_channel_access_token,line_channel_secret) to thestoretable, with a migration (f1a2b3c4d5e6). Storeslugnow holds the LINE webhookdestination(channel user id). This enables each store to have its own LINE Messaging API configuration.provision_stores.pyscript andMakefiletarget to automatically create/update stores from a JSON config, query LINE for channel info, and write credentials to the database. Extensive documentation added inREADME.md. [1] [2].env.examplewith new variables for per-store and fallback LINE credentials, Supabase Auth, and connection URLs for various tools.OAuth authentication & store access control
get_current_storedependency now binds the logged-in Google account to a unique store (1:1), auto-claiming on first login, and restricts API access to the bound store only. [1] [2] [3]X-Store-Idheader in frontend/backend communication; store context is now derived from the authenticated user. [1] [2]Developer experience & infrastructure
Makefilewith new targets for migration and provisioning, ensuring all commands use the correct Python environment and drivers.Codebase structure & maintainability
line_client.pymodule to encapsulate per-store LINE API client logic, falling back to global credentials if needed.These changes significantly improve the project's support for multiple stores, security, and maintainability, while providing clear developer workflows for provisioning and managing store credentials.
Summary by CodeRabbit
New Features
Documentation
Tests