fix(auth): route opaque-token debug through process.logger - #263
Conversation
… console.debug console.debug leaks to stdout in CI and can't be silenced via the configured logger. Route the 'token is not a decodable JWT' message through process.logger?.debug?.() so it respects the log level and is a no-op when no logger is installed. - Add process.d.ts type augmentation for process.logger - Enable ts-node files:true so .d.ts is loaded during build scripts - Add tests: logger routing + no-op when logger unset
![4gl-reviewer[bot]](https://avatars.githubusercontent.com/in/4587573?s=60&v=4){kind=small}
| // Route through process.logger so the message respects the configured | ||
| // log level and doesn't pollute stdout in CI/test runs. Falls back to a | ||
| // no-op when no logger has been installed (e.g. library-only usage). | ||
| process.logger?.debug?.( |
There was a problem hiding this comment.
Optional chaining on .debug is harmless and arguably good defensive practice (protects against a test/library stubbing process.logger = {}), but note it's technically redundant: a real Logger instance always defines debug as an own arrow-function property. Not requesting a change — just flagging in case the intent was to guard something else.
There was a problem hiding this comment.
Hermes Agent Code Review
Verdict: ✅ APPROVE
Small, focused fix that correctly routes the opaque-token debug line away from console.debug (which bypassed the configured log level and polluted stdout in CI/test runs) through process.logger?.debug?.().
Looks Good
- The
process.logger?.guard cleanly no-ops when no logger is installed (library-only usage), and this case is explicitly tested (auth.spec.tslines 75–95). src/types/system/process.d.tscorrectly augmentsNodeJS.Processwith an optionallogger, and the relative import path (../../logger/logger) resolves correctly. Picked up viatypeRootsintsconfig.json.- The
ts-node: { files: true }addition is the right fix so ts-node loads the ambient.d.tsaugmentation (ts-node otherwise ignores declaration files unlessfiles: true). - Tests are thorough: both the logger-present case (asserts the message hits
logger.debugand notconsole.debug) and the logger-absent case (assertsconsole.debugis not called) are covered, withprocess.loggerrestored infinallyblocks. - All 24 tests in
auth.spec.tspass locally.
Suggestions (non-blocking)
- The second optional chain in
process.logger?.debug?.()is redundant for a realLoggerinstance (debugis always defined as an own property) but harmless as defense-in-depth. See inline comment.
No security, correctness, or performance concerns.
Reviewed by Hermes Agent (GitHub App)
|
🎉 This PR is included in version 3.6.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
## [3.6.2](v3.6.1...v3.6.2) (2026-08-19) ### Bug Fixes * **auth:** remove unnecessary opaque-token debug message ([011eb7e](011eb7e)), closes [#263](#263)
1 participant
Addresses review feedback: isTokenExpiring was printing console.debug for every opaque (non-JWT) refresh-token check, leaking to stdout in CI with no way to silence it via the configured logger. Routes through process.logger?.debug?.() instead. Adds type augmentation, ts-node config, and two tests.