Hotfix SDK Dropping Logs#37
Conversation
WalkthroughThe changes introduce OpenAI API key configuration support across two executor services. In 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 docstrings
🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/claude-executor.ts`:
- Around line 216-233: The CI flagged Prettier formatting issues around the
synthetic user-message handling block (the code checking message.type === 'user'
and message.isSynthetic and updating currentResponse and calling
process.stdout.write(chalk.cyan(...))). Re-run the formatter (npx prettier
--write) on src/services/claude-executor.ts or manually adjust formatting in
that block so the nested ternary and array processing (the content-to-text
conversion), the process.stdout.write(chalk.cyan(text) + '\n'), and
currentResponse += text + '\n' lines conform to project Prettier rules
(consistent indentation, line breaks, and spacing) to resolve the CI failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 84aaca4c-941f-40c6-aa7f-bcccf74b3ac2
📒 Files selected for processing (2)
src/services/claude-cli-executor.tssrc/services/claude-executor.ts
📜 Review details
🧰 Additional context used
🪛 GitHub Actions: CI
src/services/claude-executor.ts
[warning] 1-1: Prettier --check reported formatting/style issues in this file.
🔇 Additional comments (4)
src/services/claude-cli-executor.ts (1)
43-46: LGTM!The truthiness check
if (openaiKey)correctly prevents overwriting a valid pre-existingOPENAI_API_KEYenvironment variable when the config value is an empty string. This defensive approach properly handles the case whereopenaiApiKeyis initialized as''in the config defaults (as shown insrc/config.ts:418).src/services/claude-executor.ts (3)
45-48: LGTM!The OPENAI_API_KEY handling is correctly implemented with a truthiness guard, consistent with the approach in
claude-cli-executor.ts.
216-233: LGTM!The synthetic user message handling is well-implemented:
- Correctly checks
message.isSyntheticto filter hook-injected context- Handles both string and array content formats with proper type narrowing
- The type predicate filter safely narrows the content block type
This change addresses the "SDK Dropping Logs" issue by ensuring hook-injected content (e.g., ivan learnings) is now displayed and logged.
251-256: Verify that session ID is reliably captured from system-init messages.The code now captures
session_idonly fromsystemmessages withsubtype === 'init'. The caller insrc/services/task-executor.ts(lines 740, 855) depends on the returnedsessionIdto maintain context across consecutiveexecuteTask()calls. If the SDK sendssession_idin other message types (e.g., during resume or other events), those will no longer be captured, potentially breaking session continuity.Confirm with the SDK documentation that
session_idis always reliably emitted in the system-init message and not in other message types during typical execution flows.
| } else if (message.type === 'user') { | ||
| // Synthetic user messages carry hook-injected context (e.g. ivan learnings) | ||
| if (message.isSynthetic) { | ||
| const content = message.message.content; | ||
| const text = | ||
| typeof content === 'string' | ||
| ? content | ||
| : Array.isArray(content) | ||
| ? content | ||
| .filter((b): b is { type: 'text'; text: string } => b.type === 'text') | ||
| .map((b) => b.text) | ||
| .join('\n') | ||
| : null; | ||
| if (text) { | ||
| process.stdout.write(chalk.cyan(text) + '\n'); | ||
| currentResponse += text + '\n'; | ||
| } | ||
| } |
There was a problem hiding this comment.
Address Prettier formatting issues flagged by CI.
The pipeline reports formatting issues in this file. Please run the formatter to resolve:
npx prettier --write src/services/claude-executor.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/claude-executor.ts` around lines 216 - 233, The CI flagged
Prettier formatting issues around the synthetic user-message handling block (the
code checking message.type === 'user' and message.isSynthetic and updating
currentResponse and calling process.stdout.write(chalk.cyan(...))). Re-run the
formatter (npx prettier --write) on src/services/claude-executor.ts or manually
adjust formatting in that block so the nested ternary and array processing (the
content-to-text conversion), the process.stdout.write(chalk.cyan(text) + '\n'),
and currentResponse += text + '\n' lines conform to project Prettier rules
(consistent indentation, line breaks, and spacing) to resolve the CI failure.
1 participant
Summary by CodeRabbit
Bug Fixes
Improvements