Skip to content

Conversation

@uyu423
Copy link

@uyu423 uyu423 commented Feb 11, 2026

Summary

Changes

Problem

#1756 fixed READ operations (.toLowerCase()) on output.output with ?? "" guards in 3 files, but WRITE operations (output.output += text) remained unguarded across 13 hook files. When MCP tools (Atlassian, Exa, Chrome DevTools, grep.app, etc.) return undefined instead of a string for output.output, these writes cause:

  • TypeError: undefined is not an object — crashes the entire tool execution pipeline
  • "undefinedSome reminder text" — silent data corruption when JS coerces undefined + string

Solution

New helper (src/hooks/hook-output-guard.ts):

export function appendToOutput(output: { output: string }, text: string): void {
    if (output.output == null) {
        output.output = text   // initialize from null/undefined
        return
    }
    if (typeof output.output === "string") {
        output.output += text  // normal append
    }
    // non-string (object/array) → leave untouched to preserve structured responses
}

Design decisions:

  • == null (loose equality) catches both null and undefined in one check
  • Non-string values (objects, arrays) are left untouched to prevent corrupting structured MCP responses
  • Initializes from null/undefined rather than silently skipping, so context injections are preserved

13 hook files updated — all output.output += ... and output.output = `${output.output}...` patterns replaced with appendToOutput() calls:

File Change
agent-usage-reminder/hook.ts += REMINDER_MESSAGEappendToOutput
category-skill-reminder/hook.ts += reminderMessageappendToOutput
claude-code-hooks/.../tool-execute-after-handler.ts template literal concat → appendToOutput (2 sites)
comment-checker/cli-runner.ts += result.messageappendToOutput (2 sites)
context-window-monitor.ts += CONTEXT_REMINDERappendToOutput
delegate-task-retry/hook.ts += guidanceappendToOutput + READ guard ?? ""
directory-agents-injector/injector.ts += directory contextappendToOutput
directory-readme-injector/injector.ts += README contentappendToOutput
edit-error-recovery/hook.ts += EDIT_ERROR_REMINDERappendToOutput
interactive-bash-session/hook.ts += reminderappendToOutput
interactive-bash-session/interactive-bash-session-hook.ts += reminderToAppendappendToOutput
rules-injector/injector.ts += rule contentappendToOutput
task-reminder/hook.ts += REMINDER_MESSAGEappendToOutput

Testing

bun run typecheck   # passes
bun run build       # passes
bun test src/hooks/hook-output-guard.test.ts  # 8 pass, 0 fail

Test cases cover:

  • Normal string append
  • Empty string append
  • undefined → initializes with text (core MCP bug scenario)
  • null → initializes with text
  • Non-string object → preserved (no corruption)
  • Number → preserved
  • Multiple sequential appends
  • Multiple appends starting from undefined

Related Issues

Complements #1756 (d5fd918) — that PR guards READ operations, this PR guards WRITE operations.
Related to #1720 (original MCP tool output crash report).


Summary by cubic

Adds a centralized appendToOutput helper to make after-hook output writes null-safe, preventing crashes and "undefinedstring" corruption when MCP tools return undefined. Replaces ad-hoc string concatenation across hooks and adds tests.

  • Bug Fixes
    • Introduced appendToOutput: initializes on null/undefined, appends to strings, leaves non-strings untouched.
    • Replaced output.output concatenations with the helper across 13 hooks; added a read guard in delegate-task-retry.
    • Added 8 unit tests; typecheck and build pass.

Written for commit 4b21310. Summary will update on new commits.

…s in after-hooks

MCP tools (Atlassian, Exa, Chrome DevTools, grep.app) can return
responses where output.output is undefined. While d5fd918 fixed
read operations (toLowerCase), write operations (output.output +=)
were still unguarded, causing either TypeError crashes or silent
'undefinedstring' concatenation.

Introduces appendToOutput() helper that:
- Initializes null/undefined output to the appended text (preserving
  context injection instead of silently skipping)
- Appends to existing string output normally
- Leaves non-string output (objects/arrays) untouched

Replaces inline typeof guards across 12 hook files with the
centralized helper, and adds 8 unit tests for the helper.

Affected hooks:
- agent-usage-reminder, category-skill-reminder
- claude-code-hooks/tool-execute-after-handler
- comment-checker/cli-runner (2 locations)
- context-window-monitor, delegate-task-retry
- directory-agents-injector, directory-readme-injector
- edit-error-recovery, interactive-bash-session
- rules-injector, task-reminder
…ession-hook

- Fix accidental 1-space indentation shift in 10 modified hook files
- Guard output.output += in interactive-bash-session-hook.ts (was missed)
@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2026

All contributors have signed the CLA. Thank you! ✅
Posted by the CLA Assistant Lite bot.

@uyu423
Copy link
Author

uyu423 commented Feb 11, 2026

I have read the CLA Document and I hereby sign the CLA

@uyu423
Copy link
Author

uyu423 commented Feb 11, 2026

recheck

github-actions bot added a commit that referenced this pull request Feb 11, 2026
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 15 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Auto-approved: Refactoring PR: safely replaces direct string concatenation with a new appendToOutput helper across 13 hooks. The helper handles null/undefined MCP tool outputs (common crash source) and preserves 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant