Skip to content

chore(release): staging to production - 2026.01.21 - #873

Merged
maxtechera merged 16 commits into
productionfrom
staging
Jan 21, 2026
Merged

chore(release): staging to production - 2026.01.21#873
maxtechera merged 16 commits into
productionfrom
staging

Conversation

@github-actions

@github-actions github-actions Bot commented Jan 21, 2026

Copy link
Copy Markdown

🚀 Release: Staging to Production

Release Date: 2026-01-21

Changes in this release

  • fix(AGENT-405): add /sidekick-studio prefix to agentflow URL after save (5d93f7c)
  • fix(AGENT-639): use workspaceId filter in queryVectorStore (c26810d)
  • fix(AGENT-626): redirect to Auth0 login instead of non-existent /login route (6ca5379)
  • fix(AGENT-617): forward credential props in recursive NodeInputHandler tab calls (de1c222)
  • fix(SUPPORT-12): add enforceAbility middleware and fix response format (f6c2bbc)
  • fix(SUPPORT-12): add multi-tenancy authorization to credential refresh (72d2704)
  • fix(SUPPORT-12): restore AAI credential refresh routes lost in Flowise merge (34d53a1)
  • fix(AGENT-612): fix agent avatar images in canvas chat (56edb2e)
  • feat(AGENT-573): use default chatflow for chat navigation (2c10cff)

This PR is automatically created/updated when commits are pushed to staging.
Merging this PR will trigger the release workflow to create a new GitHub release.

diecoscai and others added 8 commits December 10, 2025 18:00
Update AppDrawer chat link to navigate to user's default chatflow
when available, falling back to /chat if not set.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add fallback handling for agent avatar images that fail to load from
the API. When the node-icon API endpoint returns an error (404 or other),
the component now gracefully falls back to static PNG images based on
the agent type (supervisor vs worker).

Changes:
- Import fallback images (multiagent_supervisor.png, multiagent_worker.png)
- Add useState for tracking icon source and error state
- Add onError handler to img element for graceful fallback
- Use useCallback to prevent unnecessary re-renders
…e merge

- Create aai/routes/credentials-refresh.ts with Google and Atlassian OAuth refresh routes
- Mount AAI credentials router before Flowise router to intercept refresh requests
- Add invalid_grant error handling with user-friendly re-auth message
- Return 401 instead of 500 for re-authentication required errors
- Add OAuth Token Refresh documentation to AUTHORIZATION.md

Routes:
- POST /credentials/refresh-token (Google OAuth)
- POST /credentials/refresh-atlassian-token (Atlassian OAuth)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Security fix addressing PR review feedback:
- Add workspaceId validation in controllers before calling service
- Add workspaceId filter to credential queries (prevents cross-tenant access)
- Use InternalFlowiseError for consistent error handling
- Remove Spanish comments and commented code
- Keep REAUTH_REQUIRED error handling for 401 responses

Prevents cross-tenant authorization bypass where attacker could refresh
another organization's credentials by guessing UUID.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PR review fixes:
- Add enforceAbility('Credential') middleware for organization isolation
- Fix response format to use direct res.json(apiResponse) for consistency

Routes now use both middlewares per .claude/rules/api-routes.md:
1. enforceAbility('Credential') - organization isolation
2. checkPermission('credentials:update') - permission check

Response format now matches other credential endpoints.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…r tab calls

Addresses PR #824 review feedback from Max Techera. The original fix added
credential state management to NodeInputHandler.jsx, but didn't handle the
recursive rendering scenario when GoogleDrivePicker is inside tabs.

Changes:
- Accept credential props (selectedCredential, selectedCredentialData,
  handleCredentialDataChange) in component signature
- Use props with fallback to local state for nested component support
- Forward credential props in recursive tab rendering call
- Add PropTypes for new props
- Notify parent via callback when credential data changes

This ensures GoogleDrivePicker works correctly in both direct usage and
when nested inside tabs in Canvas/Chatflow views.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…n route

- Change client.js 401 handler to always redirect to /api/auth/login
- Fix account/index.jsx to use Auth0 login path
- Add logging for 401 responses in getCachedSession for debugging

Fixes critical issue where unauthenticated users saw 404 on /login page
instead of being redirected to Auth0 authentication.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…-404-redirect

fix(AGENT-626): redirect to Auth0 login instead of non-existent /login route
@vercel

vercel Bot commented Jan 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
answerai-docs Building Building Preview Jan 21, 2026 4:39pm
the-answerai Building Building Preview Jan 21, 2026 4:39pm

Request Review

…nputhandler-credential-props

fix(AGENT-617): forward credential props in recursive NodeInputHandler tab calls
@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

Pull Request Review: Staging to Production Release (2026.01.21)

Overview

This PR contains two bug fixes ready for production deployment:

  1. AGENT-626: Auth0 login redirect fix
  2. AGENT-617: Credential prop forwarding in NodeInputHandler

✅ Strengths

1. Authentication Fix (AGENT-626)

  • Correctly addresses critical UX issue where users were hitting 404 on non-existent /login route
  • Consistent redirect behavior across all 401 scenarios to Auth0's /api/auth/login
  • Proper cleanup removed conditional logic that was causing inconsistency

2. Credential Prop Forwarding (AGENT-617)

  • Solves recursive rendering issue for GoogleDrivePicker in tabs
  • Backward compatible with proper fallback to local state when props not provided
  • Well-documented with clear inline comments explaining the purpose

3. Logging Improvements

  • Added helpful console.warn messages for 401 and unexpected status codes in getCachedSession.ts
  • Aids debugging without breaking functionality

🔍 Code Quality Assessment

AGENT-626: Auth0 Login Redirect

Files Changed:

  • packages/ui/src/api/client.js (lines 47-50)
  • packages/ui/src/views/account/index.jsx (line 111)
  • packages-answers/ui/src/getCachedSession.ts (lines 114-118)

Analysis:
✅ Correct pattern - All redirects now use /api/auth/login
✅ Removed dead code - Eliminated fallback to non-existent /login route
✅ Consistent with codebase - Matches patterns in other files (verified via grep)
✅ No security concerns - Auth redirects handled appropriately

AGENT-617: NodeInputHandler Credential Props

File Changed: packages/ui/src/views/canvas/NodeInputHandler.jsx

Analysis:
✅ Proper prop forwarding - Credential props now passed in recursive tab rendering
✅ Backward compatible - Uses nullish coalescing operator for safe fallback to local state
✅ Proper React patterns - Uses useCallback with correct dependencies
✅ PropTypes added - New props properly documented
✅ Parent notification - Callback properly invokes parent handler when provided

🔒 Security Review

No Security Concerns Found

✅ Authentication redirects handled safely - no open redirects
✅ No credential exposure - Props only forward internal state
✅ No XSS vulnerabilities - No dynamic HTML rendering
✅ No injection risks - No database queries or API calls modified
✅ Proper Auth0 integration - Uses documented Auth0 patterns

Verification:

  • Checked for open redirect vulnerabilities - none found (redirects to hardcoded /api/auth/login)
  • Credential data remains client-side only
  • No changes to authorization middleware or multi-tenancy filters

⚡ Performance Review

✅ No performance impact - Changes are straightforward redirects and prop passing
✅ No unnecessary re-renders - useCallback properly memoized with correct deps
✅ No new network calls - Only logging additions (warning level, minimal cost)

🧪 Testing Considerations

AGENT-626 (Auth Fix):

  • ✅ Should be validated by existing E2E auth tests
  • ⚠️ Recommendation: Verify E2E tests cover 401 redirect scenario explicitly

AGENT-617 (Credential Props):

  • ⚠️ Missing tests for recursive tab rendering with GoogleDrivePicker
  • ⚠️ Recommendation: Add unit test for prop forwarding behavior

📋 Best Practices Adherence

✅ Follows CLAUDE.md conventions
✅ Commit messages use proper conventional format (fix(AGENT-XXX))
✅ Linear ticket IDs included in all commits
✅ Co-authored by Claude properly attributed
✅ Matches existing auth redirect patterns across codebase
✅ Consistent with React prop patterns
✅ PR correctly targets production from staging (release workflow)

⚠️ Minor Concerns (Non-blocking)

  1. Console Warnings in Production

    • getCachedSession.ts now includes console.warn calls
    • Impact: Low - These are warnings, not errors
    • Recommendation: Consider using a structured logging service (DataDog, Sentry) for production
  2. Test Coverage Gap

    • NodeInputHandler changes would benefit from unit tests to prevent regression

🎯 Deployment Readiness

Pre-Deployment Checklist:

  • ✅ Code quality: PASS - Clean, well-structured fixes
  • ✅ Security: PASS - No vulnerabilities introduced
  • ✅ Performance: PASS - No negative impact
  • ⚠️ Tests: PARTIAL - Existing tests should cover AGENT-626, but AGENT-617 lacks specific tests
  • ✅ Documentation: PASS - Commit messages clear, inline comments added
  • ✅ Breaking changes: NONE - Backward compatible

Recommended Actions Before Merge:

  1. Run full E2E test suite: pnpm test:e2e
  2. Verify auth flow in staging
  3. Monitor after deployment for getCachedSession warnings

💡 Recommendations for Future Work

  1. Add integration test for GoogleDrivePicker credential prop forwarding
  2. Consider structured logging instead of console.warn for production diagnostics
  3. Document Auth0 redirect pattern in a shared constant

✅ Final Verdict

APPROVED ✅ - This PR is ready for production deployment.

Both fixes address real issues without introducing new risks. The changes are:

  • Well-scoped to specific bugs
  • Backward compatible
  • Follow repository conventions
  • No security concerns
  • Low risk for production

The minor testing gaps are acceptable for a bug fix release, but should be addressed in follow-up work.


Reviewed by: Claude Code Agent
Review Date: 2026-01-21
PR Target: staging → production

Changed queryVectorStore to filter by workspaceId instead of
userId+organizationId for consistency with other document store methods.
…ore-workspace-filter

fix(AGENT-639): use workspaceId filter in queryVectorStore
…ck-document-store-chunks-missing

fix(SUPPORT-12): Restore AAI credential refresh routes lost in Flowise merge
@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

🔍 Pull Request Review

This release PR contains three targeted bug fixes. Overall, the changes are solid and follow repository conventions. Here's my detailed analysis:


✅ Strengths

1. AGENT-639: workspaceId Filter Fix

Files: packages/server/src/controllers/documentstore/index.ts, packages/server/src/services/documentstore/index.ts

What changed:

  • Replaced userId + organizationId filtering with workspaceId in queryVectorStore
  • Added proper validation for workspaceId existence before querying
  • Improved error handling with correct HTTP status codes

Why this is good:

  • ✅ Aligns with the DocumentStore entity schema (line 49 shows workspaceId as required field)
  • ✅ Simplified signature: queryVectorStore(data, workspaceId) instead of (data, userId, organizationId)
  • ✅ Proper use of InternalFlowiseError with StatusCodes.PRECONDITION_FAILED for validation
  • ✅ Changed status code from INTERNAL_SERVER_ERROR to NOT_FOUND when document store doesn't exist (more semantically correct)

2. AGENT-626: Auth0 Login Redirect Fix 🔐

Files: packages/ui/src/api/client.js, packages/ui/src/views/account/index.jsx

What changed:

  • All authentication redirects now use /api/auth/login (Auth0)
  • Removed conditional logic for /login vs Auth0

Why this is good:

  • ✅ Consistent authentication flow across the application
  • ✅ Eliminates potential 404 errors from non-existent /login route
  • ✅ Cleaner code (removed unnecessary conditional)

3. AGENT-617: Credential Props Forwarding 📋

File: packages/ui/src/views/canvas/NodeInputHandler.jsx

What changed:

  • Added credential-related props to component signature
  • Props are forwarded in recursive NodeInputHandler calls (particularly in tab panels)
  • Uses prop values when available, falls back to local state otherwise

Why this is good:

  • ✅ Fixes Google Drive/Gmail picker credential issues in nested/tabbed components
  • ✅ Backward compatible design (props are optional, with fallback to local state)
  • ✅ Proper React patterns (uses useCallback with dependency array)
  • ✅ Clear documentation in comments

4. Session Handling Improvement 🔧

File: packages-answers/ui/src/getCachedSession.ts

What changed:

  • Added graceful handling for 401 responses from /auth/me
  • Added logging for unexpected status codes

Why this is good:

  • ✅ Prevents silent failures during token expiration
  • ✅ Better observability with targeted warning messages
  • ✅ Doesn't break session enrichment flow

@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

🔍 Security & Multi-Tenancy Analysis

✅ Security Checklist

  • Route protection: queryVectorStore has checkPermission('documentStores:view') middleware (verified in routes)
  • Input validation: Added workspaceId validation in controller
  • Error handling: Proper use of InternalFlowiseError with appropriate status codes
  • No credential leaks: Credential handling follows established patterns

✅ Multi-Tenancy Compliance

The workspaceId change is exactly the right approach for multi-tenancy:

// OLD: Required separate filtering by userId + organizationId
where: { id: data.storeId, userId, organizationId }

// NEW: Single workspace filter (workspace already scopes to org + user)
where: { id: data.storeId, workspaceId }

This is more aligned with the repository's workspace-based isolation model.


💡 Suggestions (Optional Improvements)

1. Consider Adding Metrics (Low Priority)

The insertIntoVectorStore function has metrics tracking (lines 510-517), but queryVectorStore doesn't. Consider adding similar metrics for consistency.

2. Error Message Enhancement (Nitpick)

In queryVectorStore controller, consider including the storeId in error messages for better debugging when available.


🧪 Testing Recommendations

Before merging to production:

  1. Test AGENT-639:

    • Verify vector store queries work correctly with workspaceId filtering
    • Test with multiple workspaces to ensure isolation
    • Verify 404 response for non-existent document stores
  2. Test AGENT-626:

    • Test 401 redirect flows in both api/client.js and account settings
    • Verify Auth0 login completes successfully
    • Test token expiration handling
  3. Test AGENT-617:

    • Test Google Drive/Gmail credential pickers in nested tabs
    • Verify credential data persists across tab switches
    • Test both with and without existing credentials

@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

📊 Overall Assessment

Category Rating Notes
Code Quality ⭐⭐⭐⭐⭐ Clean, follows conventions
Security ⭐⭐⭐⭐⭐ Proper validation and auth checks
Multi-Tenancy ⭐⭐⭐⭐⭐ Correctly uses workspaceId isolation
Testing ⭐⭐⭐⭐ Good changes, needs E2E validation
Performance ⭐⭐⭐⭐⭐ No performance concerns

✅ Recommendation

APPROVED - This PR is ready to merge after successful CI/CD checks and smoke testing.

The changes are well-structured, follow repository conventions from CLAUDE.md, and properly address their respective issues. The workspaceId refactor is particularly good as it simplifies the multi-tenancy model.

Great work on maintaining code quality and security standards! 🎉

…images

fix(AGENT-612): fix broken agent avatar images in canvas chat
@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

Pull Request Review: Staging to Production

Overview

This release contains 6 bug fixes. Code quality is good with proper security patterns.

Critical Issues (Must Fix)

1. Response Format Inconsistency

Location: packages/server/src/controllers/credentials/index.ts:129

Current returns raw credential, but AUTHORIZATION.md documents response as success/message/data object.

Impact: Breaking change for UI components.

2. Missing Ownership Check

Per .claude/rules/api-routes.md, controllers must use checkOwnership(). While workspaceId filtering exists, explicit check is missing.

Strengths

  • Strong multi-tenancy (workspaceId filtering everywhere)
  • All routes use enforceAbility middleware
  • RBAC checkPermission enforcement
  • Excellent documentation (134-line AUTHORIZATION.md)
  • Clean architecture (AAI routes isolated)

Medium Priority

  • Error messages reference wrong method name (updateRefreshToken vs updateAndRefreshToken)
  • Missing error.code check in refreshGoogleAccessToken.ts
  • React state won't sync if props change after mount

Test Coverage

No tests added. Should add:

  • Credential refresh error handling tests
  • Multi-tenancy authorization tests
  • UI credential forwarding tests

Security

Strengths: enforceAbility, RBAC, encryption, multi-tenancy
Concerns: Missing checkOwnership(), no rate limiting

Verdict: APPROVED WITH REQUIRED CHANGES

Fix 2 critical issues before production merge:

  1. Response format (breaking change)
  2. Add checkOwnership() check

See detailed review at packages/server/src/controllers/credentials/index.ts:129 and packages/server/src/aai/routes/credentials-refresh.ts

@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

Detailed Findings

Critical Issue 1: Response Format

Current code (line 129):

return res.json(apiResponse)

Should be:

return res.json({
    success: true,
    message: 'Token refreshed successfully',
    data: apiResponse
})

Same issue in updateAndRefreshAtlassianToken (line 151).

Critical Issue 2: Missing checkOwnership()

Add to controller after line 120:

const credential = await credentialsService.getCredentialById(req.body.credentialId)
if (req.user && !(await checkOwnership(credential, req.user, req))) {
    throw new InternalFlowiseError(StatusCodes.UNAUTHORIZED, 'Unauthorized')
}

Error Message Fix

Lines 236 and 240 in services/credentials/index.ts:
Change updateRefreshToken to updateAndRefreshToken in error messages.

Test Recommendations

Create packages/server/test/api/credentials-refresh.spec.ts:

  • Test invalid_grant returns 401
  • Test workspace isolation
  • Test response format
  • Test checkOwnership enforcement

@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

Additional Context

Why These Changes Are Important

Multi-Tenancy (AGENT-639, SUPPORT-12):
The workspaceId filtering prevents data leaks between organizations. All 6 fixes properly implement this pattern.

OAuth Refresh Architecture:
Smart design keeping AAI routes in aai/routes/ directory prevents Flowise merge conflicts. Router ordering (AAI before Flowise) ensures custom routes intercept requests.

Error Handling (refreshGoogleAccessToken.ts):
New user-friendly message for invalid_grant is excellent UX. Users know exactly what to do (re-authenticate in Credentials settings) instead of seeing cryptic OAuth errors.

Security Analysis

The enforceAbility + checkPermission combination is strong:

  • enforceAbility: Validates user has Credential access
  • checkPermission: Validates credentials:update RBAC permission
  • workspaceId filtering: Enforces data isolation

Adding checkOwnership() completes the defense-in-depth strategy.

Performance Notes

Database query optimization opportunity:
Both updateAndRefreshToken (line 203) and updateAndRefreshAtlassianToken (line 250) query credentials, then query again after refresh (lines 231, 266). Consider caching initial lookup.

Documentation Quality

AUTHORIZATION.md is exceptional:

  • Complete API reference with request/response examples
  • Architecture explanation with diagrams
  • Error handling reference table
  • Code flow documentation
  • Related files reference

This will significantly reduce support burden.

@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

Review Summary

Files Reviewed: 12

  • ✅ 5 backend files (routes, controllers, services, utils)
  • ✅ 3 UI files (client, account, NodeInputHandler)
  • ✅ 2 documentation files
  • ✅ 2 config files

Issues Summary

  • 🔴 2 Critical (blocking)
  • 🟡 3 Medium priority
  • 🟢 2 Minor issues
  • ℹ️ 0 tests added (should add)

Security Score: 8/10

Strong patterns throughout. -1 for missing checkOwnership(), -1 for no rate limiting.

Code Quality Score: 8/10

Clean code, good patterns. -1 for error message inconsistency, -1 for React state sync issue.

Documentation Score: 10/10

Exceptional AUTHORIZATION.md documentation.

Action Items for Merge

Before Production:

  1. Fix response format (2 locations)
  2. Add checkOwnership() checks (2 controllers)
  3. Fix error message method names (2 locations)

After Merge:
4. Add integration tests
5. Add rate limiting
6. Optimize duplicate DB queries

Estimated Fix Time

Critical issues: ~15 minutes
Medium priority: ~30 minutes
Tests: ~2 hours

Total: ~3 hours to production-ready state

@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

🔍 PR Review: Staging to Production Release

✅ Overall Assessment

This release looks solid! The changes are well-structured, follow repository conventions, and address important bug fixes across authentication, multi-tenancy, and credential management. The PR targets production which is appropriate for a release from staging.


📊 Summary

  • 6 bug fixes addressing authentication, credentials, and multi-tenancy issues
  • 315 additions / 65 deletions across 13 files
  • New documentation for OAuth token refresh architecture
  • Security improvements with proper authorization enforcement

🎯 Code Quality & Best Practices

✅ Strengths

  1. Excellent Architecture Documentation (AUTHORIZATION.md)

    • Comprehensive OAuth token refresh documentation (134 new lines)
    • Clear separation between AAI-specific and Flowise core code
    • Well-documented router ordering strategy to prevent merge conflicts
  2. Proper Multi-Tenancy Implementation

    • queryVectorStore now correctly filters by workspaceId (AGENT-639)
    • Both credential refresh methods updated with workspace-scoped queries
    • Prevents data leakage between organizations ✅
  3. Consistent Error Handling

    • Controllers properly use InternalFlowiseError with appropriate status codes
    • Error messages follow the Error: {service}.{method} - {description} convention
    • Removed console.error statements in favor of proper error propagation
  4. Security Enhancements

    • Added enforceAbility middleware to credential refresh routes (SUPPORT-12)
    • Added checkPermission('credentials:update') for RBAC
    • Proper 401 responses for expired/revoked OAuth tokens
  5. Clean Response Format

    • Credential refresh endpoints now return clean data without wrapping in success/message objects
    • More consistent with other API endpoints

🐛 Potential Issues & Concerns

🟡 Minor Concerns

  1. PR Target: Production Branch ⚠️

    • Per CLAUDE.md: "Always create pull requests against staging branch, NOT main"
    • This PR targets production directly from staging
    • Question: Is this intentional for releases? If production is your main branch, update CLAUDE.md to clarify release PR workflow.
  2. Credential Refresh Response Format Change (Breaking?)

    • Before:
      { "success": true, "message": "...", "data": {...} }
    • After: Direct credential object
      { "id": "...", "name": "...", ... }
    • Impact: If UI components expect the wrapped format, this could break them
    • Check: Verify GoogleDrivePicker.jsx and GmailLabelPicker.jsx handle the new format
  3. Error Handling in refreshGoogleAccessToken.ts

    • Good: Properly catches invalid_grant errors
    • Concern: Uses custom error properties (code, requiresReauth) but only checks code in service
    • Line 224: Only checks error.code === 'REAUTH_REQUIRED' but doesn't check error.message.includes('re-authenticate')
    • Consider: Make error detection more robust
  4. Agent Icon Fallback Logic (AgentReasoningCard.jsx)

    • Added fallback for missing agent icons (good!)
    • Line 31: hasError state prevents retry but onError is recreated on every render due to dependencies
    • Minor: Consider using useRef instead of state for hasError to avoid re-renders

🔒 Security Analysis

✅ Security Strengths

  1. Multi-tenancy enforcement in credential refresh (workspaceId checks)
  2. Authorization middleware properly applied to sensitive routes
  3. No credential data exposure - tokens handled securely
  4. Proper 401 responses for expired credentials requiring re-authentication

🟢 No Security Concerns Identified

All changes follow security best practices from CLAUDE.md and AUTHORIZATION.md.


🚀 Performance Considerations

✅ Performance Impacts

  1. Database Queries Optimized

    • queryVectorStore now includes workspaceId in query (better indexing)
    • Credential lookups filter by workspaceId upfront (reduces result set)
  2. No N+1 Query Issues

    • All queries properly scoped
    • No additional database round-trips introduced
  3. Frontend Improvements

    • Added early bailout logging for 401 responses (getCachedSession.ts)
    • Agent icon error handling prevents infinite retry loops

🧪 Test Coverage

⚠️ Testing Concerns

  1. No test files modified - Are these changes covered by existing tests?

    • Credential refresh endpoints (new routes in credentials-refresh.ts)
    • Multi-tenancy filters in queryVectorStore
    • Error handling for OAuth token refresh failures
  2. Manual Testing Checklist:

    • Google Drive Picker token refresh with valid credentials
    • Google Drive Picker with expired/revoked credentials (should show 401)
    • Gmail Label Picker token refresh
    • Atlassian credential refresh
    • Document store query filtering by workspace
    • Agent icons with missing images (fallback rendering)

📝 Specific File Feedback

packages/server/src/aai/routes/credentials-refresh.ts

Excellent approach! Separating AAI-specific routes prevents merge conflicts with Flowise upgrades. The documentation explains the router ordering clearly.

Suggestion: Consider adding integration tests for these routes.

packages/server/src/controllers/credentials/index.ts

