Skip to content

[code-review] hooks ExtractFilePath returns garbage for non-string JSON values #121

Description

@topcheer

File and Lines

internal/hooks/runner.go:375-396

Problem Description

ExtractFilePath performs manual JSON parsing without validating that the value after a file-path key is actually a string. For non-string JSON values (null, numbers, booleans), the function returns garbage substrings.

For input {"file_path": null, "other": "value"}, the function returns null, as the file path.

Trigger Scenario

  1. LLM generates a tool call with "file_path": null (happens when optional parameter is omitted but LLM explicitly includes the key with null)
  2. ExtractFilePath returns null, as the file path
  3. Garbage flows into HookEnv.FilePath (agent_tool.go:334) and TUI display (tool_labels.go:42)

Expected vs Actual Behavior

  • Expected: Return empty string for non-string values
  • Actual: Returns garbage substring from other JSON fields

Fix Suggestion

After extracting the value, check that it starts with a quote before processing. If not, skip to the next key.

Severity

Low-Medium - LLM-generated JSON occasionally includes null for optional keys. Garbage flows into hook env vars and TUI display.

Verification

Independently verified by subagent sa-8. Confirmed garbage output for null, number, and boolean types. Two production call sites confirmed.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions