Standardize Codex manager logging and fix documentation typos#1060
Standardize Codex manager logging and fix documentation typos#1060Ivorisnoob wants to merge 3 commits intopingdotgg:mainfrom
Conversation
- Migrate console logs to Effect.log in apps/server/src/codexAppServerManager.ts - Fix grammar and typos in AGENTS.md - Standardize on Effect-based logging for better persistence and scoping
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
github-actions
bot
added
size:S
| ## Maintainability | ||
|
|
||
| Long term maintainability is a core priority. If you add new functionality, first check if there are shared logic that can be extracted to a separate module. Duplicate logic across mulitple files is a code smell and should be avoided. Don't be afraid to change existing code. Don't take shortcuts by just adding local logic to solve a problem. | ||
| Long term maintainability is a core priority. If you add new functionality, first check if there is shared logic that can be extracted to a separate module. Duplicate logic across multiple files is a code smell and should be avoided. Don't be afraid to change existing code. Don't take shortcuts by just adding local logic to solve a problem. |
There was a problem hiding this comment.
What does this get to do with the PR?
There was a problem hiding this comment.
What does this get to do with the PR?
Sorry for the noise, Those were just small typos I spotted in the docs while working on this. They aren't related to the main logic, just felt like a quick win to fix them here
There was a problem hiding this comment.
You can leave it as is. I am just a contributor trying to help the maintainers; they should have the final say in that. But I agree with you that it is a small change :)
There was a problem hiding this comment.
Appreciate it, I'll leave it in as-is and see what they say. Thanks for the help
|
I know its silly seeing how small they are... but can we have a PR for each please? |
|
Both changes, Typo and Codex logging one. Thanks for the help I will make in a while |
|
Closing this, made it into 2 Different PRs as per @binbandit |
3 participants
What Changed
consoletoEffect.log.Why
consoleare not captured by the project's rotating file sink. This change ensures critical provider events are persisted to disk.UI Changes
N/A
Checklist
Note
Standardize
CodexAppServerManager.startSessionlogging to use Effect-based log callsconsole.log/console.errorcalls instartSessionwithEffect.logInfo/Effect.logWarningexecuted viarunPromise, matching the logging style used elsewhere in the manager.causefield extracted fromError.messageorString(error).Macroscope summarized ca526c0.