Skip to content

Conversation

@Kavirubc
Copy link

@Kavirubc Kavirubc commented Dec 24, 2025

Purpose

Add ability to bulk import environment variables by pasting .env content or uploading a file, instead of adding them one by one.

Resolves #106

Goals

  • Enable users to paste .env file content into a textarea for quick import
  • Enable users to upload .env files directly
  • Parse and validate environment variables before populating editable fields
  • Merge imported variables with existing ones (replacing duplicates)

Approach

  • Created envParser.ts utility to parse .env content (handles comments, quoted values, empty lines, duplicates)
  • Created EnvBulkImportModal.tsx component with textarea and file upload button
  • Integrated "Bulk Import" button into EnvironmentVariable components on both Create Agent and Deploy pages
  • Used replace() from react-hook-form's useFieldArray to properly sync form fields

User stories

  • As a user, I want to paste my .env file content to quickly import multiple environment variables
  • As a user, I want to upload a .env file to import environment variables
  • As a user, I want imported variables to merge with existing ones without creating duplicates

Release note

Added bulk import functionality for environment variables. Users can now paste .env content or upload .env files to quickly populate environment variable fields when creating or deploying agents.

Documentation

N/A - This is a UI enhancement that follows existing patterns. The feature is self-explanatory with clear button labels and modal instructions.

Training

N/A

Certification

N/A - Minor UI enhancement

Marketing

N/A

Automation tests

  • Unit tests

    N/A - Frontend component changes

  • Integration tests

    Manual testing performed for all user flows

Security checks

Samples

N/A

Related PRs

N/A

Migrations (if applicable)

N/A - No migrations required

Test environment

  • Node.js v20.19.6
  • macOS Darwin 24.5.0
  • Chrome browser
  • Vite dev server v7.1.7

Learning

Used react-hook-form's useFieldArray with replace() method for proper form field synchronization. Used getValues() instead of watched values to avoid stale closure issues in callbacks.

Summary by CodeRabbit

  • New Features

    • Bulk import modal for environment variables via paste or file upload with real-time variable count, invalid-key warnings, and visual feedback.
    • Import button added next to Add; imported variables merge with existing ones (imports override by key; empty rows ignored). Import/Cancel clear input and close the modal.
  • Chores

    • Parsing utility and modal exported from shared components for reuse.
  • Bug Fix / Validation

    • Environment variable key validation tightened to enforce valid identifier format.

✏️ Tip: You can customize this high-level summary in your review settings.

Add utility function to parse .env file content into key-value pairs.
Handles comments, empty lines, quoted values, and duplicate keys.
Add modal dialog for importing environment variables from .env content.
Supports both pasting content and file upload with real-time preview.
Add Bulk Import button and modal integration.
Imported variables merge with existing ones (duplicates are replaced).
Apply the same bulk import functionality to the add-new-agent page.
This fixes the empty first row issue that occurred after
bulk importing environment variables.
Enables bulk import modal usage in the add new agent page.
Gets current form values directly at import time instead of
relying on memoized watched values.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 24, 2025

📝 Walkthrough

Walkthrough

Adds a bulk-import modal and .env parser, integrates import UI into environment-variable editors (shared and page-level), and re-exports the new component and utilities from the shared-component package.

Changes

Cohort / File(s) Change Summary
New Modal Component
console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx
New React modal to paste or upload .env content; tracks textarea/file input, shows live variable count and invalid-key warnings, parses via parseEnvContent(), calls onImport(parsedVars), clears content and closes on import/cancel.
Parsing Utility
console/workspaces/libs/shared-component/src/utils/envParser.ts, console/workspaces/libs/shared-component/src/utils/index.ts
New parseEnvContent(content: string): ParseResult plus EnvVariable/ParseResult types. Splits lines, ignores comments/empty, splits on first =, trims/strips quotes, validates keys with ENV_KEY_REGEX, deduplicates keeping last, returns valid and invalid.
Shared-component Integration
console/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsx, console/workspaces/libs/shared-component/src/components/index.ts, console/workspaces/libs/shared-component/src/index.ts
Integrated modal into shared EnvironmentVariable (local modal state, handleImport merges imported vars by key with imported values overriding, ignores empty rows); exported EnvBulkImportModal from components barrel; re-exported utils from package index.
Page-level Integration & Dependency
console/workspaces/pages/add-new-agent/package.json, console/workspaces/pages/add-new-agent/src/components/EnvironmentVariable.tsx
Added workspace dependency on @agent-management-platform/shared-component; page-level EnvironmentVariable mirrors modal state, adds Import button and merge logic to consume parsed variables.
Form Validation Tightening
console/workspaces/pages/add-new-agent/src/form/schema.ts
Key field validation updated to enforce regex ([A-Za-z_][A-Za-z0-9_]*) instead of only trimmed non-empty, causing stricter validation/reporting for invalid keys.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Modal as EnvBulkImportModal
    participant Parser as parseEnvContent
    participant Form as EnvironmentVariable

    User->>Modal: Open modal
    User->>Modal: Paste content / Upload file
    Modal->>Modal: Read textarea or file text
    Note right of Modal: Live variable count\nand invalid-key warnings

    User->>Modal: Click Import
    Modal->>Parser: parseEnvContent(content)
    Parser-->>Modal: { valid: EnvVariable[], invalid: string[] }
    Modal->>Form: onImport(valid)
    Form->>Form: Merge imported vars by key\n(import overrides), drop empty rows
    Form-->>User: Updated form state/display
    Modal->>Modal: Clear content and close
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I munched through lines of keys and quotes,
I hushed the comments and chased stray notes,
Paste or upload, I count and pair,
I stitch each key with careful care.
Imported—hop! The envs are fair. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'feat: add bulk import for environment variables' clearly and concisely summarizes the main change—adding bulk import functionality for environment variables.
Description check ✅ Passed The PR description is comprehensive and follows the template well, covering Purpose, Goals, Approach, User stories, Release notes, and all required sections with appropriate information or N/A justifications.
Linked Issues check ✅ Passed The PR successfully implements all coding requirements from issue #106: enables pasting .env content, supports file upload, parses and validates variables, and merges imported variables with existing ones.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing bulk import functionality. The key validation regex addition in schema.ts aligns with the parsing validation in envParser.ts, supporting the core feature.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
console/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsx (1)

19-115: Consider consolidating duplicate EnvironmentVariable components.

This component is nearly identical to the one in console/workspaces/pages/add-new-agent/src/components/EnvironmentVariable.tsx, with only minor differences in import paths and styling. Since both components share the same logic for handling environment variables and bulk imports, consolidating them into a single shared implementation would:

  • Reduce code duplication
  • Simplify maintenance
  • Ensure consistent behavior across pages

Consider using this shared-component version in add-new-agent by importing it rather than maintaining two separate implementations.

🔎 Verification: Check if EnvironmentVariable is exported from shared-component
#!/bin/bash
# Check if EnvironmentVariable is exported from shared-component index
rg -n "EnvironmentVariable" console/workspaces/libs/shared-component/src/components/index.ts console/workspaces/libs/shared-component/src/index.ts
console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx (1)

61-79: Consider adding error handling for file upload.

The file upload handler lacks error handling for:

  • FileReader errors (e.g., file read failures)
  • Large file sizes that might cause browser issues
  • Non-text file content

For production robustness, consider adding:

const handleFileUpload = useCallback(
    (e: ChangeEvent<HTMLInputElement>) => {
        const file = e.target.files?.[0];
        if (!file) return;

        // Optional: check file size (e.g., 1MB limit)
        if (file.size > 1024 * 1024) {
            // Show error to user
            return;
        }

        const reader = new FileReader();
        reader.onload = (event) => {
            const text = event.target?.result;
            if (typeof text === "string") {
                setContent(text);
            }
        };
        reader.onerror = () => {
            // Show error to user
        };
        reader.readAsText(file);

        e.target.value = "";
    },
    []
);
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 228334b and eadb2ed.

⛔ Files ignored due to path filters (1)
  • console/common/config/rush/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (8)
  • console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx
  • console/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsx
  • console/workspaces/libs/shared-component/src/components/index.ts
  • console/workspaces/libs/shared-component/src/index.ts
  • console/workspaces/libs/shared-component/src/utils/envParser.ts
  • console/workspaces/libs/shared-component/src/utils/index.ts
  • console/workspaces/pages/add-new-agent/package.json
  • console/workspaces/pages/add-new-agent/src/components/EnvironmentVariable.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
console/workspaces/libs/shared-component/src/utils/envParser.ts (1)
console/workspaces/libs/shared-component/src/utils/index.ts (2)
  • EnvVariable (20-20)
  • parseEnvContent (19-19)
console/workspaces/pages/add-new-agent/src/components/EnvironmentVariable.tsx (2)
console/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsx (1)
  • EnvironmentVariable (27-115)
console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx (1)
  • EnvBulkImportModal (39-190)
console/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsx (2)
console/workspaces/pages/add-new-agent/src/components/EnvironmentVariable.tsx (1)
  • EnvironmentVariable (26-104)
console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx (1)
  • EnvBulkImportModal (39-190)
🔇 Additional comments (17)
console/workspaces/libs/shared-component/src/components/index.ts (1)

28-28: LGTM!

The export follows the established barrel pattern and correctly exposes the new EnvBulkImportModal component.

console/workspaces/libs/shared-component/src/index.ts (1)

20-20: LGTM!

The export correctly expands the public API to include utilities (parseEnvContent and EnvVariable type) for use by consumers.

console/workspaces/pages/add-new-agent/package.json (1)

52-52: LGTM!

The workspace dependency correctly enables the add-new-agent page to consume EnvBulkImportModal and related utilities from the shared-component library.

console/workspaces/libs/shared-component/src/utils/index.ts (1)

19-20: LGTM!

The barrel exports correctly separate type and value exports, following TypeScript best practices for public API design.

console/workspaces/libs/shared-component/src/utils/envParser.ts (3)

19-22: LGTM!

The EnvVariable interface appropriately models environment variables with a simple key-value structure.


25-34: LGTM!

The stripQuotes helper correctly handles basic quote stripping for single and double quotes, which is appropriate for typical .env file formats.


37-70: LGTM!

The parseEnvContent function implements a solid basic .env parser that handles:

  • Cross-platform line endings
  • Comments and empty lines
  • Quoted values (single and double)
  • Duplicate keys (last value wins)
  • Proper key-value splitting at the first =

The implementation is appropriate for typical .env file formats and aligns with the PR objectives.

console/workspaces/pages/add-new-agent/src/components/EnvironmentVariable.tsx (5)

19-19: LGTM!

The imports are correctly structured and all new dependencies (useState, useCallback, useMemo, FileText, EnvBulkImportModal, EnvVariable) are properly utilized in the component.

Also applies to: 21-21, 24-24


27-30: LGTM!

The hook additions correctly support the bulk import feature:

  • getValues avoids stale closures in handleImport
  • replace enables swapping the entire env array
  • useWatch provides reactive env values
  • useState manages modal visibility

32-36: LGTM!

The memoization correctly stabilizes envValues for use in callbacks and validation logic, preventing unnecessary re-renders and ensuring consistent behavior.

Also applies to: 38-38


41-64: LGTM!

The handleImport merge logic correctly:

  • Uses getValues to avoid stale closures
  • Filters empty rows before merging
  • Uses a Map for efficient key-based merging (imported values override existing)
  • Preserves key insertion order (existing keys maintain position, new imports append)
  • Replaces the entire array for clean state management

87-100: LGTM!

The UI integration correctly adds:

  • A "Bulk Import" button with appropriate icon and styling
  • EnvBulkImportModal properly wired with open state and handlers

The implementation provides a smooth user experience for bulk importing environment variables.

console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx (5)

19-31: LGTM!

All imports are correctly structured and utilized throughout the component.


33-37: LGTM!

