Conversation
| const [objects, serverlessFunctions, application, sources] = | ||
| await Promise.all([ | ||
| Promise.resolve(collectObjects(program)), | ||
| Promise.resolve(collectServerlessFunctions(program)), | ||
| Promise.resolve(collectServerlessFunctions(program, appPath)), | ||
| Promise.resolve(extractTwentyAppConfig(program)), | ||
| loadFolderContentIntoJson(appPath), |
There was a problem hiding this comment.
Bug: appPath is not resolved to an absolute path before being used with TypeScript compiler APIs.
Severity: CRITICAL | Confidence: 0.95
🔍 Detailed Analysis
The appPath variable is not resolved to an absolute path before being passed to getProgramFromTsconfig in loadManifest. This occurs when appPath is provided as a relative path, for example, via options.appPath. Consequently, TypeScript's parseJsonConfigFileContent may incorrectly resolve paths, and relative(appPath, fileName) can produce incorrect results, leading to miscalculated serverless handler paths.
💡 Suggested Fix
Ensure appPath is always an absolute path by using resolve(path ?? process.cwd()) before passing it to getProgramFromTsconfig.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/twenty-cli/src/utils/load-manifest.ts#L515-L520
Potential issue: The `appPath` variable is not resolved to an absolute path before being
passed to `getProgramFromTsconfig` in `loadManifest`. This occurs when `appPath` is
provided as a relative path, for example, via `options.appPath`. Consequently,
TypeScript's `parseJsonConfigFileContent` may incorrectly resolve paths, and
`relative(appPath, fileName)` can produce incorrect results, leading to miscalculated
serverless handler paths.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Fixed incorrect handlerPath generation for serverless functions by passing appPath parameter through the call chain instead of relying on process.cwd().
Key Changes:
- Modified
getProgramFromTsconfig()to requireappPathparameter (removed optional) - Updated
posixRelativeFromCwd()to accept bothfileNameandappPathparameters - Updated
collectServerlessFunctions()to accept and passappPathtoposixRelativeFromCwd() - Fixed parameter passing in
loadManifest()to ensureappPathflows through correctly
Why This Matters:
The CLI can be invoked from different directories (via CURRENT_EXECUTION_DIRECTORY which uses process.env.INIT_CWD or process.cwd()), but the previous implementation always calculated relative paths from process.cwd() instead of the actual appPath. This could generate incorrect handlerPath values in the manifest when the CLI working directory differs from the app directory, causing serverless function deployment failures.
Confidence Score: 5/5
- This PR is safe to merge with no risk - it's a targeted bug fix with clear logic improvements
- The fix addresses a clear bug where relative paths were calculated from the wrong base directory. The changes are well-scoped, maintain backward compatibility (existing tests pass), improve parameter type safety (removed optional
appPath), and follow the codebase pattern of explicitly passing context through the call chain. No breaking changes or side effects expected. - No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| packages/twenty-cli/src/utils/load-manifest.ts | 5/5 | Fixed handlerPath calculation to use appPath instead of process.cwd(), ensuring correct relative paths when CLI is invoked from different directories |
Sequence Diagram
sequenceDiagram
participant CLI as CLI Command
participant LM as loadManifest()
participant GPT as getProgramFromTsconfig()
participant CSF as collectServerlessFunctions()
participant PRC as posixRelativeFromCwd()
CLI->>LM: loadManifest(appPath)
Note over LM: appPath = /user/project
LM->>GPT: getProgramFromTsconfig(appPath)
GPT-->>LM: TypeScript Program
LM->>CSF: collectServerlessFunctions(program, appPath)
loop For each source file
CSF->>CSF: findHandlerAndConfig(sf)
Note over CSF: sf.fileName = /user/project/src/hello.ts
CSF->>PRC: posixRelativeFromCwd(sf.fileName, appPath)
Note over PRC: Before fix: used process.cwd()<br/>After fix: uses appPath parameter
PRC->>PRC: relative(appPath, sf.fileName)
Note over PRC: Returns: src/hello.ts
PRC-->>CSF: "src/hello.ts"
CSF->>CSF: Push to serverlessFunctions array
end
CSF-->>LM: serverlessFunctions with correct handlerPath
LM-->>CLI: Manifest with correct paths
2 files reviewed, no comments
as title
After a sync hellp-world from twenty: