fix(hooks): guard against undefined output.output in tool.execute.after hooks#1727
Closed
sjawhar wants to merge 1 commit intocode-yeongyu:devfrom
Closed
fix(hooks): guard against undefined output.output in tool.execute.after hooks#1727sjawhar wants to merge 1 commit intocode-yeongyu:devfrom
sjawhar wants to merge 1 commit intocode-yeongyu:devfrom
Conversation
Contributor
|
All contributors have signed the CLA. Thank you! ✅ |
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Simple defensive type check additions. Both changes add typeof output.output !== \"string\" guards before calling .toLowerCase(), preventing runtime errors on non-string outputs. No logic changes,
761a42e to
47df964
Compare
….after hooks MCP tool results (e.g. Slack, Toggl) don't populate output.output as a string, causing TypeError crashes in hooks that call .toLowerCase() or .startsWith(), and silent "undefined" string corruption in hooks that use +=. Add `output.output == null` guard to all 14 tool.execute.after hooks and injectors that access output.output. This is consistent with the existing guard style in empty-task-response-detector (which uses ?.) and normalizes the previous typeof check in tool-output-truncator. Hooks fixed: - comment-checker (crash: .toLowerCase on undefined) - edit-error-recovery (crash, behind tool filter) - task-resume-info (crash: .startsWith on undefined, behind tool filter) - delegate-task-retry (passed to function, behind tool filter) - context-window-monitor (corrupt: += on undefined) - task-reminder (corrupt: += on undefined) - claude-code-hooks handler (corrupt: template literal with undefined) - category-skill-reminder (corrupt, behind tool filter) - agent-usage-reminder (corrupt, behind tool filter) - interactive-bash-session (corrupt, behind tool filter) - directory-readme-injector (corrupt, behind tool filter) - directory-agents-injector (corrupt, behind tool filter) - rules-injector (corrupt, behind tool filter) - tool-output-truncator (normalize typeof to == null)
47df964 to
76e3a51
Compare
Author
|
I have read the CLA Document and I hereby sign the CLA |
marlon-costa-dc
added a commit
to marlon-costa-dc/oh-my-opencode
that referenced
this pull request
Feb 10, 2026
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
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
Guard against
undefinedoutput.outputin all 14tool.execute.afterhooks and injectors that access the field.MCP tool results (e.g. Slack, Toggl) don't populate
output.outputas a string, causing:.toLowerCase()or.startsWith()onundefined"undefined"string corruption in hooks that use+=(producing"undefined\n\nSome reminder")Root Cause
The
tool.execute.afterhook fires for all tool calls, including MCP tools. The hook type signature declaresoutput: { output: string }, but MCP tool results don't always populate this field. Many hooks accessoutput.outputwithout null-checking first.Two existing hooks already guarded against this:
empty-task-response-detectorusesoutput.output?.trim() ?? ""tool-output-truncatorusedtypeof output.output !== 'string'The rest did not, and the codebase was inconsistent.
Fix
Add
output.output == nullearly-return guard to everytool.execute.afterhandler that accessesoutput.output. This:== null) to catch bothnullandundefinedtypeof !== 'string'check intool-output-truncatorto matchHooks fixed
comment-checker.toLowerCase()on undefinededit-error-recovery.toLowerCase()on undefinededitonly)task-resume-info.startsWith()on undefinedtask/call_omo_agent)delegate-task-retrytaskonly)context-window-monitorundefined +=task-reminderundefined +=claude-code-hooks handlercategory-skill-reminderundefined +=agent-usage-reminderundefined +=interactive-bash-sessionundefined +=interactive_bash)directory-readme-injectorundefined +=read/glob/grep)directory-agents-injectorundefined +=read/glob/grep)rules-injectorundefined +=read/glob/grep)tool-output-truncatorTesting
file://plugin path in OpenCode configconversations_add_messageandchannels_listcalls succeed after fix (previously all Slack MCP calls returned TypeError)output.outputis nullish