fix(webui): Add absolute path validation to compression paths input (fixes #1702). by junhaoliao · Pull Request #1705 · y-scope/clp

junhaoliao · 2025-12-01T03:00:11Z

Description

This change introduces validation for absolute file system paths in compression job
submission workflows.

Changes include:

Adding AbsolutePathSchema to constrain values to non-empty strings beginning with /.
Updating CompressionJobCreationSchema so paths is now an array of AbsolutePathSchema with
minItems: 1.
Adding validateAbsolutePaths to the ingest page to validate each line of multiline input.
Replacing the old required-rule check with a custom validator that provides line-specific error
messages.
Updating the placeholder text to clarify that only absolute paths are valid.

These updates ensure that only valid absolute paths can be submitted, preventing backend errors and
improving user feedback.

Checklist

The PR satisfies the contribution guidelines.
This is a breaking change and that has been indicated in the PR title, OR this isn't a
breaking change.
Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Manually tested frontend validation in the ingestion UI:
- Entered valid absolute paths:
  - /var/log/syslog
  - /home/samples
  - Empty lines mixed with valid paths
- Verified that form submission succeeded with no validation errors.
Tested invalid values:
- Relative paths (logs/syslog)
- Paths with tildes (~/samples)
- Lines containing only whitespace
- Verified error messages referenced correct line number and blocked submission.

Summary by CodeRabbit

Bug Fixes
- Per-line validation for path inputs: ignores blank lines and shows a clear, line-specific error for the first non-absolute path.
New Features
- Forms now require at least one absolute path (one per line) and include an updated placeholder guiding users to enter absolute paths on the server host.

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

…ixes y-scope#1702).

coderabbitai · 2025-12-01T03:00:16Z

Walkthrough

Added multiline absolute-path validation for compression jobs: a new validator checks each non-empty input line is an absolute path; the form item enforces this with a required/validator rule and updated placeholder; the compression schema now requires an array of absolute paths (minItems: 1).

Changes

Cohort / File(s)	Summary
Paths input form `components/webui/client/src/pages/IngestPage/Compress/PathsInputFormItem.tsx`	Imported `validateAbsolutePaths`. Expanded `Form.Item` rules to include a required, whitespace-insensitive check with message "Please enter at least one path." and an async validator that calls `validateAbsolutePaths(value)` and rejects with its message on error. Updated `Input.TextArea` placeholder to "Enter absolute paths on the server host, one per line".
Validation utilities `components/webui/client/src/pages/IngestPage/Compress/validation.ts`	Added `validateAbsolutePaths(value: string): Nullable<string>` to validate each non-empty line against `AbsolutePathSchema`, returning the first line-specific error or `null`. Updated imports to include `AbsolutePathSchema` and `Nullable`. Export block now exposes `validateAbsolutePaths` alongside `validateDatasetName`.
Schema definitions `components/webui/common/src/schemas/compression.ts`	Added `AbsolutePathSchema` (non-empty string pattern starting with "/"). Updated `CompressionJobCreationSchema` to use `paths: Type.Array(AbsolutePathSchema, { minItems: 1 })` instead of a plain string array. Export list now includes `AbsolutePathSchema`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Review line-splitting, trimming, and handling of blank/whitespace-only lines in validateAbsolutePaths.
Verify Ant Design async validator integration and that error messages surface on the correct form field.
Confirm AbsolutePathSchema pattern, minItems and imports/exports are consistent with backend/other modules.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: adding absolute path validation to the compression paths input field, with a reference to the fixed issue number.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8750729 and c45c629.

📒 Files selected for processing (1)

components/webui/common/src/schemas/compression.ts (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}

⚙️ CodeRabbit configuration file

Prefer false == <expression> rather than !<expression>.

Files:

components/webui/common/src/schemas/compression.ts

🧠 Learnings (2)

📓 Common learnings

Learnt from: quinntaylormitchell
Repo: y-scope/clp PR: 1125
File: components/job-orchestration/job_orchestration/scheduler/compress/compression_scheduler.py:267-291
Timestamp: 2025-09-15T22:20:40.750Z
Learning: For CLP compression jobs, the team has decided to fail the entire job immediately upon encountering any invalid input path, rather than continuing to process valid paths. This decision was made during PR #1125 development.

Learnt from: haiqi96
Repo: y-scope/clp PR: 651
File: components/clp-package-utils/clp_package_utils/scripts/compress.py:0-0
Timestamp: 2025-01-16T16:58:43.190Z
Learning: In the clp-package compression flow, path validation and error handling is performed at the scheduler level rather than in the compress.py script to maintain simplicity and avoid code duplication.

📚 Learning: 2024-11-15T16:22:33.635Z

Learnt from: haiqi96
Repo: y-scope/clp PR: 594
File: components/clp-package-utils/clp_package_utils/scripts/native/del_archives.py:90-96
Timestamp: 2024-11-15T16:22:33.635Z
Learning: In the CLP codebase, `table_prefix` is a trusted configuration value and can be safely embedded into SQL queries.

Applied to files:

components/webui/common/src/schemas/compression.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: package-image
GitHub Check: lint-check (ubuntu-24.04)
GitHub Check: lint-check (macos-15)
GitHub Check: build (ubuntu-24.04)
GitHub Check: build (macos-15)

🔇 Additional comments (4)

components/webui/common/src/schemas/compression.ts (4)

9-9: ESM import extension update looks correct

Using "../config.js" keeps this module consistent with explicit ESM import paths and should work as long as the rest of the webui uses the same convention. No further changes needed here.

45-53: AbsolutePathSchema correctly encodes POSIX absolute paths; confirm trimming and platform assumptions

This schema enforces non-empty strings beginning with /, which matches the new UI requirement for absolute paths and is consistent with a POSIX-only environment. If users might paste paths with leading/trailing spaces or you ever need to support non-POSIX absolute paths (e.g., Windows C:\logs), please confirm that upstream code trims/normalises lines and that this POSIX-only restriction is intentional so it stays aligned with scheduler-level validation that fails jobs on any invalid path (based on learnings, this scheduler behaviour is the source of truth).

75-82: Exporting AbsolutePathSchema is a good reuse point

Exposing AbsolutePathSchema alongside the other schemas is helpful for keeping client-side validators and any other schema users consistent with the compression API’s path requirements.

59-62: paths array now requires at least one absolute path; verify external consumers

Switching paths to Type.Array(AbsolutePathSchema, {minItems: 1}) tightens the contract in a good way and aligns with the scheduler's behavior from PR #1125, which fails compression jobs immediately upon encountering invalid input paths rather than filtering them out. Ensure all consumers of CompressionJobCreationSchema expect absolute, non-empty paths with at least one entry.

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.}

davemarco

small comments

davemarco · 2025-12-01T18:34:19Z

+                // The `validator` function expects a `Promise` to be returned.
+                // eslint-disable-next-line @typescript-eslint/require-await
+                validator: async (_, value?: string) => {
+                    const error = validateAbsolutePaths(value ?? "");


I think they want you to use promises. Maybe look at the example here https://ant.design/components/form#form-demo-register.

if i'm not wrong, the async syntax is also returning promises and cleaner to write / read. in fact, the async syntax enforces only Promises can be returned

similarly, if anything is thrown in the async function, promise rejections are automatically created

davemarco · 2025-12-01T18:37:30Z

        label={"Paths"}
        name={"paths"}
-        rules={[{required: true, message: "Please enter at least one path"}]}
+        rules={[


maybe leave the rule, and just add the white space flag

then we might not need the check at the end as well. Since it would be redundant

good point. let me try

not sure how, but it seems once a validator is defined, any other fields in the rule are ignored

actually let me try another approach

Something like this dosent work?

splitting the rules works, but the whitespace thing we actually need to set it to true to prohibit whitespaces lol (like i thought setting whitespace: false would be more intuitive

anyways it's all good for review now

…ing.

davemarco

mostly lgtm, do u want to move ur validator to my new validate.ts file after you merge main? then i will approve

# Conflicts: # components/webui/common/src/schemas/compression.ts

…itialization.

davemarco

lgtm

error

…ility.

davemarco

lgtm

…ixes y-scope#1702). (y-scope#1705)

fix(webui): Add absolute path validation to compression paths input (f…

169061e

…ixes y-scope#1702).

junhaoliao marked this pull request as ready for review December 1, 2025 03:07

junhaoliao requested a review from a team as a code owner December 1, 2025 03:07

junhaoliao requested review from davemarco and kirkrodrigues December 1, 2025 03:07

davemarco requested changes Dec 1, 2025

View reviewed changes

junhaoliao and others added 2 commits December 1, 2025 17:59

Absolute path prompt - Apply suggestions from code review

64718dc

fix(webui): Move path validation to form rules for better error handl…

a35e8a8

…ing.

junhaoliao requested a review from davemarco December 1, 2025 23:23

Merge branch 'main' into validate-paths

d24699a

davemarco requested changes Dec 1, 2025

View reviewed changes

junhaoliao added 3 commits December 1, 2025 18:49

Merge remote-tracking branch 'origin/main' into validate-paths

7c47c64

# Conflicts: # components/webui/common/src/schemas/compression.ts

move validateAbsolutePaths to validation.ts

8642617

fix(webui): Simplify validator by removing redundant default value in…

8750729

…itialization.

junhaoliao requested a review from davemarco December 1, 2025 23:55

davemarco previously approved these changes Dec 1, 2025

View reviewed changes

fix(webui): Update import to use explicit file extension for compatib…

c45c629

…ility.

davemarco self-requested a review December 2, 2025 00:16

davemarco approved these changes Dec 2, 2025

View reviewed changes

kirkrodrigues merged commit 85cc1a1 into y-scope:main Dec 2, 2025
20 checks passed

junhaoliao deleted the validate-paths branch May 7, 2026 19:46

junhaoliao added a commit to junhaoliao/clp that referenced this pull request May 17, 2026

fix(webui): Add absolute path validation to compression paths input (f…

7d190eb

…ixes y-scope#1702). (y-scope#1705)

Conversation

junhaoliao commented Dec 1, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Validation performed

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

davemarco left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junhaoliao Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davemarco left a comment

Choose a reason for hiding this comment

Uh oh!

davemarco left a comment

Choose a reason for hiding this comment

Uh oh!

davemarco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

junhaoliao commented Dec 1, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Dec 1, 2025 •

edited

Loading

junhaoliao Dec 1, 2025 •

edited

Loading