The props interface correctly defines a standard modal contract with clear responsibilities for open state, close handling, and import callback.


44-50: LGTM!

State management is clean and efficient:

  • Content state for user input
  • Ref for file input control
  • Memoized parsing to avoid redundant work

87-99: LGTM!

Both handlers are correctly implemented:

  • handleImport validates count before proceeding and properly cleans up
  • handleClose ensures clean state reset

101-189: LGTM!

The UI implementation is excellent:

  • Clean dialog structure with appropriate sizing
  • Monospace textarea with helpful placeholder and theme-aware styling
  • Standard file upload pattern with clear accept types
  • Real-time visual feedback via count indicator with color coding
  • Proper button states (Import disabled when no variables detected)

The user experience is intuitive and polished.

- Updated button text from 'Bulk Import' to 'Import'
- Updated modal title from 'Bulk Import Environment Variables' to 'Import Environment Variables'
@Kavirubc
Copy link
Author

Kavirubc commented Jan 5, 2026

@menakaj ,

#106 (comment) is done.

Thanks.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
console/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsx (2)

41-65: Merge logic is well-implemented.

Good approach using getValues() to read fresh form state and a Map for deduplication. The imported variables correctly override existing ones with the same key.

One consideration: line 47 filters out entries where value is falsy, which would exclude intentionally empty values (e.g., EMPTY_VAR=). If empty values are valid in your use case, you might want to adjust the filter to only check for the key:

-        const nonEmptyExisting = currentEnv.filter((env) => env?.key && env?.value);
+        const nonEmptyExisting = currentEnv.filter((env) => env?.key);

This is a design decision—current behavior matches the PR description of "empty rows ignored."


107-112: Modal integration is correct.

The modal is properly controlled with state. The inline onClose handler creates a new function reference on each render—if you're concerned about re-render performance, you could memoize it:

const handleModalClose = useCallback(() => setImportModalOpen(false), []);

This is a minor optimization and likely unnecessary given the modal's usage pattern.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eadb2ed and bf4f21a.

📒 Files selected for processing (3)
  • console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx
  • console/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsx
  • console/workspaces/pages/add-new-agent/src/components/EnvironmentVariable.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
console/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsx (2)
console/workspaces/pages/add-new-agent/src/components/EnvironmentVariable.tsx (1)
  • EnvironmentVariable (26-104)
console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx (1)
  • EnvBulkImportModal (39-190)
console/workspaces/pages/add-new-agent/src/components/EnvironmentVariable.tsx (2)
console/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsx (1)
  • EnvironmentVariable (27-115)
console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx (1)
  • EnvBulkImportModal (39-190)
🔇 Additional comments (11)
console/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsx (5)

19-25: Imports are well-organized and complete.

All new imports are necessary for the bulk import feature: hooks for state management, the FileText icon for the button, and the modal component with its type.


27-31: Good use of react-hook-form APIs.

Using getValues() inside the callback instead of relying on watched values avoids stale closure issues. The replace method from useFieldArray is the correct choice for bulk updates.


33-39: Memoization and validation logic looks correct.

The useMemo stabilizes the envValues reference, and the optional chaining in isOneEmpty safely handles potential null/undefined entries.


75-86: Fields rendering follows react-hook-form best practices.

Using field.id as the key (rather than index) ensures stable component identity when fields are reordered or removed.


88-106: Action buttons are well-structured.

The Import button being always enabled (unlike Add) is a good UX choice—users can import regardless of current form state, and the merge logic handles cleanup.

console/workspaces/pages/add-new-agent/src/components/EnvironmentVariable.tsx (6)

19-24: LGTM!

The imports are well-organized and all are used within the component. Good use of shared components and types from @agent-management-platform/shared-component.


27-38: Well-structured hook setup with proper memoization.

Good practices observed:

  • Using getValues inside handleImport to get fresh form values avoids stale closure issues.
  • Memoizing envValues stabilizes the dependency for downstream logic.
  • The isOneEmpty check prevents users from creating multiple empty rows, which is good UX.

40-64: Import merge logic is well-implemented.

