docs: expand agent guidelines - #9
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| .action((input: string, options: CompileOptions, command: Command): void => { | ||
| let tempDir: string | null = null; | ||
| try { | ||
| const merged = mergeOptions(input, options); | ||
| const result = compileFile(input, merged); | ||
| if (result.errors.length > 0) return; | ||
|
|
||
| // eslint-disable-next-line no-eval | ||
| eval(result.code); | ||
| tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'somon-run-')); | ||
| const baseName = path.basename(input); | ||
| const jsFileName = baseName.match(/\.som$/i) | ||
| ? baseName.replace(/\.som$/i, '.js') | ||
| : `${baseName}.js`; | ||
| const tempFile = path.join(tempDir, jsFileName); | ||
| fs.writeFileSync(tempFile, result.code, 'utf8'); | ||
|
|
||
| if (merged.sourceMap && result.sourceMap) { | ||
| fs.writeFileSync(`${tempFile}.map`, result.sourceMap, 'utf8'); | ||
| } | ||
|
|
||
| const child = cliRuntime.executeCompiledFile( | ||
| tempFile, | ||
| resolveForwardedArgv(command, input) | ||
| ); |
There was a problem hiding this comment.
[P1] Run command breaks relative imports
The new run implementation writes the compiled code into a temporary directory and then executes that file via spawnSync. Because the generated JS still contains require('./…') statements (the compiler leaves relative imports untouched), Node resolves those requires relative to the temporary directory instead of the original source file’s directory. Any SomonScript program that imports sibling modules now fails at runtime with “Cannot find module …”. Consider executing the temporary file with cwd set to path.dirname(input) or emitting the compiled file alongside the source so relative require paths continue to resolve correctly.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| for (const [moduleId, moduleData] of result.modules) { | ||
| const processedCode = this.rewriteRequiresForModule(moduleId, moduleData.code, context); | ||
| const key = moduleIdMapping.get(moduleId); | ||
| if (!key) { | ||
| continue; | ||
| } | ||
| modules.push({ id: moduleId, key, code: processedCode, map: moduleData.map }); |
There was a problem hiding this comment.
[P1] Keep source maps in sync with rewritten require paths
prepareModulesForBundle rewrites each module’s code before bundling but keeps the original moduleData.map untouched. When generateCommonJSBundle later emits mappings from these maps, it assumes the mappings refer to the rewritten code. Any change in specifier (e.g., normalising require('./dep') to require('dep.som')) shifts column offsets and produces a bundle source map that no longer matches the generated output, breaking debugging and stack traces whenever source maps are requested.
Useful? React with 👍 / 👎.
|
@codex review |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
|
✅ Tests completed on Node.js 20.x: success |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| async function executeBundleCommand(input: string, options: BundleOptions): Promise<void> { | ||
| try { | ||
| const baseDir = path.dirname(path.resolve(input)); | ||
| const config = loadConfig(baseDir); | ||
| const moduleSystem = await createModuleSystem(baseDir, config); | ||
|
|
||
| const bundleOptions = createBundleOptions(input, options, config, baseDir); | ||
| const isProduction = options.production || process.env.NODE_ENV === 'production'; | ||
|
|
||
| if (bundleOptions.format !== 'commonjs' && !bundleOptions.force) { | ||
| console.error('ESM/UMD bundle formats are experimental. Re-run with --force to proceed.'); | ||
| process.exitCode = 1; | ||
| return; | ||
| // Validate production environment if --production flag is set | ||
| if (isProduction) { | ||
| const outputPath = options.output || input.replace(/\.som$/, '.bundle.js'); | ||
|
|
||
| try { | ||
| validateProductionEnvironment(outputPath, [input]); | ||
| } catch (error) { |
There was a problem hiding this comment.
The new production-mode check validates options.output || input.replace(/\.som$/, '.bundle.js') before bundling. createBundleOptions and getBundleOutputPath resolve a relative --output against the input file’s directory, not the current working directory. When the user supplies a relative output path, the validator exercises a different path than the bundler writes to, so a directory can pass validation and still fail during the actual write (or vice versa). The validation should compute the path with the same logic used for emitting the bundle, e.g. by reusing getBundleOutputPath or resolving relative to path.dirname(input) before calling validateProductionEnvironment.
Useful? React with 👍 / 👎.
2 participants
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68d453fb0b20832c8ccf9de8e33e99b9