Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces support for distinguishing between public and non-public theme files throughout the build, extension syncing, and CLI tooling. It adds handling for a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant OwnModule
participant GitignoreManager
participant FileSystem
User->>CLI: Run "own" command (with/without --public)
CLI->>OwnModule: Pass isPublic flag
OwnModule->>GitignoreManager: Read managed .gitignore(s)
GitignoreManager->>FileSystem: Read .gitignore files (public/non-public)
FileSystem-->>GitignoreManager: Return owned file list(s)
OwnModule->>FileSystem: Claim or revert file ownership in correct dir (public/src)
OwnModule->>GitignoreManager: Write updated .gitignore(s)
GitignoreManager->>FileSystem: Write .gitignore files (public/non-public)
sequenceDiagram
participant BuildProcess
participant ResourceGenerator
participant FileSystem
BuildProcess->>ResourceGenerator: Generate resources
ResourceGenerator->>FileSystem: Check for KEYCLOAK_THEME/publicDirPath
alt Directory exists
ResourceGenerator->>FileSystem: Copy contents to build/resources/dist/KEYCLOAK_THEME
end
ResourceGenerator->>FileSystem: Handle legacy KEYCLOAKIFY_DEV_RESOURCES as before
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/bin/keycloakify/generateResources/generateResources.ts (1)
589-589: Clarify the TODO commentThe TODO comment lacks details about what needs to be implemented. Please add a description of the intended check to skip this block.
src/bin/sync-extensions/managedGitignoreFiles.ts (2)
110-112: Use optional chaining for cleaner code.The condition can be simplified using optional chaining.
Apply this diff to improve readability:
-if (content_current !== undefined && content_current.equals(content_new)) { +if (content_current?.equals(content_new)) { continue; }🧰 Tools
🪛 Biome (1.9.4)
[error] 110-110: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
114-123: Remove the unnecessary labeled block.The labeled block is unconventional for non-loop statements and can be replaced with a simple conditional.
Apply this diff to use a more idiomatic approach:
-create_dir: { - const dirPath = pathDirname(filePath); - - if (await existsAsync(dirPath)) { - break create_dir; - } - - await fsPr.mkdir(dirPath, { recursive: true }); -} +const dirPath = pathDirname(filePath); + +if (!(await existsAsync(dirPath))) { + await fsPr.mkdir(dirPath, { recursive: true }); +}🧰 Tools
🪛 Biome (1.9.4)
[error] 114-114: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
package.json(1 hunks)src/bin/keycloakify/generateResources/generateResources.ts(5 hunks)src/bin/main.ts(2 hunks)src/bin/own.ts(10 hunks)src/bin/sync-extensions/extensionModuleMeta.ts(4 hunks)src/bin/sync-extensions/getExtensionModuleFileSourceCodeReadyToBeCopied.ts(3 hunks)src/bin/sync-extensions/managedGitignoreFile.ts(0 hunks)src/bin/sync-extensions/managedGitignoreFiles.ts(1 hunks)src/bin/sync-extensions/sync-extension.ts(3 hunks)src/vite-plugin/vite-plugin.ts(3 hunks)
💤 Files with no reviewable changes (1)
- src/bin/sync-extensions/managedGitignoreFile.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/vite-plugin/vite-plugin.ts (1)
src/bin/shared/constants.ts (1)
KEYCLOAK_THEME(81-81)
src/bin/sync-extensions/extensionModuleMeta.ts (3)
src/bin/tools/crawlAsync.ts (1)
crawlAsync(6-27)src/bin/shared/constants.ts (1)
KEYCLOAK_THEME(81-81)src/bin/sync-extensions/getExtensionModuleFileSourceCodeReadyToBeCopied.ts (1)
getExtensionModuleFileSourceCodeReadyToBeCopied(15-99)
src/bin/sync-extensions/managedGitignoreFiles.ts (5)
src/bin/sync-extensions/getExtensionModuleFileSourceCodeReadyToBeCopied.ts (1)
BuildContextLike(8-11)src/bin/sync-extensions/extensionModuleMeta.ts (2)
BuildContextLike(88-93)ExtensionModuleMeta(26-36)src/bin/shared/buildContext.ts (1)
BuildContext(27-78)src/bin/shared/constants.ts (1)
KEYCLOAK_THEME(81-81)src/bin/tools/getAbsoluteAndInOsFormatPath.ts (1)
getAbsoluteAndInOsFormatPath(9-34)
src/bin/sync-extensions/getExtensionModuleFileSourceCodeReadyToBeCopied.ts (3)
src/bin/shared/buildContext.ts (1)
BuildContext(27-78)src/bin/sync-extensions/managedGitignoreFiles.ts (1)
BuildContextLike(15-18)src/bin/shared/constants.ts (1)
KEYCLOAK_THEME(81-81)
🪛 Biome (1.9.4)
src/bin/keycloakify/generateResources/generateResources.ts
[error] 602-602: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.
(lint/suspicious/noConfusingLabels)
src/bin/sync-extensions/managedGitignoreFiles.ts
[error] 110-110: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 114-114: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.
(lint/suspicious/noConfusingLabels)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test with Node v18 on ubuntu-latest
🔇 Additional comments (11)
package.json (1)
3-3: LGTM!The version bump to release candidate is appropriate for introducing the new public/non-public theme file support feature.
src/vite-plugin/vite-plugin.ts (1)
3-7: Well-structured cleanup implementation!The addition of
KEYCLOAK_THEMEcleanup follows the existing pattern and properly marks the legacy code for future removal. The consistent use of recursive and force options ensures thorough cleanup of build artifacts.Also applies to: 207-223
src/bin/main.ts (1)
284-284: Clean CLI option implementation!The
publicflag is well-integrated into the existing command structure with clear documentation and intuitive short/long flag names. The parameter passing follows the established pattern.Also applies to: 335-350, 353-359
src/bin/keycloakify/generateResources/generateResources.ts (2)
158-181: Good refactoring for dual directory support!The loop-based approach cleanly handles both the new
KEYCLOAK_THEMEand legacy directories, with clear error messages that adapt to the specific directory found. This structure will make it easy to remove the legacy support in the future.
602-631: Well-structured extension resource handling!The new
extensions_static_resourcesblock follows the established pattern with appropriate guards for native and SPA themes. The static analysis warning about the label is a false positive - this codebase consistently uses labeled break statements for conditional execution blocks.The implementation correctly copies extension static resources from the public directory to the theme's resources directory.
🧰 Tools
🪛 Biome (1.9.4)
[error] 602-602: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
src/bin/sync-extensions/extensionModuleMeta.ts (2)
4-9: LGTM!The import additions are appropriate for the new path handling logic.
30-30: LGTM!Type and schema additions are consistent and properly integrated.
Also applies to: 46-46
src/bin/sync-extensions/sync-extension.ts (1)
5-7: LGTM!The refactoring correctly handles the new ownership structure with
isPublicflag and uses appropriate deep equality checks.Also applies to: 15-16, 28-36, 43-62
src/bin/sync-extensions/getExtensionModuleFileSourceCodeReadyToBeCopied.ts (2)
10-10: LGTM!The type extension and parameter additions properly support the public/non-public file distinction.
Also applies to: 17-17, 27-27
80-85: LGTM!The conditional path logic correctly routes files to their appropriate directories.
src/bin/own.ts (1)
5-8: LGTM!The ownership management refactoring correctly handles the new public/non-public file distinction throughout all operations. The use of deep equality checks and conditional path resolution is well-implemented.
Also applies to: 15-16, 24-24, 32-44, 51-59, 88-90, 103-105, 118-133, 147-151, 169-177, 206-210, 229-238
| await crawlAsync({ | ||
| dirPath: pathJoin(dirPath, KEYCLOAK_THEME), | ||
| returnedPathsType: "relative to dirPath", | ||
| onFileFound: async fileRelativePath => { | ||
| const isPublic = fileRelativePath.startsWith(`public${pathSep}`); | ||
|
|
||
| const fileRelativePath_contextual = isPublic | ||
| ? pathRelative("public", fileRelativePath) | ||
| : fileRelativePath; | ||
|
|
||
| const sourceCode = | ||
| await getExtensionModuleFileSourceCodeReadyToBeCopied({ | ||
| buildContext, | ||
| isPublic, | ||
| fileRelativePath: fileRelativePath_contextual, | ||
| isOwnershipAction: false, | ||
| extensionModuleDirPath: dirPath, | ||
| extensionModuleName: moduleName, | ||
| extensionModuleVersion: version | ||
| }); | ||
|
|
||
| fsPr.writeFile(copyableFilePath, sourceCode); | ||
| const hash = computeHash(sourceCode); | ||
|
|
||
| files.push({ | ||
| fileRelativePath, | ||
| hash, | ||
| copyableFilePath | ||
| }); | ||
| const copyableFilePath = pathJoin( | ||
| pathDirname(cacheFilePath), | ||
| KEYCLOAK_THEME, | ||
| fileRelativePath | ||
| ); | ||
|
|
||
| { | ||
| const dirPath = pathDirname(copyableFilePath); | ||
|
|
||
| if (!(await existsAsync(dirPath))) { | ||
| await fsPr.mkdir(dirPath, { recursive: true }); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| fsPr.writeFile(copyableFilePath, sourceCode); | ||
|
|
||
| files.push({ | ||
| isPublic, | ||
| fileRelativePath: fileRelativePath_contextual, | ||
| hash, | ||
| copyableFilePath | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Fix path handling for cross-platform compatibility and consistency.
Two issues in the file crawling logic:
- The public file detection might not work correctly on Windows where paths could use forward slashes in some contexts.
- The
copyableFilePathuses the originalfileRelativePathinstead offileRelativePath_contextual, which could lead to incorrect cache paths for public files.
Apply this diff to fix both issues:
- const isPublic = fileRelativePath.startsWith(`public${pathSep}`);
+ const normalizedPath = fileRelativePath.split(pathSep).join("/");
+ const isPublic = normalizedPath.startsWith("public/");
const fileRelativePath_contextual = isPublic
? pathRelative("public", fileRelativePath)
: fileRelativePath;
const sourceCode =
await getExtensionModuleFileSourceCodeReadyToBeCopied({
buildContext,
isPublic,
fileRelativePath: fileRelativePath_contextual,
isOwnershipAction: false,
extensionModuleDirPath: dirPath,
extensionModuleName: moduleName,
extensionModuleVersion: version
});
const hash = computeHash(sourceCode);
const copyableFilePath = pathJoin(
pathDirname(cacheFilePath),
KEYCLOAK_THEME,
- fileRelativePath
+ isPublic ? pathJoin("public", fileRelativePath_contextual) : fileRelativePath_contextual
);📝 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.
| await crawlAsync({ | |
| dirPath: pathJoin(dirPath, KEYCLOAK_THEME), | |
| returnedPathsType: "relative to dirPath", | |
| onFileFound: async fileRelativePath => { | |
| const isPublic = fileRelativePath.startsWith(`public${pathSep}`); | |
| const fileRelativePath_contextual = isPublic | |
| ? pathRelative("public", fileRelativePath) | |
| : fileRelativePath; | |
| const sourceCode = | |
| await getExtensionModuleFileSourceCodeReadyToBeCopied({ | |
| buildContext, | |
| isPublic, | |
| fileRelativePath: fileRelativePath_contextual, | |
| isOwnershipAction: false, | |
| extensionModuleDirPath: dirPath, | |
| extensionModuleName: moduleName, | |
| extensionModuleVersion: version | |
| }); | |
| fsPr.writeFile(copyableFilePath, sourceCode); | |
| const hash = computeHash(sourceCode); | |
| files.push({ | |
| fileRelativePath, | |
| hash, | |
| copyableFilePath | |
| }); | |
| const copyableFilePath = pathJoin( | |
| pathDirname(cacheFilePath), | |
| KEYCLOAK_THEME, | |
| fileRelativePath | |
| ); | |
| { | |
| const dirPath = pathDirname(copyableFilePath); | |
| if (!(await existsAsync(dirPath))) { | |
| await fsPr.mkdir(dirPath, { recursive: true }); | |
| } | |
| } | |
| }); | |
| } | |
| fsPr.writeFile(copyableFilePath, sourceCode); | |
| files.push({ | |
| isPublic, | |
| fileRelativePath: fileRelativePath_contextual, | |
| hash, | |
| copyableFilePath | |
| }); | |
| } | |
| }); | |
| await crawlAsync({ | |
| dirPath: pathJoin(dirPath, KEYCLOAK_THEME), | |
| returnedPathsType: "relative to dirPath", | |
| onFileFound: async fileRelativePath => { | |
| // normalize to forward‐slash separators for reliable matching | |
| const normalizedPath = fileRelativePath.split(pathSep).join("/"); | |
| const isPublic = normalizedPath.startsWith("public/"); | |
| const fileRelativePath_contextual = isPublic | |
| ? pathRelative("public", fileRelativePath) | |
| : fileRelativePath; | |
| const sourceCode = | |
| await getExtensionModuleFileSourceCodeReadyToBeCopied({ | |
| buildContext, | |
| isPublic, | |
| fileRelativePath: fileRelativePath_contextual, | |
| isOwnershipAction: false, | |
| extensionModuleDirPath: dirPath, | |
| extensionModuleName: moduleName, | |
| extensionModuleVersion: version | |
| }); | |
| const hash = computeHash(sourceCode); | |
| const copyableFilePath = pathJoin( | |
| pathDirname(cacheFilePath), | |
| KEYCLOAK_THEME, | |
| isPublic | |
| ? pathJoin("public", fileRelativePath_contextual) | |
| : fileRelativePath_contextual | |
| ); | |
| { | |
| const dirPath = pathDirname(copyableFilePath); | |
| if (!(await existsAsync(dirPath))) { | |
| await fsPr.mkdir(dirPath, { recursive: true }); | |
| } | |
| } | |
| await fsPr.writeFile(copyableFilePath, sourceCode); | |
| files.push({ | |
| isPublic, | |
| fileRelativePath: fileRelativePath_contextual, | |
| hash, | |
| copyableFilePath | |
| }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/bin/sync-extensions/extensionModuleMeta.ts between lines 232 and 278, fix
path handling for cross-platform compatibility by replacing the string check for
public files with a path-aware method that works on all OSes, and use
fileRelativePath_contextual instead of fileRelativePath when constructing
copyableFilePath to ensure correct cache paths for public files. Update the
isPublic detection to use path segment comparison and change copyableFilePath to
join with fileRelativePath_contextual.
| `To relinquish ownership and restore this file to its original content, run the following command:`, | ||
| ``, | ||
| `$ npx keycloakify own --path "${path}" --revert` | ||
| `$ npx keycloakify own --path "${path}"${isPublic ? " --public " : ""}--revert` |
There was a problem hiding this comment.
Fix extra space in command string.
There's an extra space before --revert when isPublic is true, resulting in two spaces.
Apply this diff to fix the formatting:
- `$ npx keycloakify own --path "${path}"${isPublic ? " --public " : ""}--revert`
+ `$ npx keycloakify own --path "${path}"${isPublic ? " --public" : ""} --revert`📝 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.
| `$ npx keycloakify own --path "${path}"${isPublic ? " --public " : ""}--revert` | |
| `$ npx keycloakify own --path "${path}"${isPublic ? " --public" : ""} --revert` |
🤖 Prompt for AI Agents
In src/bin/sync-extensions/getExtensionModuleFileSourceCodeReadyToBeCopied.ts at
line 67, the command string has an extra space before the --revert flag when
isPublic is true, causing double spaces. Remove the trailing space inside the
isPublic conditional expression so that the space only appears once between the
flags, ensuring correct command formatting.
| getAbsoluteAndInOsFormatPath({ | ||
| cwd: buildContext.themeSrcDirPath, | ||
| pathIsh: line | ||
| }) | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify path resolution for public files.
The current implementation uses buildContext.themeSrcDirPath as the working directory for all files, but public files are stored in a different directory structure. This might lead to incorrect path resolution for public files.
Consider using the appropriate base directory based on the isPublicIteration flag:
-getAbsoluteAndInOsFormatPath({
- cwd: buildContext.themeSrcDirPath,
- pathIsh: line
-})
+getAbsoluteAndInOsFormatPath({
+ cwd: isPublicIteration
+ ? pathJoin(buildContext.publicDirPath, KEYCLOAK_THEME)
+ : buildContext.themeSrcDirPath,
+ pathIsh: line
+})Run the following script to verify if public files are being resolved correctly:
🏁 Script executed:
#!/bin/bash
# Description: Check if there are any .gitignore files in the public directory and analyze their content
# Find .gitignore files in directories containing "public" and "keycloak-theme"
fd -t f ".gitignore" | grep -E "public.*keycloak-theme|keycloak-theme.*public" | head -10
# If any files are found, check their content for file paths
fd -t f ".gitignore" | grep -E "public.*keycloak-theme|keycloak-theme.*public" | head -1 | xargs -I {} sh -c 'echo "=== Content of {} ==="; cat {}'Length of output: 239
🏁 Script executed:
#!/bin/bash
set -e
# Locate the readManagedGitignoresFile function and its location
echo "🔍 Finding definition of readManagedGitignoresFile..."
rg -n "readManagedGitignoresFile" -n src/bin/sync-extensions/managedGitignoreFiles.ts
# Show context around lines 128–189
echo "🔍 Showing lines 120–200 for context..."
sed -n '120,200p' src/bin/sync-extensions/managedGitignoreFiles.ts
# Find all calls to getAbsoluteAndInOsFormatPath within this file
echo "🔍 Inspecting getAbsoluteAndInOsFormatPath usages..."
grep -R "getAbsoluteAndInOsFormatPath" -n src/bin/sync-extensions/managedGitignoreFiles.ts
# Check if buildContext.publicDirPath is referenced here
echo "🔍 Searching for publicDirPath in managedGitignoreFiles.ts..."
rg -n "publicDirPath" -n src/bin/sync-extensions/managedGitignoreFiles.ts
# Look for any iteration flag or variable indicating public vs. non-public
echo "🔍 Searching for isPublicIteration or similar in the repo..."
rg -n "isPublicIteration" -n srcLength of output: 4143
Fix path resolution for public-managed .gitignore entries
The readManagedGitignoresFile helper always uses buildContext.themeSrcDirPath when calling getAbsoluteAndInOsFormatPath and pathRelative, even for public‐scoped files. As a result, any paths extracted from the public .gitignore will be resolved against the theme source directory instead of the public theme directory.
Locations to update:
- File:
src/bin/sync-extensions/managedGitignoreFiles.ts - Around lines 174–178 in the
readManagedGitignoresFilefunction
Suggested diff:
@@ export async function readManagedGitignoresFile(params: {
payload
.split("\n")
.map(line => line.trim())
.map(line => line.replace(/^# /, ""))
.filter(line => line !== "")
- .map(line =>
- getAbsoluteAndInOsFormatPath({
- cwd: buildContext.themeSrcDirPath,
- pathIsh: line
- })
- )
- .map(filePath =>
- pathRelative(buildContext.themeSrcDirPath, filePath)
- )
+ // Use the correct base directory per iteration
+ .map(line => {
+ const cwdBase = isPublicIteration
+ ? pathJoin(buildContext.publicDirPath, KEYCLOAK_THEME)
+ : buildContext.themeSrcDirPath;
+
+ const absPath = getAbsoluteAndInOsFormatPath({
+ cwd: cwdBase,
+ pathIsh: line
+ });
+
+ return pathRelative(cwdBase, absPath);
+ })
.forEach(fileRelativePath =>
ownedFilesRelativePaths.push({
isPublic: isPublicIteration,
fileRelativePath
})
);Don’t forget to import pathJoin and ensure KEYCLOAK_THEME is in scope.
Add a unit or integration test that covers a public .gitignore to confirm correct resolution.
📝 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.
| getAbsoluteAndInOsFormatPath({ | |
| cwd: buildContext.themeSrcDirPath, | |
| pathIsh: line | |
| }) | |
| ) | |
| payload | |
| .split("\n") | |
| .map(line => line.trim()) | |
| .map(line => line.replace(/^# /, "")) | |
| .filter(line => line !== "") | |
| .map(line => { | |
| // Use the correct base directory per iteration | |
| const cwdBase = isPublicIteration | |
| ? pathJoin(buildContext.publicDirPath, KEYCLOAK_THEME) | |
| : buildContext.themeSrcDirPath; | |
| const absPath = getAbsoluteAndInOsFormatPath({ | |
| cwd: cwdBase, | |
| pathIsh: line | |
| }); | |
| return pathRelative(cwdBase, absPath); | |
| }) | |
| .forEach(fileRelativePath => | |
| ownedFilesRelativePaths.push({ | |
| isPublic: isPublicIteration, | |
| fileRelativePath | |
| }) | |
| ); |
🤖 Prompt for AI Agents
In src/bin/sync-extensions/managedGitignoreFiles.ts around lines 174 to 178, the
path resolution for public .gitignore files incorrectly uses
buildContext.themeSrcDirPath as the base directory. To fix this, modify the code
to conditionally use buildContext.publicDirPath when the isPublicIteration flag
is true, and buildContext.themeSrcDirPath otherwise, when calling
getAbsoluteAndInOsFormatPath and pathRelative. Also, ensure pathJoin and
KEYCLOAK_THEME are imported and in scope. Finally, add a test case that verifies
correct path resolution for public .gitignore files.
Summary by CodeRabbit
New Features
Bug Fixes
Chores