Reject empty or whitespace-only configKey values in include_assets builds#7530
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Pull request overview
This PR adds an early validation guard to the copyConfigKeyEntry helper used by the include-assets build step to reject configKey values that are explicitly set to an empty or whitespace-only string, failing fast before any filesystem copying begins.
Changes:
- Add a pre-I/O validation pass that throws an
AbortErrorwhen a resolved config path is empty/whitespace-only. - Format the error message using the leaf field name (e.g.,
assets) rather than the full dotted configKey path. - Add unit tests covering empty string, whitespace-only string, and nested configKey behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts | Adds early validation to reject empty/whitespace-only resolved config paths and throw an AbortError with a leaf-field message. |
| packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts | Adds tests verifying the new guard triggers for empty/whitespace-only values and uses the leaf field name for nested keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MitchLillie
left a comment
There was a problem hiding this comment.
🎩 looks good
Are you sure we want to disallow all config values from being empty? I know we're against hard-coding things, but this seems too broad.
For example [[extensions]] \n description = "" is optional so we should allow that to be empty. When I add an empty description, the validation passes though, so maybe I am not understanding the flow of things.
|
@Mitch this check is only for the copy-configKey step which copy files over to the deploy bundle - setting an empty string doesn't make any sense for this. Other configs not related to file copying are unaffected and already have their own validation. |
|
It also won't fire if it's entirely absent so it will still respect optionality -- assets is optional too. Should just reject being explicitly set to empty |
e80977f to
7a0fd2d
Compare

WHY are these changes introduced?
Splitting out the empty/whitespace validation from #7504.
The additional path validation from that PR requires a larger conversation, while this portion is a lot simpler.
WHAT is this pull request doing?
Adds an empty-value validation pass in copyConfigKeyEntry that throws AbortError with message '<field>' can't be empty. when a configKey resolves to an empty or whitespace-only string.
How to test your changes?
Follow the steps in #7504
assert "cannot be empty"error is throwncannot be emptyerror is thrownPost-release steps
Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add