The approach of using a Map for deduplication and merging is clean. Key observations:

  • Correctly avoids stale closures by calling getValues('env') inside the callback.
  • Imported variables properly override existing ones with the same key.
  • JavaScript Map maintains insertion order, preserving the sequence of variables.

One note: rows where only the key OR only the value is set will be discarded during import (line 46 requires both). This appears intentional for cleaner merges, but worth confirming this matches the expected UX.


75-85: LGTM!

Correctly uses field.id as the key for the mapped elements, which is the recommended approach for useFieldArray. The props passed to EnvVariableEditor are consistent with the component's API.


87-94: Clean button layout implementation.

The side-by-side Add and Import buttons provide a clear UX. The isOneEmpty check on the Add button appropriately prevents multiple empty rows.


96-100: LGTM!

The EnvBulkImportModal is properly wired with the necessary props. The modal handles its internal state cleanup on close (clearing content), and the handleImport callback is correctly passed for the merge operation.

- Added regex validation for env variable keys (POSIX standard)
- Parser now returns both valid entries and invalid keys
- Modal displays error feedback showing which keys were skipped
- Added explanation of valid key format to help users
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Fix all issues with AI Agents 🤖
In
@console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx:
- Around line 125-146: The textarea in EnvBulkImportModal (the Box with
component="textarea" using value={content} and onChange={handleContentChange})
lacks an accessibility label; add an ARIA label or associate it with a visible
label by adding an aria-label (e.g., "Environment variables input" or similar)
or aria-labelledby referencing a nearby label element to describe its purpose
for screen readers, ensuring the attribute is applied directly on the Box
element.
♻️ Duplicate comments (1)
console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx (1)

150-155: Add accept attribute to file input for better UX.

The file input is missing an accept attribute, which relates to a previous review comment about verifying that .env files are shown in the file browser. Adding accept=".env,text/plain" would help filter the file picker, though note that .env files may not be visible in some OS file browsers since they're hidden files by default on Unix-like systems.

🔎 Suggested improvement
 <input
     ref={fileInputRef}
     type="file"
+    accept=".env,text/plain"
     onChange={handleFileUpload}
     style={{ display: "none" }}
+    aria-label="Upload environment variables file"
 />

You may want to manually test that .env files appear in the file browser on different operating systems (macOS, Windows, Linux) to confirm the user experience.

🧹 Nitpick comments (1)
console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx (1)

181-181: Use theme alpha helper for color transparency.

The color alpha manipulation using string concatenation (theme.palette.error.light + '20') is non-standard. While it may work in some contexts, it's better to use MUI's alpha() helper function or CSS rgba() for proper color transparency.

🔎 Recommended fix using alpha helper

Add alpha import at the top of the file:

 import {
     Box,
     Button,
     Dialog,
     DialogTitle,
     DialogContent,
     DialogActions,
     Typography,
     useTheme,
+    alpha,
 } from "@wso2/oxygen-ui";

Then update line 181:

 sx={{
     padding: 1.5,
-    backgroundColor: theme.palette.error.light + '20',
+    backgroundColor: alpha(theme.palette.error.light, 0.12),
     borderRadius: 1,
     border: `1px solid ${theme.palette.error.light}`,
 }}
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf4f21a and e299a9d.

📒 Files selected for processing (3)
  • console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx
  • console/workspaces/libs/shared-component/src/utils/envParser.ts
  • console/workspaces/libs/shared-component/src/utils/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • console/workspaces/libs/shared-component/src/utils/index.ts
  • console/workspaces/libs/shared-component/src/utils/envParser.ts
🧰 Additional context used
🧬 Code graph analysis (1)
console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx (2)
console/workspaces/libs/shared-component/src/utils/envParser.ts (2)
  • EnvVariable (19-22)
  • parseEnvContent (51-92)
console/workspaces/libs/shared-component/src/utils/index.ts (2)
  • EnvVariable (20-20)
  • parseEnvContent (19-19)
🔇 Additional comments (10)
console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx (10)

19-31: LGTM!

All imports are necessary and properly structured for the component's functionality.