Clean refactoring! Properly uses InternalFlowiseError and removed console.error statements.

Minor: Lines 113-128 and 135-150 are nearly identical. Consider extracting a helper:

const validateRefreshRequest = (req: Request): string => {
    if (!req.body.credentialId) {
        throw new InternalFlowiseError(
            StatusCodes.PRECONDITION_FAILED,
            'Error: credentialsController - credentialId not provided!'
        )
    }
    const workspaceId = req.user?.activeWorkspaceId
    if (!workspaceId) {
        throw new InternalFlowiseError(
            StatusCodes.NOT_FOUND,
            'Error: credentialsController - workspace not found!'
        )
    }
    return workspaceId
}

packages/server/src/services/credentials/index.ts

Good multi-tenancy fixes. Error handling improvement for invalid_grant is well done.

Question: Line 224 checks error.code === 'REAUTH_REQUIRED' but the error is created with custom properties. Does this catch work as expected? Consider adding a test.

packages/server/src/utils/refreshGoogleAccessToken.ts

Clean error handling for OAuth failures.

Suggestion: Add user-facing error message to response for better UX:

;(customError as any).userMessage = 'Please reconnect your Google account'

packages/ui/src/api/client.js

Good fix! Consolidates to Auth0 login instead of non-existent /login route.

Verify: Does this work for both AAI and enterprise deployments?

packages/ui/src/views/canvas/NodeInputHandler.jsx

Smart fix for credential prop forwarding in recursive calls! This resolves AGENT-617.

Consideration: This adds 3 new optional props. Ensure TypeScript types (if applicable) are updated.


✅ Checklist for Merge

Before merging to production:

  • Branch target verified - Is production the correct target for releases?
  • Manual testing complete - Especially OAuth refresh flows
  • UI components verified - GoogleDrivePicker and GmailLabelPicker handle new response format
  • Error messages tested - Expired credentials show proper user-facing messages
  • Multi-tenancy verified - Document store queries correctly filter by workspace
  • Breaking changes communicated - If response format change affects clients

🎉 Conclusion

Recommendation: ✅ Approve with minor follow-ups

This is a well-executed release with important security and multi-tenancy improvements. The code follows repository patterns, includes excellent documentation, and addresses real bugs reported in Linear tickets.

Minor follow-ups (non-blocking):

  1. Verify UI components handle new response format
  2. Add tests for credential refresh routes
  3. Consider DRY refactoring in controllers (helper function)
  4. Update CLAUDE.md if production is the intended release target

Great work on maintaining code quality while fixing critical issues! 🚀

// Google OAuth refresh (used by GoogleDrivePicker, GmailLabelPicker)
router.post(
'/refresh-token',
enforceAbility('Credential'),

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
authorization
, but is not rate-limited.

Copilot Autofix

AI 7 months ago

In general, the fix is to ensure that HTTP handlers performing expensive token/credential refresh operations are protected by a rate‑limiting middleware. In an Express application, this is commonly done with a library like express-rate-limit, configuring a sensible window and maximum request count, and inserting that middleware in the route definition before the actual controller handler.

For this specific file, the least invasive fix is to: (1) import express-rate-limit, (2) define a limiter instance dedicated to credential refresh operations (e.g., restricting each IP to a certain number of refreshes per time window), and (3) plug that limiter into both /refresh-token and /refresh-atlassian-token routes between the authentication/authorization middlewares and the controller. This avoids changing existing business logic while adding the required protection. Concretely, in packages/server/src/aai/routes/credentials-refresh.ts, add an import for express-rate-limit at the top, define const credentialsRefreshLimiter = rateLimit({ ... }) near the router creation, and then pass credentialsRefreshLimiter as an additional middleware argument in both router.post calls, before credentialsController.updateAndRefreshToken / updateAndRefreshAtlassianToken.

Suggested changeset 1
packages/server/src/aai/routes/credentials-refresh.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/server/src/aai/routes/credentials-refresh.ts b/packages/server/src/aai/routes/credentials-refresh.ts
--- a/packages/server/src/aai/routes/credentials-refresh.ts
+++ b/packages/server/src/aai/routes/credentials-refresh.ts
@@ -2,7 +2,7 @@
 import credentialsController from '../../controllers/credentials'
 import enforceAbility from '../../middlewares/authentication/enforceAbility'
 import { checkPermission } from '../../enterprise/rbac/PermissionCheck'
-
+import rateLimit from 'express-rate-limit'
 /**
  * Creates AAI-specific credential refresh routes
  *
@@ -19,11 +19,19 @@
 export function createCredentialsRefreshRouter(): Router {
     const router = express.Router()
 
+    const credentialsRefreshLimiter = rateLimit({
+        windowMs: 15 * 60 * 1000, // 15 minutes
+        max: 100, // limit each IP to 100 refresh requests per windowMs
+        standardHeaders: true,
+        legacyHeaders: false,
+    })
+
     // Google OAuth refresh (used by GoogleDrivePicker, GmailLabelPicker)
     router.post(
         '/refresh-token',
         enforceAbility('Credential'),
         checkPermission('credentials:update'),
+        credentialsRefreshLimiter,
         credentialsController.updateAndRefreshToken
     )
 
@@ -32,6 +35,7 @@
         '/refresh-atlassian-token',
         enforceAbility('Credential'),
         checkPermission('credentials:update'),
+        credentialsRefreshLimiter,
         credentialsController.updateAndRefreshAtlassianToken
     )
 
EOF
@@ -2,7 +2,7 @@
import credentialsController from '../../controllers/credentials'
import enforceAbility from '../../middlewares/authentication/enforceAbility'
import { checkPermission } from '../../enterprise/rbac/PermissionCheck'

import rateLimit from 'express-rate-limit'
/**
* Creates AAI-specific credential refresh routes
*
@@ -19,11 +19,19 @@
export function createCredentialsRefreshRouter(): Router {
const router = express.Router()

const credentialsRefreshLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // limit each IP to 100 refresh requests per windowMs
standardHeaders: true,
legacyHeaders: false,
})

// Google OAuth refresh (used by GoogleDrivePicker, GmailLabelPicker)
router.post(
'/refresh-token',
enforceAbility('Credential'),
checkPermission('credentials:update'),
credentialsRefreshLimiter,
credentialsController.updateAndRefreshToken
)

@@ -32,6 +35,7 @@
'/refresh-atlassian-token',
enforceAbility('Credential'),
checkPermission('credentials:update'),
credentialsRefreshLimiter,
credentialsController.updateAndRefreshAtlassianToken
)

Copilot is powered by AI and may make mistakes. Always verify output.
// Atlassian OAuth refresh
router.post(
'/refresh-atlassian-token',
enforceAbility('Credential'),

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
authorization
, but is not rate-limited.

Copilot Autofix

AI 7 months ago

In general, this should be fixed by adding a rate‑limiting middleware to the credential refresh routes so that a single client cannot send unlimited refresh requests in a short period. For Express, a standard solution is to use the express-rate-limit package to create a limiter with an appropriate window and maximum number of requests, and then apply it to the specific routes that trigger expensive operations.

In this file, the best fix with minimal functional change is to:

  1. Import express-rate-limit.
  2. Define one (or two) rateLimit instances configured for POST refresh endpoints (e.g., a short window with a modest request cap).
  3. Add the rate‑limiting middleware into the router.post calls, alongside enforceAbility and checkPermission, so that requests are authorized and permission‑checked but also throttled.
    We should avoid changing existing behavior beyond adding rate limiting, so we’ll keep the controller functions and route paths exactly as they are and just insert the limiter middleware into the chain. We’ll implement this fully within createCredentialsRefreshRouter’s file, without assuming any project‑specific helpers.

Concretely:

  • In packages/server/src/aai/routes/credentials-refresh.ts, add an import for rateLimit from express-rate-limit.
  • Inside createCredentialsRefreshRouter, after creating router, define a limiter, for example:
const credentialsRefreshLimiter = rateLimit({
    windowMs: 15 * 60 * 1000, // 15 minutes
    max: 100,
})

(or stricter if desired).

  • Add credentialsRefreshLimiter as a middleware argument to both router.post('/refresh-token', ...) and router.post('/refresh-atlassian-token', ...), placing it before the auth middlewares or immediately after; either order is acceptable functionally, but placing it early avoids unnecessary auth work on clearly abusive traffic.

Suggested changeset 1
packages/server/src/aai/routes/credentials-refresh.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/server/src/aai/routes/credentials-refresh.ts b/packages/server/src/aai/routes/credentials-refresh.ts
--- a/packages/server/src/aai/routes/credentials-refresh.ts
+++ b/packages/server/src/aai/routes/credentials-refresh.ts
@@ -2,6 +2,7 @@
 import credentialsController from '../../controllers/credentials'
 import enforceAbility from '../../middlewares/authentication/enforceAbility'
 import { checkPermission } from '../../enterprise/rbac/PermissionCheck'
+import rateLimit from 'express-rate-limit'
 
 /**
  * Creates AAI-specific credential refresh routes
@@ -19,9 +20,15 @@
 export function createCredentialsRefreshRouter(): Router {
     const router = express.Router()
 
+    const credentialsRefreshLimiter = rateLimit({
+        windowMs: 15 * 60 * 1000, // 15 minutes
+        max: 100, // limit each IP to 100 refresh requests per window
+    })
+
     // Google OAuth refresh (used by GoogleDrivePicker, GmailLabelPicker)
     router.post(
         '/refresh-token',
+        credentialsRefreshLimiter,
         enforceAbility('Credential'),
         checkPermission('credentials:update'),
         credentialsController.updateAndRefreshToken
@@ -30,6 +34,7 @@
     // Atlassian OAuth refresh
     router.post(
         '/refresh-atlassian-token',
+        credentialsRefreshLimiter,
         enforceAbility('Credential'),
         checkPermission('credentials:update'),
         credentialsController.updateAndRefreshAtlassianToken
EOF
@@ -2,6 +2,7 @@
import credentialsController from '../../controllers/credentials'
import enforceAbility from '../../middlewares/authentication/enforceAbility'
import { checkPermission } from '../../enterprise/rbac/PermissionCheck'
import rateLimit from 'express-rate-limit'

/**
* Creates AAI-specific credential refresh routes
@@ -19,9 +20,15 @@
export function createCredentialsRefreshRouter(): Router {
const router = express.Router()

const credentialsRefreshLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // limit each IP to 100 refresh requests per window
})

// Google OAuth refresh (used by GoogleDrivePicker, GmailLabelPicker)
router.post(
'/refresh-token',
credentialsRefreshLimiter,
enforceAbility('Credential'),
checkPermission('credentials:update'),
credentialsController.updateAndRefreshToken
@@ -30,6 +34,7 @@
// Atlassian OAuth refresh
router.post(
'/refresh-atlassian-token',
credentialsRefreshLimiter,
enforceAbility('Credential'),
checkPermission('credentials:update'),
credentialsController.updateAndRefreshAtlassianToken
Copilot is powered by AI and may make mistakes. Always verify output.
diecoscai and others added 3 commits January 21, 2026 16:07
- Fix hardcoded URL path in Canvas.jsx that was missing the /sidekick-studio prefix
- Prevents 404 errors when refreshing page after saving a new agentflow
- Enables chat history to load correctly after page refresh

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…flow-url-after-save

fix(AGENT-405): add /sidekick-studio prefix to agentflow URL after save
…-update

feat(AGENT-573): use default chatflow for chat navigation
@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

Pull Request Review - Staging to Production Release

This PR merges multiple bug fixes from staging to production. Overall code quality is good with proper security measures in place.

Strengths

Security and Multi-Tenancy (EXCELLENT)

  • Proper middleware stack: enforceAbility and checkPermission
  • Workspace isolation: All credential queries filter by workspaceId
  • Consistent error handling with InternalFlowiseError
  • Re-auth error handling with REAUTH_REQUIRED error code

Architecture Compliance

  • Follows repository 4-layer pattern (Routes, Controllers, Services, Entities)
  • AAI-specific routes isolated in aai/routes/ to avoid merge conflicts
  • Comprehensive documentation added to AUTHORIZATION.md

Error Handling

  • User-friendly error messages for OAuth invalid_grant errors
  • Proper distinction between 401 (re-auth needed) and 500 (server error)
  • Graceful fallback for agent avatar images

Issues Found

1. CRITICAL: Production Branch Target
This PR targets production branch. Per CLAUDE.md, PRs should target staging, NOT main.

Resolution needed: This appears to be an automated release PR (staging to production). Verify this is intentional and follows your release process.

2. Minor: Inconsistent Error Message Format
In refreshGoogleAccessToken.ts, error checking logic could use getErrorMessage() utility for consistency.

3. Code Quality: Hardcoded Fallback Logic
In AgentReasoningCard.jsx, fallback logic assumes binary agent types. Consider a mapping object for better maintainability.

Security Review

No security vulnerabilities found:

  • Authorization: All routes properly protected
  • Input Validation: Credential IDs and workspace IDs validated
  • Data Leaks: No sensitive data exposed in error messages
  • SQL Injection: Using parameterized queries (TypeORM)
  • XSS: Markdown rendering uses trusted library

OAuth Token Security:

  • Refresh tokens never exposed in responses
  • Access tokens properly encrypted in database
  • Re-authentication required for expired tokens

Performance Considerations

Good practices:

  • Database queries properly filtered by workspace (indexed field)
  • Credential data fetched only when needed
  • React components use useCallback for memoization

Potential optimization: The recursive NodeInputHandler component could benefit from React.memo() wrapper.

Test Coverage

Missing tests:

  1. Credential refresh endpoints - No E2E tests for OAuth refresh flows
  2. Multi-tenancy authorization - Should verify cross-tenant access is blocked
  3. Error scenarios - Test invalid_grant and re-auth flows

Code Quality Suggestions

  1. PropTypes Documentation: NodeInputHandler has comprehensive PropTypes. Consider adding JSDoc comments for better IDE support.

  2. Magic Strings: Canvas.jsx uses hardcoded path. Consider extracting to constant for maintainability.

  3. Type Safety: Several files use any type. Define proper interfaces for better type safety.

Checklist Compliance

Per .claude/rules/api-routes.md:

  • All routes use enforceAbility middleware
  • Controllers use checkOwnership() via workspaceId filter
  • Database operations filter by organizationId/workspaceId
  • Errors use InternalFlowiseError with proper status codes
  • Routes registered in src/index.ts

Recommendations

High Priority:

  1. Security is solid - Ready for production from security standpoint
  2. Verify branch target - Confirm this is your intended release process
  3. Add tests - Cover OAuth refresh scenarios before production

Medium Priority:
4. Refactor hardcoded strings - Use constants for paths
5. Document exceptions - Update CLAUDE.md if staging to production PRs are standard
6. Add integration tests - Test credential refresh with expired tokens

Low Priority:
7. Improve type safety - Replace any types with proper interfaces
8. Add JSDoc comments - Especially for complex prop forwarding

Final Verdict

Overall Assessment: APPROVED with minor recommendations

Deployment Risk: LOW

This is a well-implemented bug fix release with:

  • Strong security practices
  • Proper multi-tenancy enforcement
  • Clean architecture following repository patterns
  • No breaking changes

The code follows repository conventions and is production-ready. Primary concern is ensuring the branch target aligns with your release process.

Recommendation: Safe to merge after confirming this is your standard release workflow

Review conducted following repository guidelines in CLAUDE.md, .claude/rules/api-routes.md, and .claude/rules/components.md

@claude

claude Bot commented Jan 21, 2026

Copy link
Copy Markdown

PR Review: Staging to Production Release - 2026.01.21

Overview

This is a production release containing 9 bug fixes focused on multi-tenancy authorization, credential refresh functionality, OAuth token handling, and UI improvements. Overall code quality is good with proper adherence to repository patterns.


✅ Strengths

1. Multi-Tenancy Security

All credential refresh operations now properly filter by workspaceId:

  • packages/server/src/services/credentials/index.ts:203-206 - Google OAuth refresh
  • packages/server/src/services/credentials/index.ts:250-256 - Atlassian OAuth refresh
  • packages/server/src/services/documentstore/index.ts:1666-1668 - Document store queries

This prevents cross-workspace data leaks. ✅

2. Authorization & Middleware

  • Routes properly use enforceAbility('Credential') middleware (credentials-refresh.ts:25-26)
  • Additional checkPermission('credentials:update') for RBAC
  • Controllers validate workspaceId before service calls (controllers/credentials/index.ts:121-127)

Follows repository patterns from CLAUDE.md and .claude/rules/api-routes.md. ✅

3. Error Handling

  • Consistent use of InternalFlowiseError with proper status codes
  • User-friendly error messages for OAuth failures (refreshGoogleAccessToken.ts:37-43)
  • Special handling for invalid_grant errors with re-auth guidance

4. Documentation

Excellent new documentation in packages/server/AUTHORIZATION.md:

  • Clear architecture explanation (router ordering)
  • Code flow diagrams
  • Request/response examples
  • Related files reference

⚠️ Issues & Concerns

CRITICAL: Response Format Breaking Change

Location: packages/server/src/controllers/credentials/index.ts:129

Problem:

// Before (what UI expects):
return res.json({
    success: true,
    message: "Token refreshed successfully", 
    data: apiResponse
})

// After (what's returned now):
return res.json(apiResponse)  // Direct credential object

Impact: This breaks the API contract for UI components expecting the wrapped format:

  • packages/ui/src/ui-component/drive/GoogleDrivePicker.jsx
  • packages/ui/src/ui-component/gmail/GmailLabelPicker.jsx

Evidence from PR description: SUPPORT-12 claims to "fix response format", but this actually changes it from wrapped to unwrapped, which may break existing clients.

Recommendation:

// Option 1: Keep wrapped format (safer)
return res.json({
    success: true,
    message: 'Token refreshed successfully',
    data: apiResponse
})

// Option 2: If unwrapped is intentional, update UI components in same release

MEDIUM: Router Registration Order Dependency

Location: packages/server/src/routes/index.ts:107-109

Issue:

// AAI: Credential refresh routes (must be BEFORE Flowise credentials router to intercept refresh requests)
router.use('/credentials', createCredentialsRefreshRouter())
router.use('/credentials', credentialsRouter)

This relies on Express route matching order, which is fragile:

  • If someone reorders these lines, refresh endpoints break silently
  • Not enforced by TypeScript or tests
  • Comment is helpful but easy to miss

Recommendation:

  1. Add integration test verifying /credentials/refresh-token hits AAI router
  2. Consider more specific route prefix like /credentials/aai/refresh-token to eliminate ambiguity

LOW: Inconsistent Error Handling

Location: packages/server/src/services/credentials/index.ts:233-242

Issue:

catch (error: any) {
    // Check for REAUTH_REQUIRED
    if (error.code === 'REAUTH_REQUIRED' || error.message?.includes('re-authenticate')) {
        throw new InternalFlowiseError(StatusCodes.UNAUTHORIZED, ...)
    }
    // Generic fallback
    throw new InternalFlowiseError(StatusCodes.INTERNAL_SERVER_ERROR, ...)
}

The error.message?.includes('re-authenticate') is brittle (relies on string matching). The custom error code REAUTH_REQUIRED is better, but:

  • Atlassian refresh handler doesn't have similar logic (index.ts:271-275)
  • Could fail silently if Google changes error messages

Recommendation: Use only error.code === 'REAUTH_REQUIRED' and ensure all OAuth utilities set this code.


🔍 Code Quality Observations

Good Patterns

  1. Proper prop forwarding (NodeInputHandler.jsx:1036-1039)

    • Fixes credential state in recursive component renders
    • Good use of optional chaining for backward compatibility
  2. Image fallback handling (AgentReasoningCard.jsx:22-33)

    • Prevents broken images with onError callback
    • Uses agent-type-specific fallbacks
  3. Auth redirect consistency (api/client.js:47-49, account/index.jsx:111)

    • All redirects now use Auth0 (/api/auth/login)
    • Removes non-existent /login route references

Minor Issues

  1. Magic strings (Canvas.jsx:569)

    window.history.replaceState(state, null, `/sidekick-studio/v2/agentcanvas/${chatflow.id}`)

    Consider using a path builder utility or constant.

  2. Console warnings (getCachedSession.ts:114-117)
    These are helpful for debugging but consider using proper logger with levels.

  3. Double credential lookup (services/credentials/index.ts:250-256, 266-269)

    // Line 250: Find credential to verify workspace
    const existingCredential = await repository.findOneBy({ id, workspaceId })
    
    // Line 266: Find again after refresh
    const credential = await repository.findOneBy({ id, workspaceId })

    Could return existingCredential if unchanged, but current approach is safer (ensures fresh data).


🧪 Testing Recommendations

Required Tests (High Priority)

  1. Credential refresh security:

    test('should not refresh credentials from different workspace', async () => {
      const workspace1Credential = await createCredential({ workspaceId: 'ws1' })
      const workspace2User = { activeWorkspaceId: 'ws2' }
      
      const response = await request(app)
        .post('/api/v1/credentials/refresh-token')
        .send({ credentialId: workspace1Credential.id })
        .set('Authorization', `Bearer ${workspace2User.token}`)
      
      expect(response.status).toBe(404)
    })
  2. Router precedence:

    test('AAI refresh routes should intercept before Flowise routes', async () => {
      // Verify POST /credentials/refresh-token hits AAI handler
    })
  3. OAuth error handling:

    test('should return 401 with user message when Google auth expires', async () => {
      mockGoogleOAuth.refreshToken.mockRejectedValue({ 
        response: { data: { error: 'invalid_grant' } }
      })
      
      const response = await refreshToken(credentialId)
      expect(response.status).toBe(401)
      expect(response.body.message).toContain('re-authenticate')
    })

Nice to Have

  • E2E test for Google Drive Picker token refresh flow
  • Integration test for Atlassian OAuth refresh
  • Verify default chatflow navigation (AppDrawer.tsx:185)

🚨 Security Audit

PASS - All database queries filter by workspaceId
PASS - Routes use enforceAbility middleware
PASS - Controllers validate workspaceId exists before service calls
PASS - No hardcoded credentials or API keys
PASS - Error messages don't leak sensitive data
⚠️ WARNING - Response format change could break authenticated API clients (see above)


📊 Performance Considerations

  • Database queries: All use indexed fields (workspaceId, id)
  • No N+1 queries: Single lookups per operation
  • Caching: No new caching needed (credentials refreshed on demand)

Observation: Double credential lookup in Atlassian refresh (lines 250 & 266) is acceptable since this is infrequent operation.


🎯 Recommendations Summary

Before Merging (Critical):

  1. ⚠️ Verify UI components handle new credential response format (unwrapped)

    • Test GoogleDrivePicker refresh button
    • Test GmailLabelPicker refresh button
    • Or revert to wrapped format for backward compatibility
  2. 🧪 Add integration test for router ordering (AAI before Flowise)

Post-Merge (Medium Priority):

  1. 📝 Consider route prefix refactor (/credentials/aai/*) to eliminate ordering dependency
  2. 🔒 Add security tests for workspace isolation
  3. 🧹 Remove string-based error checking, use only error codes

Future Improvements:

  1. Extract path constants (/sidekick-studio/v2/agentcanvas/...)
  2. Consider unified OAuth refresh utility for all providers
  3. Add structured logging instead of console.warn

✅ Final Verdict

Recommendation: ⚠️ CONDITIONAL APPROVAL

This release contains important security fixes (multi-tenancy) and restores lost functionality (credential refresh). Code quality is good and follows repository conventions.

However, the response format change in credential controllers needs verification:

  • If UI components were updated to handle unwrapped responses → ✅ APPROVE
  • If UI still expects wrapped responses → ❌ Request changes to restore wrapped format

Action Item: Please confirm GoogleDrivePicker and GmailLabelPicker were tested with the new response format, or consider reverting controllers to return wrapped responses for backward compatibility.


📚 Adherence to Repository Standards

✅ Follows 4-layer pattern (routes → controllers → services → entities)
✅ Uses InternalFlowiseError with proper status codes
✅ Multi-tenancy filters on all queries
✅ Authentication middleware on all routes
✅ Comprehensive documentation added
✅ Conventional commit messages
✅ PR targets production (appropriate for release)

Great work overall! The architectural decision to keep AAI routes in aai/routes/ is smart for reducing merge conflicts.


Review generated using repository patterns from CLAUDE.md, packages/server/CLAUDE.md, and .claude/rules/

@maxtechera
maxtechera merged commit a055784 into production Jan 21, 2026
12 of 14 checks passed
@maxtechera
maxtechera temporarily deployed to staging - aai-unified2-flowise-moonstruck January 21, 2026 19:22 — with Render Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants