fix(adapter): don't emit permissionDecision for non-PreToolUse events#29
Open
srnnkls wants to merge 1 commit into
Open
fix(adapter): don't emit permissionDecision for non-PreToolUse events#29srnnkls wants to merge 1 commit into
srnnkls wants to merge 1 commit into
Conversation
RenderOutput defaulted every event outside contextOnlyEvents to a
permissionDecision-carrying hookSpecificOutput. Session-lifecycle and
notification events (SessionEnd, SessionStart, Notification, PreCompact)
have no hookSpecificOutput variant in the Claude Code hook protocol, so
CC rejected the response ("(root): Invalid input") and the hook failed.
Make the permission shape opt-in for PreToolUse, keep the context shape
for context events, and render a bare {} for everything else. The
parse-error fallback defaults an unextractable event name to PreToolUse
so fail-open/fail-closed still renders a valid decision.
There was a problem hiding this comment.
Pull request overview
Fixes Claude Code adapter output so it conforms to the Claude Code hook response schema: only PreToolUse responses include permissionDecision, context-injection events emit a context-only variant, and all other events emit a bare {} (no hookSpecificOutput).
Changes:
- Split hook events into
permissionEvents(PreToolUse) vscontextEventsand render the correct JSON shape per category. - Render bare
{}for session-lifecycle/notification/unknown events to avoid emitting invalid discriminated-union variants. - Default
peekHookEventNametoPreToolUseon parse-error fallback so fail-open/fail-closed still produces a decision response.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/adapter/claudecode.go | Adjusts Claude Code hook response rendering to emit schema-valid shapes per hook event kind. |
| internal/adapter/claudecode_test.go | Updates/extends tests to cover bare events and context-only events, and adjusts hook-event echo expectations. |
| cmd/fas/main.go | Changes parse-error fallback hook-event-name extraction to default to PreToolUse when unextractable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+351
to
+355
| var probe map[string]any | ||
| if err := json.Unmarshal(raw, &probe); err != nil { | ||
| t.Fatalf("emitted JSON not parseable: %v (raw=%s)", err, raw) | ||
| } | ||
| }) |
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.
Problem
fas eval --harness claudeemits an invalid hook response forSessionEnd:Claude Code rejects it with
Hook JSON output validation failed — (root): Invalid input.hookSpecificOutputis a discriminated union keyed onhookEventName, andSessionEndis not a valid variant — onlyPreToolUsecarries apermissionDecision.Root cause:
ClaudeCode.RenderOutputtreatedpermissionDecisionas the default shape for every event outside a smallcontextOnlyEventsset. This also latently affectedSessionStart,Notification, andPreCompact.Fix
permissionEvents(PreToolUse) →permissionDecisionshape.contextEvents(UserPromptSubmit, PostToolUse, PostToolBatch, Stop, SubagentStart, SubagentStop) →additionalContext-only shape.{}, which is valid against the CC schema.hook_event_namenow defaults toPreToolUseso the fail-open/fail-closed decision still renders under the one event that carries apermissionDecision.Repro (before → after)
PreToolUsestill decides:{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"allow"}}.Tests
SessionEnd/SessionStart/Notification/PreCompact/empty) omithookSpecificOutput; context events omitpermissionDecision.HookEventNameecho test (droppedSessionStart, now bare).go test ./...); gofmt/vet/build pass.Note: the pre-existing
golangci-lintfindings (gocritic/preallocin unrelated files, also present onmain) are out of scope and untouched here.