chore(workflow-workers): update tsconfig.json to remove rootDir and add path mappings #1451
Conversation
…dd path mappings\n\n- Removed the `rootDir` option to simplify the configuration.\n- Added path mappings for shared modules to enhance module resolution.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds new TypeScript path alias "@/shared/*" (and deprecates Changes
Possibly related PRs
Suggested labels
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tsconfig.json (1)
24-25: Consider consolidating to a single alias.Both
@shared/*and@/shared/*map to the same location. Having two aliases for the same path adds maintenance overhead and can cause confusion about which alias to use in new code.If this duplication is temporary for migration purposes, consider adding a comment to clarify the intent and timeline for removing the old alias.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/workers/workflow-workers/tsconfig.jsonlib/neo4j/index.tstsconfig.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx,mts,mjs}
📄 CodeRabbit inference engine (.cursor/rules/code-structure.mdc)
Avoid barrel files: do not create files whose sole purpose is to re-export symbols from multiple modules; prefer importing from original module paths to maintain clear dependency graphs and better tree-shaking
Files:
lib/neo4j/index.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/neo4j/convert.ts:69-74
Timestamp: 2025-09-22T09:34:43.580Z
Learning: The file lib/neo4j/convert.ts is being moved away from/deprecated, so fixes in this file may not be worth implementing since it will be replaced.
Learnt from: CR
Repo: youngchingjui/issue-to-pr PR: 0
File: .cursor/rules/code-structure.mdc:0-0
Timestamp: 2025-09-04T14:57:04.612Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mts,mjs} : Avoid barrel files: do not create files whose sole purpose is to re-export symbols from multiple modules; prefer importing from original module paths to maintain clear dependency graphs and better tree-shaking
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/neo4j/services/event.ts:365-370
Timestamp: 2025-09-22T09:33:51.194Z
Learning: The file lib/neo4j/services/event.ts is planned for migration/deprecation, so minor fixes may not be worth implementing since the file will be replaced.
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/adapters/PersistingEventBusAdapter.ts:55-58
Timestamp: 2025-09-22T09:28:41.886Z
Learning: The project is migrating away from neo4j/services/event adapters, so issues in files like lib/adapters/PersistingEventBusAdapter.ts that use these adapters may be temporary and lower priority.
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: shared/src/ports/events/publisher.ts:16-19
Timestamp: 2025-09-22T09:24:26.840Z
Learning: youngchingjui prefers to manage PR scope tightly and defer technical debt improvements to future PRs rather than expanding the current PR's scope, even for related consistency issues.
📚 Learning: 2025-09-04T14:57:04.612Z
Learnt from: CR
Repo: youngchingjui/issue-to-pr PR: 0
File: .cursor/rules/code-structure.mdc:0-0
Timestamp: 2025-09-04T14:57:04.612Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mts,mjs} : Avoid barrel files: do not create files whose sole purpose is to re-export symbols from multiple modules; prefer importing from original module paths to maintain clear dependency graphs and better tree-shaking
Applied to files:
tsconfig.jsonapps/workers/workflow-workers/tsconfig.json
📚 Learning: 2025-09-22T09:34:43.580Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/neo4j/convert.ts:69-74
Timestamp: 2025-09-22T09:34:43.580Z
Learning: The file lib/neo4j/convert.ts is being moved away from/deprecated, so fixes in this file may not be worth implementing since it will be replaced.
Applied to files:
lib/neo4j/index.ts
📚 Learning: 2025-09-22T09:33:51.194Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/neo4j/services/event.ts:365-370
Timestamp: 2025-09-22T09:33:51.194Z
Learning: The file lib/neo4j/services/event.ts is planned for migration/deprecation, so minor fixes may not be worth implementing since the file will be replaced.
Applied to files:
lib/neo4j/index.ts
📚 Learning: 2025-09-22T09:28:41.886Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/adapters/PersistingEventBusAdapter.ts:55-58
Timestamp: 2025-09-22T09:28:41.886Z
Learning: The project is migrating away from neo4j/services/event adapters, so issues in files like lib/adapters/PersistingEventBusAdapter.ts that use these adapters may be temporary and lower priority.
Applied to files:
lib/neo4j/index.ts
🔇 Additional comments (3)
lib/neo4j/index.ts (1)
12-15: LGTM! Proper use of type imports.The import statement correctly uses the
typekeyword for the type-only import, which helps TypeScript optimize compilation and enables better tree-shaking. The new path alias should resolve correctly with the updated tsconfig.apps/workers/workflow-workers/tsconfig.json (2)
15-17: No@shared/*imports found in workflow-workers.Verified that workflow-workers does not use the
@shared/*alias. The root tsconfig.json includes both@shared/*and@/shared/*aliases, while workflow-workers tsconfig.json defines only@/shared/*. Since no code in workflow-workers uses@shared/*, there are no resolution failures. However, consider aligning the configuration by either adding@shared/*to workflow-workers tsconfig.json for consistency with the root configuration, or documenting why@/shared/*is the intended alias.
15-17: Path resolution is correctly configured through inherited baseUrl.The nested tsconfig extends
tsconfig.base.json, which explicitly setsbaseUrl: "."at the monorepo root. Since the nested tsconfig does not override this value, it inherits the root-level baseUrl. Therefore, the relative path mapping"@/shared/*": ["shared/src/*"]resolves correctly to the monorepo root'sshared/src/directory regardless of the nested location. No changes needed.
…nt loading\n\n- Changed the main entry point in Dockerfile and package.json to the new path for consistency.\n- Updated the environment loading logic in helper.ts to use process.cwd() for better compatibility with nested directory structures.\n- Adjusted import paths in index.ts to align with the new module resolution strategy.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/workers/workflow-workers/src/helper.ts (1)
16-18: Document the critical assumption about startup directory.The change from
fileURLToPath(import.meta.url)toprocess.cwd()makes env loading dependent on the process startup directory rather than the file location. This assumes the worker is always started from the repository root.While this simplifies the logic and works with different dist structures, it's more fragile than the previous approach. If someone runs
node dist/apps/workers/workflow-workers/src/index.jsfrom a subdirectory, env files won't be found.Consider adding a startup validation check that verifies expected files exist:
🔎 Proposed validation check
function loadEnvFromWorkerRoot(): void { if (envLoaded) return // Use process.cwd() as the worker root, which is where the process is started from // This works correctly whether the dist structure is flat or nested const workerRoot = process.cwd() + + // Validate we're starting from the expected directory + const packageJsonPath = path.join(workerRoot, 'package.json') + if (!fs.existsSync(packageJsonPath)) { + throw new Error( + `Worker must be started from repository root. Current directory: ${workerRoot}` + ) + } const nodeEnv = process.env.NODE_ENV ?? "development"
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/workers/workflow-workers/Dockerfileapps/workers/workflow-workers/package.jsonapps/workers/workflow-workers/src/helper.tsapps/workers/workflow-workers/src/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx,mts,mjs}
📄 CodeRabbit inference engine (.cursor/rules/code-structure.mdc)
Avoid barrel files: do not create files whose sole purpose is to re-export symbols from multiple modules; prefer importing from original module paths to maintain clear dependency graphs and better tree-shaking
Files:
apps/workers/workflow-workers/src/index.tsapps/workers/workflow-workers/src/helper.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: shared/src/ports/events/publisher.ts:16-19
Timestamp: 2025-09-22T09:24:26.840Z
Learning: youngchingjui prefers to manage PR scope tightly and defer technical debt improvements to future PRs rather than expanding the current PR's scope, even for related consistency issues.
📚 Learning: 2025-09-22T09:32:04.764Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/types/db/neo4j.ts:89-91
Timestamp: 2025-09-22T09:32:04.764Z
Learning: The new WorkflowStateEventSchema in shared/src/entities/events/WorkflowEvent.ts is built from BaseFields (id, timestamp) and does not contain a workflowId field, so DB-level schemas should not attempt to omit workflowId when using this schema.
Applied to files:
apps/workers/workflow-workers/src/index.ts
📚 Learning: 2025-09-22T09:32:04.764Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/types/db/neo4j.ts:89-91
Timestamp: 2025-09-22T09:32:04.764Z
Learning: The new WorkflowStateEventSchema in shared/src/entities/events/WorkflowEvent.ts does not contain a workflowId field, so DB-level schemas should not attempt to omit it when using this schema.
Applied to files:
apps/workers/workflow-workers/src/index.ts
📚 Learning: 2025-09-26T03:43:36.162Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1297
File: apps/workers/workflow-workers/src/orchestrators/simulateLongRunningWorkflow.ts:8-13
Timestamp: 2025-09-26T03:43:36.162Z
Learning: For workflow workers, Redis client connections should be established once at worker startup using a singleton pattern and shared across helper functions, rather than creating new connections for each function invocation to avoid connection leaks.
Applied to files:
apps/workers/workflow-workers/src/index.ts
🔇 Additional comments (1)
apps/workers/workflow-workers/Dockerfile (1)
57-57: LGTM! Entry point update aligns with package.json changes.The CMD update correctly reflects the new nested dist structure. In the Docker context:
WORKDIRis/app(line 32), soprocess.cwd()returns/apphelper.tswill look for .env files at/app/.env*- Built artifacts are at
/app/dist/apps/workers/workflow-workers/src/index.jsNote: .env files aren't explicitly copied in the Dockerfile, which is typical for production deployments that use environment variables. The
dotenv.config()calls inhelper.tsgracefully handle missing .env files.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
shared/tsconfig.json (1)
11-12: Clarify the migration path from@shared/*to@/shared/*.The duplicate path aliases are an intentional migration pattern—
__tests__/tsconfig.jsonalready includes a deprecation comment directing to the new@/shared/*alias. Since@shared/*is still actively used throughout the codebase while@/shared/*is not yet adopted, consider adding similar deprecation comments to other tsconfig files (root, shared) to document the transition timeline and reduce confusion for contributors.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shared/tsconfig.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: shared/src/ports/events/publisher.ts:16-19
Timestamp: 2025-09-22T09:24:26.840Z
Learning: youngchingjui prefers to manage PR scope tightly and defer technical debt improvements to future PRs rather than expanding the current PR's scope, even for related consistency issues.
📚 Learning: 2025-09-04T14:57:04.612Z
Learnt from: CR
Repo: youngchingjui/issue-to-pr PR: 0
File: .cursor/rules/code-structure.mdc:0-0
Timestamp: 2025-09-04T14:57:04.612Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mts,mjs} : Avoid barrel files: do not create files whose sole purpose is to re-export symbols from multiple modules; prefer importing from original module paths to maintain clear dependency graphs and better tree-shaking
Applied to files:
shared/tsconfig.json
…e import paths\n\n- Added detailed comments explaining the environment loading strategy for different deployment scenarios.\n- Updated import paths to use aliasing for better module resolution consistency.
youngchingjui
deleted the
chore/workflow-workers-tsconfig-path-mappings
branch
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/workers/workflow-workers/src/helper.tsapps/workers/workflow-workers/src/index.tsshared/src/adapters/ioredis/client.tsshared/tsconfig.jsontsconfig.json
✅ Files skipped from review due to trivial changes (1)
- shared/src/adapters/ioredis/client.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/workers/workflow-workers/src/index.ts
- apps/workers/workflow-workers/src/helper.ts
- shared/tsconfig.json
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: shared/src/ports/events/publisher.ts:16-19
Timestamp: 2025-09-22T09:24:26.840Z
Learning: youngchingjui prefers to manage PR scope tightly and defer technical debt improvements to future PRs rather than expanding the current PR's scope, even for related consistency issues.
📚 Learning: 2025-09-22T09:34:43.580Z
Learnt from: youngchingjui
Repo: youngchingjui/issue-to-pr PR: 1282
File: lib/neo4j/convert.ts:69-74
Timestamp: 2025-09-22T09:34:43.580Z
Learning: The file lib/neo4j/convert.ts is being moved away from/deprecated, so fixes in this file may not be worth implementing since it will be replaced.
Applied to files:
tsconfig.json
📚 Learning: 2025-09-04T14:57:04.612Z
Learnt from: CR
Repo: youngchingjui/issue-to-pr PR: 0
File: .cursor/rules/code-structure.mdc:0-0
Timestamp: 2025-09-04T14:57:04.612Z
Learning: Applies to **/*.{ts,tsx,js,jsx,mts,mjs} : Avoid barrel files: do not create files whose sole purpose is to re-export symbols from multiple modules; prefer importing from original module paths to maintain clear dependency graphs and better tree-shaking
Applied to files:
tsconfig.json
| "@shared/*": ["shared/src/*"], // @deprecated, migrate to using "@/shared/*" instead | ||
| "@/shared/*": ["shared/src/*"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the tsconfig.json file to understand the context
cat -n tsconfig.json | head -30Repository: youngchingjui/issue-to-pr
Length of output: 1096
🏁 Script executed:
# Search for any remaining usages of the deprecated "@shared/*" alias
rg -n '@shared/' --type=ts --type=js -g '!node_modules/**' -g '!**/dist/**' -g '!**/*.d.ts' 2>/dev/null | head -20Repository: youngchingjui/issue-to-pr
Length of output: 2214
🏁 Script executed:
# Also check for dynamic imports with the deprecated alias
rg -n "import\(['\"]\@shared" --type=ts --type=js -g '!node_modules/**' -g '!**/dist/**' 2>/dev/nullRepository: youngchingjui/issue-to-pr
Length of output: 51
🏁 Script executed:
# Check if there are any other files that might reference this
rg -l '@shared/' --type=ts --type=js --type=json -g '!node_modules/**' -g '!**/dist/**' 2>/dev/null | head -20Repository: youngchingjui/issue-to-pr
Length of output: 760
Migration from "@shared/" to "@/shared/" is incomplete and must be completed before this change can be merged.
The new "@/shared/" alias follows the consistent naming pattern with other path mappings. However, the codebase still contains numerous active imports using the deprecated "@shared/" alias that must be migrated:
shared/src/ui/button.tsx,shared/src/ui/alert.tsxlib/workflows/createDependentPR.ts,lib/workflows/createDependentPR.formatMessage.tslib/agents/DependentPRAgent.tslib/github/auth.ts,lib/github/users.tsshared/src/adapters/github/octokit/graphql/pullRequest.reader.ts,pullRequest.writer.tsshared/src/lib/tools/UpdatePRTool.ts__tests__/shared/usecases/generateBranchName.test.ts- And additional files
Update all imports to use "@/shared/" instead of "@shared/" before merging this change.
🤖 Prompt for AI Agents
In tsconfig.json around lines 24 to 25, the deprecated "@shared/*" path alias
remains and many files still import using it; update all imports to use the new
"@/shared/*" alias: search the codebase for occurrences of "@shared/" (including
the listed files: shared/src/ui/button.tsx, shared/src/ui/alert.tsx,
lib/workflows/createDependentPR.ts,
lib/workflows/createDependentPR.formatMessage.ts,
lib/agents/DependentPRAgent.ts, lib/github/auth.ts, lib/github/users.ts,
shared/src/adapters/github/octokit/pullRequest.reader.ts, pullRequest.writer.ts,
shared/src/lib/tools/UpdatePRTool.ts,
__tests__/shared/usecases/generateBranchName.test.ts, etc.) and replace them
with "@/shared/"; update any import typings or relative paths if necessary,
remove the deprecated comment in tsconfig or keep but mark migrated, run the
TypeScript build and tests to catch remaining references and fix any resulting
import path errors before merging.
Summary
rootDiroption to simplify the configuration@/shared/*→shared/src/*) to enhance module resolutionContext
This change was extracted from a larger Neo4j feature PR to be tested separately, as it could have wide-ranging effects on the monorepo build process.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.