Allowing workflows in the root of workflows folder #20
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #20 +/- ##
==========================================
+ Coverage 53.11% 53.25% +0.13%
==========================================
Files 15 15
Lines 1702 1707 +5
==========================================
+ Hits 904 909 +5
Misses 709 709
Partials 89 89
|
WalkthroughThe pull request introduces a refactoring of the workflow discovery logic in the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
testData/kindeSrc/environment/workflows/rootWorkflow.ts (1)
36-36: Fix typo in log object key.The log object contains a typo in the key: "balh" should be "blah".
- console.log('logging from action', {"balh": "blah"}); + console.log('logging from action', {"blah": "blah"});projectBundler/projectBundler.go (1)
68-68: LGTM! Consider improving error handling.The refactoring to use
maybeAddWorkflowfor both root-level and subdirectory workflows is a good improvement that reduces code duplication. However, the error handling could be enhanced.Consider handling potential errors from
os.ReadDirinstead of discarding them:- files, _ := os.ReadDir(workflowsPath) + files, err := os.ReadDir(workflowsPath) + if err != nil { + log.Warn().Err(err).Msgf("error reading directory: %s", workflowsPath) + continue + }Also applies to: 71-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
projectBundler/projectBundler.go(1 hunks)projectBundler/projectBundler_test.go(1 hunks)runtime_test.go(1 hunks)testData/kindeSrc/environment/workflows/rootWorkflow.ts(1 hunks)
🔇 Additional comments (4)
projectBundler/projectBundler_test.go (1)
27-27: LGTM! The assertion is updated to reflect the new workflow count.
The change correctly updates the expected number of workflows from 2 to 3, matching the new functionality that allows workflows in the root folder.
testData/kindeSrc/environment/workflows/rootWorkflow.ts (1)
1-16: Consider renaming the workflow ID to better reflect its purpose.
The workflow ID 'tokenGen' suggests token generation functionality, but the implementation appears to be more of a test/demo workflow that performs various operations. Consider a more appropriate name like 'demoWorkflow' or 'testWorkflow'.
Verify the security implications of resetting claims.
Both token services have resetClaims: true. This will clear all existing claims before setting new ones, which might not be the intended behavior in a production environment.
✅ Verification successful
Setting resetClaims: true is consistent with test workflow patterns
The usage of resetClaims: true in this workflow aligns with the established patterns in other test workflows (tokensWorkflow.ts and authWorkflow.ts). All test workflows consistently:
- Set
resetClaims: truefor both ID and access tokens - Use similar test claim modifications via
setCustomClaim
Since this is in a test data directory and follows the same testing patterns as other workflow files, the security concern about resetting claims is not applicable.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other workflows that modify token claims to understand the common patterns
rg -l "setCustomClaim|resetClaims" --type ts
Length of output: 231
Script:
#!/bin/bash
# Check the usage of resetClaims in other workflow files
rg "resetClaims" -C 5 --type ts
Length of output: 3405
Script:
#!/bin/bash
# Check for token claim modifications in other workflow files
rg "setCustomClaim" -C 5 --type ts
Length of output: 3536
projectBundler/projectBundler.go (1)
68-87: Verify workflow discovery behavior.
Let's verify that the workflow discovery works correctly for both root and nested workflows.
✅ Verification successful
Workflow discovery implementation is working as expected
The verification shows that:
- The code correctly handles both root-level and nested workflow files (found in
testData/kindeSrc/environment/workflows/) - Proper discovery of workflow files with
.tsextension (e.g.,rootWorkflow.ts,tokensWorkflow.ts,authWorkflow.ts) - The implementation correctly traverses directories and processes workflows in subdirectories (
evTest/andevTest2/) - File extension checking is implemented correctly in
maybeAddWorkflowfor both.tsand.jsfiles
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify workflow discovery behavior
# Test 1: Check for workflow files in both root and subdirectories
echo "Checking workflow file locations..."
fd -e ts -e js . | grep -E 'workflow\.(ts|js)$'
# Test 2: Verify the structure matches the expected pattern
echo "Checking directory structure..."
tree environment/workflows 2>/dev/null || find environment/workflows -type f 2>/dev/null
# Test 3: Look for any potential duplicate workflow processing
echo "Checking for potential duplicate workflow definitions..."
rg -l "workflow\.(ts|js)'" | xargs -I {} dirname {} | sort | uniq -c | sort -nr
Length of output: 548
Script:
#!/bin/bash
# Let's try a different approach to verify workflow discovery
# Check for any workflow-related patterns in the codebase
echo "Searching for workflow patterns in the codebase..."
rg -l "workflow\.(ts|js)" --type ts --type js
# Look for actual workflow files
echo -e "\nListing all .ts and .js files:"
fd -e ts -e js .
# Check the actual implementation of workflow discovery
echo -e "\nChecking workflow discovery implementation:"
rg -A 5 -B 5 "maybeAddWorkflow" .
Length of output: 2053
runtime_test.go (1)
146-149: LGTM! Test updates correctly verify root workflow support.
The test assertions have been properly updated to verify the discovery of an additional workflow, maintaining consistency with the existing error checking pattern.
Let's verify the test data setup:
✅ Verification successful
The test data structure shows a rootWorkflow.ts file in the root of the workflows directory, along with other workflow files in subdirectories evTest and evTest2. This aligns with the test's expectation of 3 workflows. Let me verify the content of these files to ensure they are valid workflow files.
Test assertions correctly match the test data structure
The test data setup is valid and properly structured:
- Root workflow (
rootWorkflow.ts) is a complete workflow file with proper exports and handler function - The test's expectation of 3 workflows aligns with the directory structure:
- Workflows in
evTest/directory - Workflows in
evTest2/directory - Root workflow in the workflows directory root
- Workflows in
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test data structure includes a workflow in the root folder
# Expected: Find workflow files in both subdirectories and root of workflows folder
echo "Checking workflow files structure:"
fd -t f . testData/kindeSrc/environment/workflows
Length of output: 428
Script:
#!/bin/bash
# Check content of rootWorkflow.ts and a sample workflow from subdirectory
echo "=== Root Workflow Content ==="
cat testData/kindeSrc/environment/workflows/rootWorkflow.ts
echo -e "\n=== Sample Subdirectory Workflow Content ==="
cat testData/kindeSrc/environment/workflows/evTest/hello.ts
Length of output: 1551
evgenyk
dismissed
coderabbitai[bot]’s stale review
this is internal method, which modifies the state via side-effect, no need to return
Allowing workflows in the root of workflows folder
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.