33-37: LGTM!

The interface is well-defined with appropriate types for a modal component.


39-51: LGTM!

Proper use of useMemo to optimize parsing performance, and state management is correctly structured.


53-59: LGTM!

The textarea change handler is properly implemented with useCallback optimization.


82-85: LGTM!

Standard pattern for triggering the hidden file input.


87-94: LGTM!

The import handler correctly validates that variables exist before importing and properly cleans up state.


96-100: LGTM!

Proper cleanup of modal state before closing.


166-174: LGTM!

The variables count indicator with proper singular/plural handling provides helpful real-time feedback to users.


177-196: LGTM!

Excellent user feedback for invalid keys! The warning box clearly shows which keys were skipped and explains the valid key format requirements, helping users quickly identify and fix issues.


200-209: LGTM!

The dialog actions correctly disable the Import button when no valid variables are detected, preventing invalid operations.

Comment on lines +61 to +80
// Handle file upload
const handleFileUpload = useCallback(
(e: ChangeEvent<HTMLInputElement>) => {
const file = e.target.files?.[0];
if (!file) return;

const reader = new FileReader();
reader.onload = (event) => {
const text = event.target?.result;
if (typeof text === "string") {
setContent(text);
}
};
reader.readAsText(file);

// Reset input so same file can be selected again
e.target.value = "";
},
[]
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add file size validation and error handling.

The file upload handler has the following concerns:

  1. Missing file size validation: Large files could cause memory issues or make the browser unresponsive.
  2. Missing error handling: If FileReader fails (e.g., unreadable file, permission issues), the user receives no feedback.
  3. No file type validation: Binary or non-text files could be selected and cause issues.
🔎 Proposed fix with validation and error handling
 const handleFileUpload = useCallback(
     (e: ChangeEvent<HTMLInputElement>) => {
         const file = e.target.files?.[0];
         if (!file) return;
+
+        // Validate file size (e.g., max 1MB)
+        const MAX_FILE_SIZE = 1024 * 1024; // 1MB
+        if (file.size > MAX_FILE_SIZE) {
+            alert(`File is too large. Maximum size is ${MAX_FILE_SIZE / 1024}KB.`);
+            e.target.value = "";
+            return;
+        }

         const reader = new FileReader();
+        reader.onerror = () => {
+            alert("Failed to read file. Please try again.");
+            e.target.value = "";
+        };
         reader.onload = (event) => {
             const text = event.target?.result;
             if (typeof text === "string") {
                 setContent(text);
             }
         };
         reader.readAsText(file);

         // Reset input so same file can be selected again
         e.target.value = "";
     },
     []
 );

Note: Consider using a proper notification/toast system instead of alert() for better UX.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Handle file upload
const handleFileUpload = useCallback(
(e: ChangeEvent<HTMLInputElement>) => {
const file = e.target.files?.[0];
if (!file) return;
const reader = new FileReader();
reader.onload = (event) => {
const text = event.target?.result;
if (typeof text === "string") {
setContent(text);
}
};
reader.readAsText(file);
// Reset input so same file can be selected again
e.target.value = "";
},
[]
);
// Handle file upload
const handleFileUpload = useCallback(
(e: ChangeEvent<HTMLInputElement>) => {
const file = e.target.files?.[0];
if (!file) return;
// Validate file size (e.g., max 1MB)
const MAX_FILE_SIZE = 1024 * 1024; // 1MB
if (file.size > MAX_FILE_SIZE) {
alert(`File is too large. Maximum size is ${MAX_FILE_SIZE / 1024}KB.`);
e.target.value = "";
return;
}
const reader = new FileReader();
reader.onerror = () => {
alert("Failed to read file. Please try again.");
e.target.value = "";
};
reader.onload = (event) => {
const text = event.target?.result;
if (typeof text === "string") {
setContent(text);
}
};
reader.readAsText(file);
// Reset input so same file can be selected again
e.target.value = "";
},
[]
);

Comment on lines +125 to +146
<Box
component="textarea"
value={content}
onChange={handleContentChange}
placeholder={`# Example format:\nAPI_KEY=your_api_key\nDATABASE_URL=postgres://...\nDEBUG="true"`}
sx={{
width: "100%",
minHeight: 200,
padding: 1.5,
fontFamily: "monospace",
fontSize: 13,
border: `1px solid ${theme.palette.divider}`,
borderRadius: 1,
resize: "vertical",
backgroundColor: theme.palette.background.paper,
color: theme.palette.text.primary,
"&:focus": {
outline: "none",
borderColor: theme.palette.primary.main,
},
}}
/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add accessibility label to textarea.

The textarea is missing an aria-label attribute, which is an accessibility blocker for screen reader users who need to understand the purpose of the input field.

🔎 Proposed fix
 <Box
     component="textarea"
     value={content}
     onChange={handleContentChange}
     placeholder={`# Example format:\nAPI_KEY=your_api_key\nDATABASE_URL=postgres://...\nDEBUG="true"`}
+    aria-label="Environment variables content"
     sx={{
         width: "100%",
         minHeight: 200,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Box
component="textarea"
value={content}
onChange={handleContentChange}
placeholder={`# Example format:\nAPI_KEY=your_api_key\nDATABASE_URL=postgres://...\nDEBUG="true"`}
sx={{
width: "100%",
minHeight: 200,
padding: 1.5,
fontFamily: "monospace",
fontSize: 13,
border: `1px solid ${theme.palette.divider}`,
borderRadius: 1,
resize: "vertical",
backgroundColor: theme.palette.background.paper,
color: theme.palette.text.primary,
"&:focus": {
outline: "none",
borderColor: theme.palette.primary.main,
},
}}
/>
<Box
component="textarea"
value={content}
onChange={handleContentChange}
placeholder={`# Example format:\nAPI_KEY=your_api_key\nDATABASE_URL=postgres://...\nDEBUG="true"`}
aria-label="Environment variables content"
sx={{
width: "100%",
minHeight: 200,
padding: 1.5,
fontFamily: "monospace",
fontSize: 13,
border: `1px solid ${theme.palette.divider}`,
borderRadius: 1,
resize: "vertical",
backgroundColor: theme.palette.background.paper,
color: theme.palette.text.primary,
"&:focus": {
outline: "none",
borderColor: theme.palette.primary.main,
},
}}
/>
🤖 Prompt for AI Agents
In
@console/workspaces/libs/shared-component/src/components/EnvBulkImportModal.tsx
around lines 125-146, The textarea in EnvBulkImportModal (the Box with
component="textarea" using value={content} and onChange={handleContentChange})
lacks an accessibility label; add an ARIA label or associate it with a visible
label by adding an aria-label (e.g., "Environment variables input" or similar)
or aria-labelledby referencing a nearby label element to describe its purpose
for screen readers, ensuring the attribute is applied directly on the Box
element.

- Added yup regex validation for env key field in add-new-agent form
- Keys must start with letter/underscore, contain only alphanumeric and underscores
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
console/workspaces/pages/add-new-agent/src/form/schema.ts (1)

111-114: The regex patterns intentionally differ; do not extract as a shared constant.

The schema regex /^$|^[A-Za-z_][A-Za-z0-9_]*$/ allows empty strings to support incomplete form rows (users can add rows and fill them incrementally). The complete-pairs test (line 116–119) enforces that both key and value are filled or both are empty, and the unique-keys test filters empty keys, preventing conflicts.

The ENV_KEY_REGEX in envParser.ts (/^[A-Za-z_][A-Za-z0-9_]*$/) intentionally does not allow empty strings because it validates parsed .env file content, not form input. These serve different validation contexts and should remain separate.

To improve clarity, add a comment in schema.ts explaining why the regex allows empty strings for the incomplete-pair use case.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e299a9d and b80310b.

📒 Files selected for processing (1)
  • console/workspaces/pages/add-new-agent/src/form/schema.ts

@Kavirubc Kavirubc requested a review from menakaj January 9, 2026 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] Support importing environment variables via a .env file

2 participants