Improve overall agent friendliness and footprint#695
Conversation
Pages, components, layouts, and contexts may no longer import #api/* or auth/tokenManager directly (no-restricted-imports), mechanizing the rule already documented in AGENTS.md. The three documented OAuth-CSRF-helper call sites keep a narrow, named exception. Fixes the two existing violations this surfaced: OIDCCallback.tsx and OrgHome.tsx called #api/projects directly instead of going through a hook (new useFetchProjects), and AppLayout.tsx called tokenManager's switchOrgToken directly instead of through a hook (new useSwitchOrgToken). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CodeViewer.tsx and ApiChatMessage.tsx imported react-syntax-highlighter's full Prism build, which eagerly registers ~250 language grammars. Switch to prism-light with explicit registerLanguage calls for just the languages each component actually renders (markup/json/yaml/javascript/ typescript, and json respectively); 'text' needs no registration since the library already special-cases it as plain output. ApiChatMessage sits behind AppLayout -> CopilotDrawer -> CopilotChatWindow -> CopilotMessage, and AppLayout wraps every authenticated route, so the full Prism build was previously loaded on every session regardless of whether the copilot was opened. Lazy-load CopilotDrawer itself so none of that chain — including the syntax highlighter — loads until a user actually opens the drawer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces two new React Query hooks: Suggested Reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
This PR tightens the UI→hooks→API layering (reducing direct data/token access from UI code), and reduces the UI footprint by deferring/downsizing heavy modules (Copilot drawer + syntax highlighting).
Changes:
- Replaced direct
#api/*usage in UI pages with hook-based imperative fetchers (useFetchProjects,useFetchProjectsByOrgId). - Lazy-loaded the WIP Copilot drawer and wrapped org-token switching behind a hook (
useSwitchOrgToken). - Switched syntax highlighting to Prism “light” builds with explicit language registration to reduce bundle size.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ipaas/src/pages/OrgHome.tsx | Uses hook-based imperative project fetch instead of direct API import. |
| ipaas/src/pages/OIDCCallback.tsx | Uses hook-based project fetch to avoid direct API access. |
| ipaas/src/layouts/AppLayout.tsx | Lazy-loads Copilot drawer; routes org token switching through a hook. |
| ipaas/src/hooks/useProjects.ts | Adds useFetchProjects() imperative fetcher that shares React Query cache. |
| ipaas/src/hooks/useOrg.ts | Adds useSwitchOrgToken() wrapper around token exchange. |
| ipaas/src/components/CodeViewer.tsx | Uses prism-light and registers only supported languages. |
| ipaas/src/components/AiCopilot/ApiChatMessage.tsx | Uses prism-light and registers only JSON highlighting. |
| ipaas/eslint.config.js | Adds architectural guardrails via no-restricted-imports for UI layer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| patterns: [NO_DIRECT_API_PATTERN], | ||
| paths: [ | ||
| { | ||
| name: '../auth/tokenManager', | ||
| importNames: TOKEN_MANAGER_DATA_ACCESS_EXPORTS, | ||
| message: 'Only the OAuth CSRF helpers (generateAndSaveGitHubState, validateAndClearGitHubState, validateAndClearOIDCState, getAndClearRedirectUrl) may be imported directly here — everything else must go through a hook. See src/pages/AGENTS.md.', | ||
| }, | ||
| ], |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ipaas/eslint.config.js`:
- Around line 83-111: The UI-layer rule configuration for src/pages,
src/components, src/layouts, and src/contexts files does not include the product
API folder restrictions that are defined in the earlier rule. Add the product
folder pattern (the group with patterns for **/api/wip/*, **/api/cloud/*, and
**/api/icp/*) to the patterns array in the UI-layer no-restricted-imports rule
alongside NO_DIRECT_API_PATTERN so that UI-layer files cannot import directly
from literal product folders, maintaining consistency with the earlier
restriction.
- Around line 116-127: The tokenManager import restriction in the file override
for OIDCCallback.tsx, Project.tsx, and CreateIntegrationOptions.tsx uses a
denylist approach (TOKEN_MANAGER_DATA_ACCESS_EXPORTS) which implicitly allows
the four approved helpers and creates maintenance risk for future exports.
Replace the denylist with an explicit allowlist by changing the importNames
configuration from TOKEN_MANAGER_DATA_ACCESS_EXPORTS to an array containing only
the four approved helper function names: generateAndSaveGitHubState,
validateAndClearGitHubState, validateAndClearOIDCState, and
getAndClearRedirectUrl. This ensures only these specific functions are allowed
and any future additions to tokenManager will not be automatically permitted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29990699-0ff2-4e79-b1bc-d8ae43ccbcf8
📒 Files selected for processing (8)
ipaas/eslint.config.jsipaas/src/components/AiCopilot/ApiChatMessage.tsxipaas/src/components/CodeViewer.tsxipaas/src/hooks/useOrg.tsipaas/src/hooks/useProjects.tsipaas/src/layouts/AppLayout.tsxipaas/src/pages/OIDCCallback.tsxipaas/src/pages/OrgHome.tsx
| files: ['src/**/*.{ts,tsx}'], | ||
| ignores: ['src/api/**'], | ||
| rules: { | ||
| 'no-restricted-imports': [ | ||
| 'error', | ||
| { | ||
| patterns: [{ group: ['**/api/wip/*', '**/api/cloud/*', '**/api/icp/*'], message: "Import product API functions via the '#api/<domain>' alias, never a literal product folder — see AGENTS.md." }], | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| // UI layer (pages/components/layouts/contexts): no direct api/ access and no | ||
| // direct auth/tokenManager data access — all server state and token data | ||
| // must flow through src/hooks/. See AGENTS.md, src/pages/AGENTS.md, | ||
| // src/components/AGENTS.md. | ||
| { | ||
| files: ['src/pages/**/*.{ts,tsx}', 'src/components/**/*.{ts,tsx}', 'src/layouts/**/*.{ts,tsx}', 'src/contexts/**/*.{ts,tsx}'], | ||
| rules: { | ||
| 'no-restricted-imports': [ | ||
| 'error', | ||
| { | ||
| patterns: [ | ||
| NO_DIRECT_API_PATTERN, | ||
| { group: ['**/auth/tokenManager'], message: 'auth/tokenManager is raw token/data access — use a hook (useAuth, useOrgUuid, ...) instead. Only the documented OAuth CSRF helpers may bypass this (src/pages/AGENTS.md) — add a file-scoped override below if you add one.' }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the full eslint.config.js to understand the complete configuration structure
cat -n ipaas/eslint.config.js | head -150Repository: wso2/integration-control-plane
Length of output: 6025
🌐 Web query:
ESLint flat config overlapping files patterns rule override behavior FlatConfig
💡 Result:
In ESLint flat config, when multiple configuration objects have overlapping file patterns, ESLint resolves the final configuration by merging all matching objects in order from the top of the array to the bottom [1]. If multiple objects match a file, their configurations are combined, and the last matching configuration in the array takes precedence in the event of a conflict [1][2]. Key behaviors regarding overlapping patterns and precedence include: 1. Cascading Merge: When a file matches multiple configuration objects in the array, ESLint creates a single configuration for that file by merging properties [1]. If two objects define the same rule, the value from the object defined later in the array overrides the earlier one [1]. 2. Object Matching: A configuration object applies to a file if the file matches at least one pattern in the object's files array [1][3]. If a configuration object does not specify a files property, it applies to all files being linted [1][3]. 3. Ignores: Within a single configuration object, ignores patterns take priority over files patterns [4][3]. If a file matches both an include pattern and an ignore pattern within the same object, it is ignored [3]. 4. Extended Configs: When using extends within a configuration object, if both the base object and the extended configuration contain files, ESLint calculates the intersection (AND operation) of these patterns [5][6]. If they contain ignores, the patterns are merged (OR operation) [6]. Because the flat config system evaluates the array linearly, the order of objects in your eslint.config.js is critical for achieving the desired override behavior [1]. Configuration objects defined earlier in the array serve as the baseline, while objects defined later provide specific overrides [1]. Top results: [1][6][3][2]
Citations:
- 1: https://eslint.org/blog/2022/08/new-config-system-part-2/
- 2: https://eslint-eslint.mintlify.app/use/configuration
- 3: https://github.com/eslint/eslint/blob/f9e54f43/docs/src/use/configure/configuration-files.md
- 4: Bug: isPathIgnored does not obey files and ignores patterns eslint/eslint#19692
- 5: Is extends not meant to flatten files? It doesn't disable rules as a result eslint/eslint#20369
- 6: eslint/rfcs@6b0b4fa
Consolidate product API folder restrictions into the UI-layer rule.
The no-restricted-imports rule at line 99 overrides the rule at line 83 for files matching src/pages|components|layouts|contexts. However, line 99's patterns omit the literal product folder restriction (**/api/wip/*, **/api/cloud/*, **/api/icp/*) defined at line 89, meaning UI-layer files can import directly from those folders without triggering the rule. Add the product folder pattern to the UI-layer rule configuration to enforce the "never a literal product folder" constraint consistently across all src code.
🤖 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 `@ipaas/eslint.config.js` around lines 83 - 111, The UI-layer rule
configuration for src/pages, src/components, src/layouts, and src/contexts files
does not include the product API folder restrictions that are defined in the
earlier rule. Add the product folder pattern (the group with patterns for
**/api/wip/*, **/api/cloud/*, and **/api/icp/*) to the patterns array in the
UI-layer no-restricted-imports rule alongside NO_DIRECT_API_PATTERN so that
UI-layer files cannot import directly from literal product folders, maintaining
consistency with the earlier restriction.
Source: Coding guidelines
| files: ['src/pages/OIDCCallback.tsx', 'src/pages/Project.tsx', 'src/pages/CreateIntegrationOptions.tsx'], | ||
| rules: { | ||
| 'no-restricted-imports': [ | ||
| 'error', | ||
| { | ||
| patterns: [NO_DIRECT_API_PATTERN], | ||
| paths: [ | ||
| { | ||
| name: '../auth/tokenManager', | ||
| importNames: TOKEN_MANAGER_DATA_ACCESS_EXPORTS, | ||
| message: 'Only the OAuth CSRF helpers (generateAndSaveGitHubState, validateAndClearGitHubState, validateAndClearOIDCState, getAndClearRedirectUrl) may be imported directly here — everything else must go through a hook. See src/pages/AGENTS.md.', | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate tokenManager module =="
fd -i 'tokenManager.ts$' src
echo
echo "== Exported symbols from tokenManager (for drift check) =="
rg -nP --type=ts '^\s*export\s+(const|function|async function|class|type|interface)\s+([A-Za-z0-9_]+)' src | rg 'tokenManager\.ts'
echo
echo "== Imports of tokenManager in exception pages =="
rg -nP --type=ts 'from\s+[\"\x27].*auth/tokenManager[\"\x27]' src/pages/OIDCCallback.tsx src/pages/Project.tsx src/pages/CreateIntegrationOptions.tsx
echo
echo "== Current restricted list in eslint config =="
rg -n 'TOKEN_MANAGER_DATA_ACCESS_EXPORTS|importNames|name:\s*[\"\x27]\.\./auth/tokenManager[\"\x27]' ipaas/eslint.config.jsRepository: wso2/integration-control-plane
Length of output: 216
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Repository structure (top-level) =="
ls -la
echo
echo "== Check for src directory and ipaas directory =="
find . -maxdepth 2 -type d -name 'src' -o -name 'ipaas' 2>/dev/null | head -20
echo
echo "== List ipaas directory =="
ls -la ipaas/ 2>/dev/null || echo "ipaas directory not found"Repository: wso2/integration-control-plane
Length of output: 2299
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant lines from ipaas/eslint.config.js (110-135) =="
sed -n '110,135p' ipaas/eslint.config.js
echo
echo "== Search for TOKEN_MANAGER_DATA_ACCESS_EXPORTS definition =="
rg -n 'TOKEN_MANAGER_DATA_ACCESS_EXPORTS' ipaas/eslint.config.js
echo
echo "== Locate tokenManager module =="
find ipaas/src -name '*tokenManager*' -type f 2>/dev/null
echo
echo "== Exported symbols from tokenManager =="
rg -nP --type=ts '^\s*export\s+(const|function|async function|class|type|interface)\s+([A-Za-z0-9_]+)' ipaas/src | grep -i 'tokenmanager' || echo "No matches found"
echo
echo "== All content of tokenManager.ts =="
find ipaas/src -name '*tokenManager.ts' -type f -exec cat {} \;Repository: wso2/integration-control-plane
Length of output: 21281
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TOKEN_MANAGER_DATA_ACCESS_EXPORTS definition (lines 25-40) =="
sed -n '25,40p' ipaas/eslint.config.js
echo
echo "== Check actual imports in OIDCCallback.tsx =="
rg -n 'from.*tokenManager' ipaas/src/pages/OIDCCallback.tsx
echo
echo "== Check actual imports in Project.tsx =="
rg -n 'from.*tokenManager' ipaas/src/pages/Project.tsx
echo
echo "== Check actual imports in CreateIntegrationOptions.tsx =="
rg -n 'from.*tokenManager' ipaas/src/pages/CreateIntegrationOptions.tsxRepository: wso2/integration-control-plane
Length of output: 1100
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full TOKEN_MANAGER_DATA_ACCESS_EXPORTS definition (lines 25-50) =="
sed -n '25,50p' ipaas/eslint.config.js
echo
echo "== Count of exports in TOKEN_MANAGER_DATA_ACCESS_EXPORTS =="
sed -n '28,50p' ipaas/eslint.config.js | grep -c "'" || true
echo
echo "== All exported symbols from tokenManager for comparison =="
rg -nP '^\s*export\s+(const|function|async function)\s+([A-Za-z0-9_]+)' ipaas/src/auth/tokenManager.ts | wc -l
echo
echo "== Exported symbols not in denylist =="
rg -oP '^\s*export\s+(const|function|async function)\s+\K([A-Za-z0-9_]+)' ipaas/src/auth/tokenManager.ts > /tmp/all_exports.txt
sed -n '28,50p' ipaas/eslint.config.js | grep -oP "'\K[^']+" > /tmp/denylist.txt
echo "All exports:"
cat /tmp/all_exports.txt
echo
echo "Denylist:"
cat /tmp/denylist.txtRepository: wso2/integration-control-plane
Length of output: 1958
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Understanding ESLint no-restricted-imports behavior =="
echo "The 'importNames' array in ESLint's no-restricted-imports is a DENYLIST:"
echo "- It blocks imports of the listed names"
echo "- Unlisted names are implicitly allowed"
echo
echo "== Exported symbols NOT in the denylist (implicitly allowed) =="
comm -23 <(sort /tmp/all_exports.txt) <(grep -oP "'\K[^']+" ipaas/eslint.config.js | sort -u) | sort
echo
echo "== Verify: These 4 should be the approved helpers =="
echo "Expected approved helpers:"
echo " generateAndSaveGitHubState"
echo " validateAndClearGitHubState"
echo " validateAndClearOIDCState"
echo " getAndClearRedirectUrl"Repository: wso2/integration-control-plane
Length of output: 412
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== All exported symbols from tokenManager =="
rg -oP '^\s*export\s+(const|function|async function)\s+\K([A-Za-z0-9_]+)' ipaas/src/auth/tokenManager.ts | sort > /tmp/all_exports.txt
cat /tmp/all_exports.txt
echo
echo "== Names in the denylist =="
sed -n '28,50p' ipaas/eslint.config.js | grep -oP "'\K[^']+" | grep -v '^,$' | sort > /tmp/denylist.txt
cat /tmp/denylist.txt
echo
echo "== Exported symbols NOT in the denylist (these are allowed) =="
grep -v -f /tmp/denylist.txt /tmp/all_exports.txt || echo "(none)"
echo
echo "== These 4 should be implicitly allowed =="
echo "generateAndSaveGitHubState"
echo "validateAndClearGitHubState"
echo "validateAndClearOIDCState"
echo "getAndClearRedirectUrl"Repository: wso2/integration-control-plane
Length of output: 1316
Denylist approach to tokenManager imports in these pages creates maintenance risk; prefer an allowlist for the four approved helpers.
The importNames: TOKEN_MANAGER_DATA_ACCESS_EXPORTS denylist blocks 21 known exports, leaving the four approved helpers (generateAndSaveGitHubState, validateAndClearGitHubState, validateAndClearOIDCState, getAndClearRedirectUrl) implicitly allowed. Any new exports added to tokenManager.ts in the future will automatically be permitted in these three pages unless the denylist is explicitly maintained. Switch to an allowlist that explicitly permits only these four names to enforce the intended boundary without relying on ongoing denylist updates.
🤖 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 `@ipaas/eslint.config.js` around lines 116 - 127, The tokenManager import
restriction in the file override for OIDCCallback.tsx, Project.tsx, and
CreateIntegrationOptions.tsx uses a denylist approach
(TOKEN_MANAGER_DATA_ACCESS_EXPORTS) which implicitly allows the four approved
helpers and creates maintenance risk for future exports. Replace the denylist
with an explicit allowlist by changing the importNames configuration from
TOKEN_MANAGER_DATA_ACCESS_EXPORTS to an array containing only the four approved
helper function names: generateAndSaveGitHubState, validateAndClearGitHubState,
validateAndClearOIDCState, and getAndClearRedirectUrl. This ensures only these
specific functions are allowed and any future additions to tokenManager will not
be automatically permitted.
Source: Coding guidelines
Adds a unit test runner (none existed — only e2e Playwright, which needs a live backend and credentials) and covers the four pure-logic modules flagged in the architecture report: identifyIntegration.ts (including the componentSubType-overrides-displayType precedence rules), componentType.ts, nav.ts's scope/URL helpers and matrix-driven routing (generateMatrixRoutes/resourceUrl/sidebarItems), and runtimeConfig.ts's config.json merge/trim/fallback logic. #api/#product resolve to wip in tests (vitest.config.ts), matching tsconfig.app.json's IDE default. Tests import vitest's API explicitly (no injected globals), consistent with the rest of the codebase. Note: pnpm-lock.yaml is not yet updated for the new vitest/jsdom devDependencies — this sandbox's pnpm requires Node >=22.13 and the repo has Node 20.20.2, so `pnpm install` couldn't be run here. Run `pnpm install` and commit the resulting lockfile before relying on --frozen-lockfile (CI, Dockerfile) for installs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Flat config doesn't merge no-restricted-imports option objects across matching blocks for the same file — the last matching block's value replaces the earlier one entirely, it doesn't union the `patterns` arrays. The UI-layer block (pages/components/layouts/contexts) and the named OAuth-CSRF-exception block both redefined no-restricted-imports without the repo-wide "never a literal product folder" pattern, so those files could import src/api/wip/cloud/icp directly and lint would pass. Factor the pattern into a shared constant and include it in every block that sets the rule. Verified by reproducing the gap (an import from '../api/wip/components' in a pages/ file passed lint before this fix) and confirming it's now caught in all three affected blocks, with no false positives on the existing exception files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A denylist (importNames: TOKEN_MANAGER_DATA_ACCESS_EXPORTS) silently permits any future tokenManager.ts export by default — exactly the maintenance risk flagged in review. ESLint's no-restricted-imports has no "allow only these names" mode, so fix this structurally instead of in config: move the six pure OAuth CSRF state helpers (no token/network access) into their own module, src/auth/oauthState.ts. The UI-layer ESLint block now blocks auth/tokenManager outright for pages/components/layouts/contexts, with no per-file exception — the three documented pages (and useGitHubAuth.ts, ProtectedRoute.tsx, AuthContext.tsx) import the CSRF helpers from oauthState.ts instead, which the rule doesn't restrict. A new tokenManager.ts export is now blocked from UI code by default; it would have to be added to oauthState.ts first to become reachable there. Updated AGENTS.md and src/pages/AGENTS.md to describe the new module instead of the old exception list. Verified: a page importing a never-seen-before tokenManager export (simulating a future addition) is now rejected; the three real pages still pass with their actual oauthState imports; the full lint, the 81-test unit suite, and a scoped tsc check of every touched file are all clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Runs pnpm lint, pnpm test (Vitest), and pnpm build:check (tsc -b + a real PRODUCT=wip build) on every PR touching ipaas/** and on push to main/devant-migration. None of this needs live credentials or a reachable backend, unlike the existing E2E workflows, so it also runs on push for branch-health visibility. Mirrors e2e-pr-check.yml's pnpm/Node setup (pnpm 9, Node 20) for consistency with the one CI workflow already proven to work in this repo. Not wired into branch protection as a required check — same opt-in posture as the existing E2E checks; the comment in the workflow explains how to make it required. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Purpose
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning