feat: add --debug global flag for comprehensive debugging#477
Closed
heyumeng154-alt wants to merge 11 commits intolarksuite:mainfrom
Closed
feat: add --debug global flag for comprehensive debugging#477heyumeng154-alt wants to merge 11 commits intolarksuite:mainfrom
heyumeng154-alt wants to merge 11 commits intolarksuite:mainfrom
Conversation
- Design spec for global --debug parameter with file output and env var support - Comprehensive test plan covering unit tests, E2E scenarios, and error cases - Includes DebugLogger architecture, integration points, and security considerations Change-Id: I2ffe909878e1856cc5a88f367ab331cc529e544d Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Initialize debug logger from flags and environment variables - Format logs with timestamp, module, level - Mask sensitive data (tokens, API keys, passwords) - Support stderr and file output with proper permissions - Thread-safe writes using sync.Mutex Change-Id: Iba7b1c2401181518f8f6e3d873500d5c0d09d3a1 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Change-Id: Ic073db8a7b432512c55edef091a31a4d032ce9d7
- Fix timestamp format from '2006-01-02T15:04:05.000Z07:00' to '2006-01-02T15:04:05.000Z' - Implement response body truncation in Log() method (>5KB) - Apply truncateResponse() after masking sensitive data Change-Id: I2cd6bf4d06fba02a9220358817a1ec42b2f1397b Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Tests for initialization with various flag combinations - Tests for log formatting and sensitive data masking - Tests for response truncation (>5KB) - Tests for file permission handling - Tests for concurrent safety (race detection) - Tests for logger lifecycle (singleton, Close) All tests pass with -race flag. Change-Id: Ifc139c359cb9d1c1a7e6ca8ae2eae7ff971ef883 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Add Debug bool field for --debug flag - Add DebugFile string field for --debug-file <path> flag - Register flags in RegisterGlobalFlags Change-Id: I78df7557193f190871fdfa65b2dde1f29e6cef12 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Check --debug flag and LARK_CLI_DEBUG environment variable - Initialize debug logger with flags and file path - Defer Close() to flush logs on exit - Gracefully handle logger initialization errors Change-Id: I58a048256251f2dc00c07a0693244721ea821ad3 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Log HTTP method, URL, and query parameters - Log request body (sanitized by debug logger) - Log response status code - All logging only emitted when debug is enabled Change-Id: I0d2e567176b46656a63a71912ed29b3ea56f0b64 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Log when identity is forced by strict mode - Log when identity comes from --as flag - Log auto-detection steps - Log default identity from configuration - Logging only emitted when debug is enabled Change-Id: Ia32594a8d69677026d7759552d5f8c9b518730da Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
|
github-actions
bot
added
the
size/L
📝 WalkthroughWalkthroughAdds a global Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (cmd/root)
participant Logger as DebugLogger (internal/debug)
participant Client as API Client (internal/client)
participant SDK as HTTP SDK
CLI->>Logger: Initialize(enabled, debugFile)
Note right of Logger: open file (0600) or fallback to stderr
CLI->>Client: Execute request(...)
Client->>Logger: Debug("request", method/path/body...)
Client->>SDK: Do HTTP request
SDK-->>Client: HTTP response
Client->>Logger: Debug("response", status, truncated body...)
CLI->>Logger: Close()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
internal/debug/logger_test.go (1)
149-163: Assert the emitted line format here.Right now this only proves the logger does not panic. Timestamp, module, and level regressions would still pass, so the format contract is not actually protected. A temp debug file plus a regex check would make this test pull its weight.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/debug/logger_test.go` around lines 149 - 163, The test TestLogMessageFormat currently only ensures the logger doesn't panic; update it to capture and assert the actual emitted line format by redirecting log output to a temporary file or buffer, invoking Initialize(true,"") and GetLogger(), calling logger.Debug and logger.Error, flushing/closing via Close(), then reading the captured output and matching each line against a regex that enforces timestamp, module name and level (e.g. timestamp pattern + module "test_module" + level "DEBUG"/"ERROR" + message). Use the existing symbols (Initialize, GetLogger, logger.Debug, logger.Error, Close, globalLogger/loggerOnce reset) to locate and modify the test and ensure the regex assertions replace the current no-op verification so format regressions fail the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/root.go`:
- Around line 116-121: The debug logger is being initialized before CLI flags
are parsed so globals.Debug and globals.DebugFile reflect defaults; move the
debug.Initialize(...) / defer debug.Close() call to run after flag parsing
(e.g., immediately after rootCmd.Execute() returns or inside a post-parse hook)
or alternatively parse the debug flags inside BootstrapInvocationContext so that
globals.Debug/globals.DebugFile are set before calling debug.Initialize; update
references to debug.Initialize and debug.Close accordingly and ensure any
bootstrap code that needs early debug is adjusted to use the parsed values.
In `@docs/superpowers/specs/2026-04-14-debug-flag-design.md`:
- Around line 74-80: The spec incorrectly initializes the debug logger using
globals.Debug before CLI flags are parsed; update the design to initialize debug
only after parsed values are available by either (a) moving the call to
debug.Initialize(debugEnabled, globals.DebugFile) to run after rootCmd.Execute()
(or after flag parsing), or (b) invoking it from a pre-run hook or
bootstrap/invocation context that supplies the already-parsed debug flag; ensure
references to globals.Debug, debug.Initialize, debug.Close and rootCmd.Execute
(or the chosen pre-run hook) are used so the implementation reads the final
parsed/debug setting rather than the pre-parse default.
- Around line 26-31: Update the three fenced code blocks in the spec that show
the directory tree ("internal/debug/ ... logger_test.go"), the single-line
timestamped log example ("[2026-04-14T10:30:45.123Z] [module] [LEVEL] message"),
and the multi-line API/auth/config debug sample so that each opening fence is
labeled with a language identifier (use "text"), i.e. change ``` to ```text for
those blocks; no other content changes are needed.
In `@internal/debug/logger_test.go`:
- Around line 231-243: The test TestHandleUnwritableFilePath should not rely on
"/root"; change the failing path to use t.TempDir() so Initialize is given a
directory path (which causes a stable open failure across environments). Update
the call to Initialize in TestHandleUnwritableFilePath to pass t.TempDir() (or a
directory path derived from it) instead of "/root/forbidden.log" and keep the
same graceful-degradation assertion; locate the change by editing
TestHandleUnwritableFilePath and the Initialize invocation.
In `@internal/debug/logger.go`:
- Around line 121-139: maskSensitiveData currently only removes quoted JSON
fields and a single Authorization header pattern; update it to also sanitize the
concrete Go fmt outputs emitted by internal/client/client.go (maps and structs
formatted with %v). Extend the function to (1) pre-scan and replace structured
token shapes like map[access_token:[...]] or map[password:secret] by matching
unquoted key:value and key:[value] patterns (e.g., keys like access_token,
password, refresh_token, api_key, credential) and replacing their values with
"***", (2) handle struct field dumps like &{Password:secret} or {Password:
secret} by matching fieldName:value and FieldName:value patterns for those same
sensitive field names, and (3) include additional Authorization header forms
(e.g., "Authorization: token..." and bare "Bearer ..." variants); keep using
maskSensitiveData as the single sanitization point so internal/client/client.go
can log formatted arguments safely.
- Around line 54-61: Validate filePath with validate.SafeInputPath, use the
repository virtual FS to open the file (e.g., replace os.OpenFile usage with the
vfs wrapper such as repoFS.OpenFile or vfs.OpenFile) and after opening normalize
permissions to 0600 (use Chmod via vfs or repoFS) to ensure pre-existing files
are not group/world-readable; update the block that sets logger.debugFile to
call validate.SafeInputPath(filePath) first, open the file via the vfs/repo
file-layer, handle errors the same way, and then explicitly set the file mode to
0600 before assigning logger.debugFile.
---
Nitpick comments:
In `@internal/debug/logger_test.go`:
- Around line 149-163: The test TestLogMessageFormat currently only ensures the
logger doesn't panic; update it to capture and assert the actual emitted line
format by redirecting log output to a temporary file or buffer, invoking
Initialize(true,"") and GetLogger(), calling logger.Debug and logger.Error,
flushing/closing via Close(), then reading the captured output and matching each
line against a regex that enforces timestamp, module name and level (e.g.
timestamp pattern + module "test_module" + level "DEBUG"/"ERROR" + message). Use
the existing symbols (Initialize, GetLogger, logger.Debug, logger.Error, Close,
globalLogger/loggerOnce reset) to locate and modify the test and ensure the
regex assertions replace the current no-op verification so format regressions
fail the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30c4bd26-d78f-4824-a515-2e227e745d9d
📒 Files selected for processing (9)
cmd/global_flags.gocmd/root.godocs/superpowers/specs/2026-04-14-debug-flag-design.mddocs/superpowers/specs/2026-04-14-debug-flag-test-plan.mdgo.modinternal/client/client.gointernal/cmdutil/factory.gointernal/debug/logger.gointernal/debug/logger_test.go
Comment on lines
+74
to
+80
| // Initialize debug logger before executing any commands | ||
| debugEnabled := globals.Debug || os.Getenv("LARK_CLI_DEBUG") == "1" | ||
| if err := debug.Initialize(debugEnabled, globals.DebugFile); err != nil { | ||
| fmt.Fprintf(os.Stderr, "Warning: failed to initialize debug logger: %v\n", err) | ||
| } | ||
| defer debug.Close() | ||
|
|
There was a problem hiding this comment.
Initialize debug only after flags are parsed (or from pre-parsed invocation context).
In the example at Line 75, globals.Debug is read before rootCmd.Execute() parses flags, so --debug may remain false if this flow is implemented literally. Please clarify in the spec that initialization must use already-parsed values (e.g., bootstrap/invocation context) or run in a pre-run hook after parse.
Suggested doc correction
- // Initialize debug logger before executing any commands
- debugEnabled := globals.Debug || os.Getenv("LARK_CLI_DEBUG") == "1"
- if err := debug.Initialize(debugEnabled, globals.DebugFile); err != nil {
- fmt.Fprintf(os.Stderr, "Warning: failed to initialize debug logger: %v\n", err)
- }
- defer debug.Close()
+ // IMPORTANT: initialize using already-parsed values
+ // Option A: derive from bootstrap/invocation context parsed from os.Args
+ // Option B: initialize in PersistentPreRunE after Cobra parses flags
+ debugEnabled := parsedGlobals.Debug || os.Getenv("LARK_CLI_DEBUG") == "1"
+ if err := debug.Initialize(debugEnabled, parsedGlobals.DebugFile); err != nil {
+ fmt.Fprintf(os.Stderr, "Warning: failed to initialize debug logger: %v\n", err)
+ }
+ defer debug.Close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/superpowers/specs/2026-04-14-debug-flag-design.md` around lines 74 - 80,
The spec incorrectly initializes the debug logger using globals.Debug before CLI
flags are parsed; update the design to initialize debug only after parsed values
are available by either (a) moving the call to debug.Initialize(debugEnabled,
globals.DebugFile) to run after rootCmd.Execute() (or after flag parsing), or
(b) invoking it from a pre-run hook or bootstrap/invocation context that
supplies the already-parsed debug flag; ensure references to globals.Debug,
debug.Initialize, debug.Close and rootCmd.Execute (or the chosen pre-run hook)
are used so the implementation reads the final parsed/debug setting rather than
the pre-parse default.
Comment on lines
+54
to
+61
| if filePath != "" { | ||
| file, err := os.OpenFile(filePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "Warning: failed to open debug file %s: %v\n", filePath, err) | ||
| // Don't return error; fall back to stderr-only mode | ||
| return nil | ||
| } | ||
| logger.debugFile = file |
There was a problem hiding this comment.
Validate and lock down the debug file before opening it.
filePath comes from a user flag, but this writes to os.OpenFile directly. That bypasses the repo’s required path validation and filesystem abstraction, and the 0600 mode only applies when the file is first created, so a pre-existing debug log can stay group/world-readable. Please validate the path, open it through the repo file layer, and normalize permissions after open.
As per coding guidelines, "Use vfs.* instead of os.* for all filesystem access" and "Validate paths using validate.SafeInputPath before any file I/O operations."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/debug/logger.go` around lines 54 - 61, Validate filePath with
validate.SafeInputPath, use the repository virtual FS to open the file (e.g.,
replace os.OpenFile usage with the vfs wrapper such as repoFS.OpenFile or
vfs.OpenFile) and after opening normalize permissions to 0600 (use Chmod via vfs
or repoFS) to ensure pre-existing files are not group/world-readable; update the
block that sets logger.debugFile to call validate.SafeInputPath(filePath) first,
open the file via the vfs/repo file-layer, handle errors the same way, and then
explicitly set the file mode to 0600 before assigning logger.debugFile.
- Update dependencies to resolve module version conflicts - Fix go.mod format compatibility Change-Id: Ida0ee41b52eaff8885adcc4de112d7540a11002f Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- fix(CRITICAL): move debug.Initialize() to PersistentPreRunE to ensure it runs after flag parsing, not before - fix(MAJOR): add path validation using charcheck.RejectControlChars to prevent dangerous characters - fix(MAJOR): use vfs.OpenFile abstraction instead of os.OpenFile for better testability - fix(MAJOR): expand maskSensitiveData() regex patterns to catch unquoted formats (map, struct field dumps) - fix(MINOR): add language identifiers to markdown code fences in design spec - fix(MINOR): replace /root path with t.TempDir() in test for portability - fix: correct go.mod version from 1.23.0 to 1.23 All 16 debug logger unit tests pass successfully. Change-Id: Ib9a9f7da44f41ed0fec1573656aa8221ac89f920 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (3)
docs/superpowers/specs/2026-04-14-debug-flag-design.md (1)
60-80:⚠️ Potential issue | 🟡 MinorSpec example still shows pre-parse debug initialization.
At Line 75,
globals.Debugis read in a flow that implies initialization before Cobra-parsed values are finalized. Please align this example with thePersistentPreRunEpattern so--debugis honored reliably.Doc snippet adjustment
-// Initialize logger early in `Execute()`: +// Initialize logger in `PersistentPreRunE` (after flags are parsed): @@ - // Initialize debug logger before executing any commands - debugEnabled := globals.Debug || os.Getenv("LARK_CLI_DEBUG") == "1" - if err := debug.Initialize(debugEnabled, globals.DebugFile); err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to initialize debug logger: %v\n", err) - } - defer debug.Close() + rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + debugEnabled := globals.Debug || os.Getenv("LARK_CLI_DEBUG") == "1" + if err := debug.Initialize(debugEnabled, globals.DebugFile); err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to initialize debug logger: %v\n", err) + } + return nil + } + defer debug.Close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/specs/2026-04-14-debug-flag-design.md` around lines 60 - 80, The example initializes the debug logger before Cobra has parsed flags (reading globals.Debug), so move debug initialization into the command lifecycle: remove the pre-parse debug.Initialize call in Execute() and instead perform debug.Initialize(...) inside the root command's PersistentPreRunE (or PersistentPreRun) callback after RegisterGlobalFlags has been parsed; determine debugEnabled from globals.Debug or os.Getenv("LARK_CLI_DEBUG") there, call debug.Initialize with globals.DebugFile, and ensure debug.Close() is deferred or called in a matching PersistentPostRun/after Execute() to guarantee proper cleanup.internal/debug/logger.go (2)
57-71:⚠️ Potential issue | 🟠 MajorAdd
validate.SafeInputPathbefore opening--debug-fileand enforce 0600 on existing files.At Line 65,
filePathis opened directly after control-char checks. This still skips the required safe-path validation for user input, and pre-existing files can retain broader permissions unless normalized after open.Suggested fix
import ( "fmt" "os" "regexp" "sync" "time" "github.com/larksuite/cli/internal/charcheck" + "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/internal/vfs" ) @@ if filePath != "" { // Validate file path for security (reject dangerous characters) if err := charcheck.RejectControlChars(filePath, "--debug-file"); err != nil { fmt.Fprintf(os.Stderr, "Warning: invalid debug file path: %v\n", err) // Don't return error; fall back to stderr-only mode return nil } + if err := validate.SafeInputPath(filePath); err != nil { + fmt.Fprintf(os.Stderr, "Warning: invalid debug file path: %v\n", err) + // Don't return error; fall back to stderr-only mode + return nil + } file, err := vfs.OpenFile(filePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) if err != nil { fmt.Fprintf(os.Stderr, "Warning: failed to open debug file %s: %v\n", filePath, err) // Don't return error; fall back to stderr-only mode return nil } + _ = file.Chmod(0600) logger.debugFile = file }As per coding guidelines, "Validate paths using
validate.SafeInputPathbefore any file I/O operations." Based on learnings, explicitvalidate.SafeInputPathis required when bypassingruntime.FileIO()and callingvfs.*directly with user-influenced paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/debug/logger.go` around lines 57 - 71, The code opens the user-provided filePath with vfs.OpenFile after only charcheck.RejectControlChars, but per guidelines you must call validate.SafeInputPath before any file I/O and ensure existing files are set to 0600; update the debug file handling so that before calling vfs.OpenFile you run validate.SafeInputPath(filePath, "--debug-file") and handle/return its error similarly to the charcheck error path, then after opening (or when a pre-existing file is detected) enforce file mode 0600 (e.g., using vfs.Chmod or equivalent) before assigning to logger.debugFile; keep references to filePath, validate.SafeInputPath, vfs.OpenFile, logger.debugFile and the --debug-file flag in your changes.
133-163:⚠️ Potential issue | 🟠 MajorSensitive-data masking still misses
%vmap/struct shapes for some keys.
internal/client/client.gologs query/body with%v, but the current patterns still leave common forms likemap[password:secret],map[api_key:[secret]], and{Credential:secret}unredacted.Suggested regex coverage expansion
func maskSensitiveData(message string) string { @@ // Mask API keys in JSON message = regexp.MustCompile(`"api_key"\s*:\s*"[^"]*"`).ReplaceAllString(message, `"api_key": "***"`) + message = regexp.MustCompile(`api_key\s*:\s*\[[^\]]*\]`).ReplaceAllString(message, "api_key: [***]") + message = regexp.MustCompile(`api_key\s*:\s*[^\s,}]+`).ReplaceAllString(message, "api_key: ***") @@ // Mask passwords (quoted JSON format) message = regexp.MustCompile(`"password"\s*:\s*"[^"]*"`).ReplaceAllString(message, `"password": "***"`) + message = regexp.MustCompile(`password\s*:\s*\[[^\]]*\]`).ReplaceAllString(message, "password: [***]") + message = regexp.MustCompile(`password\s*:\s*[^\s,}]+`).ReplaceAllString(message, "password: ***") @@ // Mask credentials (unquoted format) message = regexp.MustCompile(`credential\s*:\s*[^\s,}]+`).ReplaceAllString(message, "credential: ***") + message = regexp.MustCompile(`Credential\s*:\s*[^\s,}]+`).ReplaceAllString(message, "Credential: ***")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/debug/logger.go` around lines 133 - 163, The masking logic in internal/debug/logger.go misses unquoted `%v` map/struct shapes emitted by internal/client/client.go (e.g., map[password:secret], map[api_key:[secret]], {Credential:secret}); update the regexp replacements that operate on the message variable to also match and redact keys inside map[...] and {...} shapes and capitalized key variants (e.g., Password, Credential, Api_key, Access_token, Refresh_token) as well as bracketed/list values (e.g., api_key:\[[^\]]*\]) and nested forms; ensure each existing replacement (Authorization/ api_key / access_token / refresh_token / password / credential) has companion patterns targeting map\[..\] and \{...\} forms so those `%v` prints are replaced with the same "***" placeholders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/superpowers/specs/2026-04-14-debug-flag-design.md`:
- Around line 60-80: The example initializes the debug logger before Cobra has
parsed flags (reading globals.Debug), so move debug initialization into the
command lifecycle: remove the pre-parse debug.Initialize call in Execute() and
instead perform debug.Initialize(...) inside the root command's
PersistentPreRunE (or PersistentPreRun) callback after RegisterGlobalFlags has
been parsed; determine debugEnabled from globals.Debug or
os.Getenv("LARK_CLI_DEBUG") there, call debug.Initialize with globals.DebugFile,
and ensure debug.Close() is deferred or called in a matching
PersistentPostRun/after Execute() to guarantee proper cleanup.
In `@internal/debug/logger.go`:
- Around line 57-71: The code opens the user-provided filePath with vfs.OpenFile
after only charcheck.RejectControlChars, but per guidelines you must call
validate.SafeInputPath before any file I/O and ensure existing files are set to
0600; update the debug file handling so that before calling vfs.OpenFile you run
validate.SafeInputPath(filePath, "--debug-file") and handle/return its error
similarly to the charcheck error path, then after opening (or when a
pre-existing file is detected) enforce file mode 0600 (e.g., using vfs.Chmod or
equivalent) before assigning to logger.debugFile; keep references to filePath,
validate.SafeInputPath, vfs.OpenFile, logger.debugFile and the --debug-file flag
in your changes.
- Around line 133-163: The masking logic in internal/debug/logger.go misses
unquoted `%v` map/struct shapes emitted by internal/client/client.go (e.g.,
map[password:secret], map[api_key:[secret]], {Credential:secret}); update the
regexp replacements that operate on the message variable to also match and
redact keys inside map[...] and {...} shapes and capitalized key variants (e.g.,
Password, Credential, Api_key, Access_token, Refresh_token) as well as
bracketed/list values (e.g., api_key:\[[^\]]*\]) and nested forms; ensure each
existing replacement (Authorization/ api_key / access_token / refresh_token /
password / credential) has companion patterns targeting map\[..\] and \{...\}
forms so those `%v` prints are replaced with the same "***" placeholders.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cecb8b03-23ec-48e6-8055-42a1a8bea40f
📒 Files selected for processing (4)
cmd/root.godocs/superpowers/specs/2026-04-14-debug-flag-design.mdinternal/debug/logger.gointernal/debug/logger_test.go
✅ Files skipped from review due to trivial changes (1)
- internal/debug/logger_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/root.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a global
--debugflag to lark-cli that enables comprehensive debugging output, including HTTP request/response logging, detailed error messages, and internal operation tracing.Features
--debugflag (enables stderr output)--debug-file <path>for file loggingLARK_CLI_DEBUG=1)Implementation Summary
New Package:
internal/debugModified Components
cmd/global_flags.go- Added --debug and --debug-file flagscmd/root.go- Initialize logger from flags and LARK_CLI_DEBUG env varinternal/client/client.go- API request/response logginginternal/cmdutil/factory.go- Identity resolution loggingLog Format
```
[2026-04-14T10:30:45.123Z] [module] [LEVEL] message
```
Testing
Unit Tests: 16 tests covering:
All tests pass with `-race` flag.
E2E Scenarios: 9 verified scenarios
Documentation
Statistics
Example Usage
```bash
Enable debug to stderr
lark-cli --debug calendar +agenda
Enable debug to file
lark-cli --debug --debug-file debug.log api GET /open-apis/calendar/v4/calendars
Enable debug via environment variable
LARK_CLI_DEBUG=1 lark-cli contact +search-user --query "John"
```
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores