- Notifications
You must be signed in to change notification settings - Fork3.1k
add iologger for debugging purposes#15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
A PR to add an IOLogger for logging stdin and stdout between the client and server for debugging purposes.
- Introduces an IOLogger struct with Read and Write methods that log data.
- Adds tests in pkg/log/io_test.go to verify logged IO behavior.
- Updates the server startup in cmd/server/main.go to conditionally wrap standard IO with the IOLogger based on a new command flag.
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/log/io.go | Introduces the IOLogger implementation and its logging behavior on Read and Write. |
| pkg/log/io_test.go | Provides tests to verify logged input and output functionality. |
| cmd/server/main.go | Modifies server initialization to retrieve and pass a command logging flag and binds a new flag. |
Files not reviewed (1)
- go.mod: Language not supported
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused.Learn more
Uh oh!
There was an error while loading.Please reload this page.
SamMorrowDrums left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I can see how this would save a lot of pain when iterating on things.
e0bffe9 intomainUh oh!
There was an error while loading.Please reload this page.
add iologger for debugging purposes
gelu22 commentedSep 12, 2025
Progress Update - Phase 2 Completed ✅🎯Major Achievement: Endpoint Coverage ImprovementSuccessfully resolved missing API endpoints causing 404 errors: ✅Created new routes:
📊Test Results AnalysisBefore (Phase 1): ~21% success rate (majority schema errors) Current comprehensive test status:
🔍Current Focus: Authentication & AuthorizationPrimary issues now:
Pattern identified: Tests failing on authentication flow rather than missing endpoints - this is progress! 🚀Next Steps - Phase 3: Authentication SystemImmediate priorities:
Target: Achieve 80%+ comprehensive test success rate 💡Key Insights
Status: Phase 2 Complete ✅ | Phase 3 Authentication Debugging In Progress 🔄 |
Uh oh!
There was an error while loading.Please reload this page.
Context
This PR introduces an
IOLoggerstruct that logs allstdinandstdoutinteractions between the client and the server.This feature is particularly useful for debugging, as it provides visibility into the exact input and output exchanged between the client and the server.
By default, logging is disabled but can be enabled by setting the
enable-command-loggingflag totrue. I considered enabling it only when the log level is set todebug, but decided that having a separate toggle would be preferable due to potential privacy concerns.