Skip to content

[AUTOMATION] feat: clean up hook payload JSON helpers#268

Open
michiosw wants to merge 1 commit into
mainfrom
feat/cleanup-hook-payload-json
Open

[AUTOMATION] feat: clean up hook payload JSON helpers#268
michiosw wants to merge 1 commit into
mainfrom
feat/cleanup-hook-payload-json

Conversation

@michiosw

@michiosw michiosw commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary
This cleans up hook payload JSON helpers by moving map encode/decode and numeric-preserving JSON parsing into one canonical hook helper.

Before this, localruntime, sidecar, and hookruntime each carried their own JSON map helpers or decode path, which made hook payload handling easier to drift.

Now internal/hook/json.go is the canonical path:

data, err := hook.MarshalMapJSON(event.ToolInput)
value, err := hook.UnmarshalMapJSON(req.ToolInput)

Why
This gives kontext-cli a cleaner maintenance path for hook payload handling:

hook event/result
-> internal/hook/json.go
-> localruntime, sidecar, hookruntime
-> one typed decision path and one JSON codec

This PR does not broaden behavior beyond the cleanup scope.

What changed
Added internal/hook/json.go and regression tests for nil-map handling and large-number preservation
Removed duplicate JSON map helpers from internal/localruntime and internal/sidecar
Consolidated Claude tool-response decoding onto the shared helper
Strengthened localruntime.EvaluateResult.Decision to use hook.Decision
Updated localruntime and sidecar tests for the typed decision path

Verification
go test ./internal/hook ./internal/hookruntime ./internal/localruntime ./internal/sidecar -count=1
go test ./...
go vet ./...

michiosw commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@michiosw michiosw changed the title feat(localruntime): consolidate hook payload JSON helpers [AUTOMATION] feat: clean up hook payload JSON helpers Jun 8, 2026
@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR centralizes hook payload JSON handling and updates local runtime result typing. The main changes are:

  • Adds shared JSON helpers under internal/hook.
  • Reuses the helpers in localruntime, sidecar, and Claude hook response decoding.
  • Changes localruntime result decisions to use hook.Decision.
  • Adds tests for large number preservation and decision handling.

Confidence Score: 4/5

This is close, but the Claude input number handling should be fixed before merging.

  • Claude tool_response now preserves large numbers, but tool_input still loses precision through plain JSON decoding.

  • The issue affects realistic tool payloads with large IDs and can produce incorrect downstream policy inputs.

  • The other helper substitutions appear wire-compatible with the previous behavior.

  • internal/hookruntime/claude.go should use number-preserving decoding for tool_input as well as tool_response.

Important Files Changed

Filename Overview
internal/hookruntime/claude.go Moves tool response decoding to the shared helper, but leaves tool input decoding on plain json.Unmarshal.
internal/hook/json.go Introduces shared JSON helpers for map marshaling and number-preserving unmarshaling.
internal/localruntime/conversions.go Replaces local JSON helpers with the shared hook helpers and keeps localruntime conversions equivalent.

Comments Outside Diff (1)

  1. internal/hookruntime/claude.go, line 61 (link)

    P1 Preserve input numbers

    ToolInput and ToolInputAlt are still decoded by the outer json.Unmarshal into map[string]any, so large integers in Claude tool inputs are converted to float64 before the new shared helper ever sees them. When Claude sends a PreToolUse payload such as tool_input.id = 9007199254740993, this path rounds it before localruntime or sidecar forwarding, and downstream policy checks, recordings, or hashes can receive the wrong ID. Decode the Claude input with UseNumber too, or keep the tool input fields as json.RawMessage and pass them through hook.UnmarshalMapJSON like the updated response path.

Reviews (1): Last reviewed commit: "feat(localruntime): consolidate hook pay..." | Re-trigger Greptile

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