From a9a5d582747b5005848d257c852507e193f5fc98 Mon Sep 17 00:00:00 2001 From: tumberger Date: Fri, 31 Jul 2026 00:48:01 +0200 Subject: [PATCH 1/2] feat(agent): add a Devin hook adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registers a `devin` agent so `kontext hook --agent devin` can translate the Devin runtime's plugin hook payloads. Policy evaluation is unchanged: the adapter only decodes the incoming event and encodes the decision in the form that runtime reads from stdout. Decode maps `session_id` and `tool_use_id` straight through — Devin session IDs are already namespaced, so unlike the Codex adapter there is no prefix to add and no synthetic ID to mint. `tool_response` is captured so post-tool-use records an outcome rather than only an intent. `prompt_id` is deliberately dropped for now. It is a useful per-turn correlator but has no home on hook.Event, and folding it into ToolInput would be wrong: ToolInput becomes the Cedar evaluation context, so a synthetic attribute there would be policy-visible and would change the parameters hash recorded on every decision. Adding a first-class field is left for a follow-up. Encode emits a decision only for pre-tool-use; every other event is observation and emits no directive. Anything other than an explicit allow blocks, so an unrecognized decision value fails closed. An updated tool input is refused rather than applied. That runtime replaces tool_input wholesale instead of merging, so emitting a partial rewrite would silently drop every field the policy did not restate, and the post-tool-use hook would then observe only the rewritten input — leaving the original absent from the ledger. Nothing populates UpdatedInput today, so this is a guard against a future change rather than active logic. Adds hook.ErrSkipEvent for events an adapter recognizes but does not translate, honored by both hook runners. Some agent runtimes read a non-zero hook exit as "block the tool call", so reporting an unhandled lifecycle event as a decode failure would stall the agent on a legitimate signal. Unknown event names skip for the same reason; malformed input still fails, since that indicates a bug or a contract change rather than an event we chose not to handle. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/kontext/main.go | 1 + internal/agent/devin/devin.go | 24 ++ internal/agent/devin/devin_test.go | 59 ++++ internal/guard/hookruntime/runtime.go | 7 + internal/guard/hookruntime/runtime_test.go | 39 +++ internal/hook/domain.go | 16 +- internal/hookruntime/devin.go | 157 +++++++++++ internal/hookruntime/devin_test.go | 298 +++++++++++++++++++++ internal/hookruntime/runner.go | 7 + internal/hookruntime/runner_test.go | 54 +++- 10 files changed, 659 insertions(+), 3 deletions(-) create mode 100644 internal/agent/devin/devin.go create mode 100644 internal/agent/devin/devin_test.go create mode 100644 internal/hookruntime/devin.go create mode 100644 internal/hookruntime/devin_test.go diff --git a/cmd/kontext/main.go b/cmd/kontext/main.go index fa515abc..7667d7b6 100644 --- a/cmd/kontext/main.go +++ b/cmd/kontext/main.go @@ -30,6 +30,7 @@ import ( _ "github.com/kontext-security/kontext-cli/internal/agent/claude" _ "github.com/kontext-security/kontext-cli/internal/agent/codex" _ "github.com/kontext-security/kontext-cli/internal/agent/cowork" + _ "github.com/kontext-security/kontext-cli/internal/agent/devin" ) var version = "dev" diff --git a/internal/agent/devin/devin.go b/internal/agent/devin/devin.go new file mode 100644 index 00000000..6c21ea59 --- /dev/null +++ b/internal/agent/devin/devin.go @@ -0,0 +1,24 @@ +// Package devin registers the Devin agent adapter. +package devin + +import ( + "github.com/kontext-security/kontext-cli/internal/agent" + "github.com/kontext-security/kontext-cli/internal/hook" + "github.com/kontext-security/kontext-cli/internal/hookruntime" +) + +func init() { + agent.Register(&Devin{}) +} + +type Devin struct{} + +func (d *Devin) Name() string { return "devin" } + +func (d *Devin) DecodeHookInput(input []byte) (hook.Event, error) { + return hookruntime.DecodeDevinEvent(input, d.Name()) +} + +func (d *Devin) EncodeHookResult(event hook.Event, result hook.Result) ([]byte, error) { + return hookruntime.EncodeDevinResult(event.HookName.String(), result) +} diff --git a/internal/agent/devin/devin_test.go b/internal/agent/devin/devin_test.go new file mode 100644 index 00000000..7e758bdf --- /dev/null +++ b/internal/agent/devin/devin_test.go @@ -0,0 +1,59 @@ +package devin + +import ( + "errors" + "testing" + + "github.com/kontext-security/kontext-cli/internal/agent" + "github.com/kontext-security/kontext-cli/internal/hook" +) + +func TestDevinRegisteredUnderItsName(t *testing.T) { + t.Parallel() + + registered, ok := agent.Get("devin") + if !ok { + t.Fatal("agent.Get(\"devin\") not found, want the adapter registered") + } + if registered.Name() != "devin" { + t.Fatalf("Name() = %q, want devin", registered.Name()) + } +} + +func TestDecodeHookInputStampsAgentName(t *testing.T) { + t.Parallel() + + event, err := (&Devin{}).DecodeHookInput([]byte( + `{"hook_event_name":"PreToolUse","tool_name":"exec","tool_input":{"command":"ls"}}`, + )) + if err != nil { + t.Fatalf("DecodeHookInput() error = %v", err) + } + if event.Agent != "devin" { + t.Fatalf("Agent = %q, want devin", event.Agent) + } +} + +func TestDecodeHookInputReportsSkipForEventsWithoutDecisions(t *testing.T) { + t.Parallel() + + _, err := (&Devin{}).DecodeHookInput([]byte(`{"hook_event_name":"PostCompaction"}`)) + if !errors.Is(err, hook.ErrSkipEvent) { + t.Fatalf("DecodeHookInput() error = %v, want ErrSkipEvent", err) + } +} + +func TestEncodeHookResultUsesTheEventOnTheEvent(t *testing.T) { + t.Parallel() + + out, err := (&Devin{}).EncodeHookResult( + hook.Event{HookName: hook.HookPreToolUse}, + hook.Result{Decision: hook.DecisionDeny, Reason: "denied by policy"}, + ) + if err != nil { + t.Fatalf("EncodeHookResult() error = %v", err) + } + if want := `{"decision":"block","reason":"denied by policy"}`; string(out) != want { + t.Fatalf("EncodeHookResult() = %s, want %s", out, want) + } +} diff --git a/internal/guard/hookruntime/runtime.go b/internal/guard/hookruntime/runtime.go index b794abed..802c9c96 100644 --- a/internal/guard/hookruntime/runtime.go +++ b/internal/guard/hookruntime/runtime.go @@ -2,6 +2,7 @@ package hookruntime import ( "context" + "errors" "fmt" "io" "strings" @@ -34,6 +35,12 @@ type Processor interface { func Run(ctx context.Context, adapter Adapter, processor Processor, mode Mode, stdin io.Reader, stdout, stderr io.Writer) error { event, err := adapter.Decode(stdin) if err != nil { + // An event the adapter recognizes but does not translate carries no + // decision: emit nothing and succeed, rather than encoding a deny for + // what is only a lifecycle signal. + if errors.Is(err, hook.ErrSkipEvent) { + return nil + } fmt.Fprintf(stderr, "kontext: malformed hook input: %v\n", err) malformed := hook.Event{HookName: adapter.MalformedHookName()} return adapter.Encode(stdout, malformed, effectiveResult(malformed, hook.Result{ diff --git a/internal/guard/hookruntime/runtime_test.go b/internal/guard/hookruntime/runtime_test.go index 86771e5a..239d8fc0 100644 --- a/internal/guard/hookruntime/runtime_test.go +++ b/internal/guard/hookruntime/runtime_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "io" "testing" @@ -183,3 +184,41 @@ func TestEffectiveResultRemoteDowngradesUnstampedToObserve(t *testing.T) { t.Fatalf("reason = %q, want observe would-note", result.Reason) } } + +// An event the adapter recognizes but does not translate carries no decision. +// It must not be encoded as a deny the way malformed input is, and it must not +// reach the processor. +func TestRunSkippedEventEncodesNothing(t *testing.T) { + t.Parallel() + + adapter := &stubAdapter{ + decodeErr: fmt.Errorf("devin: event PostCompaction: %w", hook.ErrSkipEvent), + } + stdout := &bytes.Buffer{} + + err := Run(context.Background(), adapter, stubProcessor{err: errors.New("must not be called")}, ModeEnforce, bytes.NewReader(nil), stdout, io.Discard) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if adapter.result.Decision != "" { + t.Fatalf("encoded decision = %q, want nothing encoded", adapter.result.Decision) + } + if stdout.String() != "" { + t.Fatalf("stdout = %q, want empty", stdout.String()) + } +} + +// Malformed input keeps its existing behavior: a policy-controlled result is +// still encoded, so this must not be swept into the skip path. +func TestRunMalformedInputStillEncodesDenyInEnforce(t *testing.T) { + t.Parallel() + + adapter := &stubAdapter{decodeErr: errors.New("malformed")} + err := Run(context.Background(), adapter, stubProcessor{}, ModeEnforce, bytes.NewReader(nil), io.Discard, io.Discard) + if err != nil { + t.Fatalf("Run() error = %v", err) + } + if adapter.result.Decision != hook.DecisionDeny { + t.Fatalf("decision = %q, want deny", adapter.result.Decision) + } +} diff --git a/internal/hook/domain.go b/internal/hook/domain.go index 5798b1f4..28ecb34f 100644 --- a/internal/hook/domain.go +++ b/internal/hook/domain.go @@ -1,6 +1,20 @@ package hook -import "strings" +import ( + "errors" + "strings" +) + +// ErrSkipEvent reports a hook event an adapter recognizes but deliberately does +// not translate — a lifecycle signal that carries no authorization decision and +// must not be recorded as one. +// +// Adapters wrap it; hook runners test for it with errors.Is and exit +// successfully without writing a decision. It exists because some agent +// runtimes read a non-zero hook exit as "block the tool call", so reporting an +// unhandled event as a decode failure would stall the agent on an event we +// simply chose not to handle. +var ErrSkipEvent = errors.New("hook event skipped by adapter") type HookName string diff --git a/internal/hookruntime/devin.go b/internal/hookruntime/devin.go new file mode 100644 index 00000000..904159f5 --- /dev/null +++ b/internal/hookruntime/devin.go @@ -0,0 +1,157 @@ +package hookruntime + +import ( + "bytes" + "encoding/json" + "fmt" + + "github.com/kontext-security/kontext-cli/internal/hook" +) + +// devinDecisionBlock is the decision verb the Devin runtime understands on a +// pre-tool-use hook's stdout. Devin surfaces the accompanying reason to the +// agent verbatim, so reasons double as policy messaging. +const devinDecisionBlock = "block" + +// devinHookInput mirrors the payload the Devin runtime writes to a plugin +// hook's stdin. Devin emits snake_case only, so unlike the Claude Code adapter +// there are no camelCase aliases to tolerate. +// +// Fields Devin sends that we deliberately do not decode: +// +// - prompt_id: a per-turn correlator with no home on hook.Event yet. It is +// deliberately NOT folded into ToolInput, because ToolInput becomes the +// Cedar evaluation context; injecting a synthetic attribute there would +// make it policy-visible and would change the parameters hash recorded on +// every decision. +// - summary: only present on the compaction event, which is skipped outright. +// It carries verbatim conversation content and must not reach the ledger +// without going through payload capture. +type devinHookInput struct { + HookEventName string `json:"hook_event_name"` + SessionID string `json:"session_id"` + ToolName string `json:"tool_name"` + ToolInput json.RawMessage `json:"tool_input"` + ToolResponse json.RawMessage `json:"tool_response"` + ToolUseID string `json:"tool_use_id"` +} + +// devinHookOutput is the decision object Devin parses from hook stdout. An +// empty object is the "no directive, continue" form. +type devinHookOutput struct { + Decision string `json:"decision,omitempty"` + Reason string `json:"reason,omitempty"` +} + +// DecodeDevinEvent translates a Devin hook payload into a hook.Event. +// +// Events Devin emits that carry no authorization decision — and any event name +// this adapter does not recognize — are reported as hook.ErrSkipEvent rather +// than as a decode failure. That distinction matters: the Devin runtime treats +// a non-zero hook exit as "block the tool call", so failing on an event we +// simply do not handle would stall the agent on a legitimate lifecycle signal. +// +// A payload that is malformed or missing its event name is still an error. That +// indicates a bug or a contract change rather than an event we chose not to +// handle, and failing closed on it is the safer default. +func DecodeDevinEvent(input []byte, agentName string) (hook.Event, error) { + var h devinHookInput + if err := decodeUseNumber(input, &h); err != nil { + return hook.Event{}, fmt.Errorf("devin: decode hook input: %w", err) + } + if h.HookEventName == "" { + return hook.Event{}, fmt.Errorf("devin: hook event name missing") + } + + hookName := hook.HookName(h.HookEventName) + if !devinSupportedHook(hookName) { + return hook.Event{}, fmt.Errorf("devin: event %s: %w", h.HookEventName, hook.ErrSkipEvent) + } + + toolInput, err := normalizeDevinToolInput(h.ToolInput) + if err != nil { + return hook.Event{}, err + } + return hook.Event{ + // Devin session IDs are already namespaced (devin-), so unlike + // the Codex adapter there is no prefix to add and no synthetic ID to + // mint — session_id and tool_use_id map straight through. + SessionID: h.SessionID, + Agent: agentName, + HookName: hookName, + ToolName: h.ToolName, + ToolInput: toolInput, + ToolResponse: normalizeToolResponse(h.ToolResponse), + ToolUseID: h.ToolUseID, + }, nil +} + +// EncodeDevinResult renders a decision in the form Devin parses from stdout. +// +// Only pre-tool-use carries a decision; every other event is observation and +// must emit no directive. Anything other than an explicit allow blocks, so an +// unrecognized decision value fails closed rather than letting a tool call +// through. +func EncodeDevinResult(hookEventName string, result hook.Result) ([]byte, error) { + if hook.HookName(hookEventName) != hook.HookPreToolUse { + return json.Marshal(devinHookOutput{}) + } + + // Devin applies an updated tool input by REPLACING the original wholesale + // rather than merging it. Emitting a partial rewrite would silently drop + // every field the policy did not restate — turning a redaction into an + // arbitrary change to the agent's command — and the post-tool-use hook + // would then observe only the rewritten input, so the original would never + // reach the ledger. Until input rewriting is modelled end to end, refuse + // visibly instead of rewriting wrongly. + if result.UpdatedInput != nil { + return json.Marshal(devinHookOutput{ + Decision: devinDecisionBlock, + Reason: "Kontext: policy requested an input rewrite, which this adapter does not support; refusing instead of altering the request.", + }) + } + + if result.Decision != hook.DecisionAllow { + return json.Marshal(devinHookOutput{ + Decision: devinDecisionBlock, + Reason: result.ClaudeReason(), + }) + } + return json.Marshal(devinHookOutput{}) +} + +// devinSupportedHook reports the Devin events this adapter translates. Devin +// also emits compaction and permission-request events; both are skipped, as is +// any event name added in future, so an unhandled event never blocks a tool +// call. +func devinSupportedHook(hookName hook.HookName) bool { + switch hookName { + case hook.HookSessionStart, + hook.HookPreToolUse, + hook.HookPostToolUse, + hook.HookSessionEnd: + return true + default: + return false + } +} + +// normalizeDevinToolInput decodes tool_input into the map the rest of the +// pipeline expects. An absent or null input yields nil. A non-object input is +// not expected from Devin; it is wrapped rather than rejected so an unforeseen +// shape degrades to a recordable event instead of blocking the agent. +func normalizeDevinToolInput(raw json.RawMessage) (map[string]any, error) { + trimmed := bytes.TrimSpace(raw) + if len(trimmed) == 0 || string(trimmed) == "null" { + return nil, nil + } + var obj map[string]any + if err := decodeUseNumber(trimmed, &obj); err == nil { + return obj, nil + } + var value any + if err := decodeUseNumber(trimmed, &value); err != nil { + return nil, fmt.Errorf("devin: decode tool input: %w", err) + } + return map[string]any{"value": value}, nil +} diff --git a/internal/hookruntime/devin_test.go b/internal/hookruntime/devin_test.go new file mode 100644 index 00000000..b6bc65f9 --- /dev/null +++ b/internal/hookruntime/devin_test.go @@ -0,0 +1,298 @@ +package hookruntime + +import ( + "encoding/json" + "errors" + "strings" + "testing" + + "github.com/kontext-security/kontext-cli/internal/hook" +) + +func TestDecodeDevinEventMapsPreToolUsePayload(t *testing.T) { + t.Parallel() + + input := []byte(`{ + "hook_event_name": "PreToolUse", + "session_id": "devin-02714895131a4094893ce70889e3f2b6", + "prompt_id": "prompt-1", + "tool_name": "exec", + "tool_use_id": "toolu_9", + "tool_input": {"command": "curl https://example.com"} + }`) + + event, err := DecodeDevinEvent(input, "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + if event.HookName != hook.HookPreToolUse { + t.Fatalf("HookName = %q, want PreToolUse", event.HookName) + } + // Devin session IDs arrive already namespaced; the adapter must not add a + // prefix or mint a replacement. + if event.SessionID != "devin-02714895131a4094893ce70889e3f2b6" { + t.Fatalf("SessionID = %q, want the payload value verbatim", event.SessionID) + } + if event.ToolName != "exec" { + t.Fatalf("ToolName = %q, want exec", event.ToolName) + } + if event.ToolUseID != "toolu_9" { + t.Fatalf("ToolUseID = %q, want toolu_9", event.ToolUseID) + } + if event.Agent != "devin" { + t.Fatalf("Agent = %q, want devin", event.Agent) + } + if got := event.ToolInput["command"]; got != "curl https://example.com" { + t.Fatalf("ToolInput[command] = %v, want the command", got) + } +} + +// prompt_id must not leak into ToolInput: ToolInput becomes the Cedar +// evaluation context, so a synthetic attribute there would be policy-visible +// and would change the recorded parameters hash. +func TestDecodeDevinEventKeepsPromptIDOutOfToolInput(t *testing.T) { + t.Parallel() + + event, err := DecodeDevinEvent([]byte( + `{"hook_event_name":"PreToolUse","prompt_id":"prompt-1","tool_name":"exec","tool_input":{"command":"ls"}}`, + ), "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + if len(event.ToolInput) != 1 { + t.Fatalf("ToolInput = %v, want only the original tool input keys", event.ToolInput) + } + if _, present := event.ToolInput["prompt_id"]; present { + t.Fatal("ToolInput contains prompt_id, want it excluded from the Cedar context") + } +} + +func TestDecodeDevinEventCapturesToolResponse(t *testing.T) { + t.Parallel() + + event, err := DecodeDevinEvent([]byte( + `{"hook_event_name":"PostToolUse","tool_name":"exec","tool_input":{},"tool_response":{"success":false,"error":"boom"}}`, + ), "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + if event.ToolResponse["success"] != false { + t.Fatalf("ToolResponse[success] = %v, want false", event.ToolResponse["success"]) + } + if event.ToolResponse["error"] != "boom" { + t.Fatalf("ToolResponse[error] = %v, want boom", event.ToolResponse["error"]) + } +} + +func TestDecodeDevinEventPreservesLargeIntegerPrecision(t *testing.T) { + t.Parallel() + + event, err := DecodeDevinEvent([]byte( + `{"hook_event_name":"PreToolUse","tool_name":"exec","tool_input":{"id":12345678901234567890}}`, + ), "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + number, ok := event.ToolInput["id"].(json.Number) + if !ok { + t.Fatalf("ToolInput[id] = %T, want json.Number", event.ToolInput["id"]) + } + if number.String() != "12345678901234567890" { + t.Fatalf("ToolInput[id] = %s, want the exact literal", number) + } +} + +// Events Devin emits that carry no decision, and any event name added later, +// must report ErrSkipEvent. Reporting a decode failure would exit non-zero, +// which the Devin runtime reads as "block the tool call". +func TestDecodeDevinEventSkipsEventsWithoutDecisions(t *testing.T) { + t.Parallel() + + for _, name := range []string{"PostCompaction", "PermissionRequest", "SomeFutureEvent"} { + t.Run(name, func(t *testing.T) { + t.Parallel() + + _, err := DecodeDevinEvent([]byte(`{"hook_event_name":"`+name+`","session_id":"devin-1"}`), "devin") + if !errors.Is(err, hook.ErrSkipEvent) { + t.Fatalf("DecodeDevinEvent() error = %v, want ErrSkipEvent", err) + } + }) + } +} + +func TestDecodeDevinEventTranslatesSessionLifecycle(t *testing.T) { + t.Parallel() + + for name, want := range map[string]hook.HookName{ + "SessionStart": hook.HookSessionStart, + "SessionEnd": hook.HookSessionEnd, + "PostToolUse": hook.HookPostToolUse, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + event, err := DecodeDevinEvent([]byte(`{"hook_event_name":"`+name+`","session_id":"devin-1"}`), "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + if event.HookName != want { + t.Fatalf("HookName = %q, want %q", event.HookName, want) + } + }) + } +} + +// A malformed payload is a contract violation rather than an event we chose not +// to handle, so it must stay an error and must not be mistaken for a skip. +func TestDecodeDevinEventRejectsMalformedInput(t *testing.T) { + t.Parallel() + + for name, input := range map[string]string{ + "missing event name": `{"tool_name":"exec"}`, + "not json": `{`, + "trailing value": `{"hook_event_name":"PreToolUse"} {}`, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + _, err := DecodeDevinEvent([]byte(input), "devin") + if err == nil { + t.Fatal("DecodeDevinEvent() error = nil, want a decode error") + } + if errors.Is(err, hook.ErrSkipEvent) { + t.Fatalf("DecodeDevinEvent() error = %v, want a failure rather than a skip", err) + } + }) + } +} + +func TestDecodeDevinEventAllowsAbsentToolInput(t *testing.T) { + t.Parallel() + + event, err := DecodeDevinEvent([]byte(`{"hook_event_name":"SessionStart","tool_input":null}`), "devin") + if err != nil { + t.Fatalf("DecodeDevinEvent() error = %v", err) + } + if event.ToolInput != nil { + t.Fatalf("ToolInput = %v, want nil", event.ToolInput) + } +} + +func TestEncodeDevinResultBlocksPreToolUseDeny(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(hook.HookPreToolUse.String(), hook.Result{ + Decision: hook.DecisionDeny, + Reason: "network egress is not permitted", + }) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + var decoded devinHookOutput + if err := json.Unmarshal(out, &decoded); err != nil { + t.Fatalf("json.Unmarshal(%s) error = %v", out, err) + } + if decoded.Decision != "block" { + t.Fatalf("Decision = %q, want block", decoded.Decision) + } + // Devin surfaces this string to the agent verbatim, so it must survive. + if decoded.Reason != "network egress is not permitted" { + t.Fatalf("Reason = %q, want the policy reason verbatim", decoded.Reason) + } +} + +func TestEncodeDevinResultAlwaysCarriesABlockReason(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(hook.HookPreToolUse.String(), hook.Result{Decision: hook.DecisionDeny}) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + var decoded devinHookOutput + if err := json.Unmarshal(out, &decoded); err != nil { + t.Fatalf("json.Unmarshal(%s) error = %v", out, err) + } + if strings.TrimSpace(decoded.Reason) == "" { + t.Fatal("Reason is empty, want the fallback reason so the agent is told why") + } +} + +func TestEncodeDevinResultEmitsNoDirectiveOnAllow(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(hook.HookPreToolUse.String(), hook.Result{ + Decision: hook.DecisionAllow, + Reason: "allowed", + }) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + if string(out) != "{}" { + t.Fatalf("EncodeDevinResult() = %s, want {}", out) + } +} + +// Only pre-tool-use gates a call. A deny recorded against an observation event +// must not turn into a directive Devin would act on. +func TestEncodeDevinResultNeverBlocksObservationEvents(t *testing.T) { + t.Parallel() + + for _, name := range []hook.HookName{hook.HookPostToolUse, hook.HookSessionStart, hook.HookSessionEnd} { + t.Run(name.String(), func(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(name.String(), hook.Result{ + Decision: hook.DecisionDeny, + Reason: "would deny", + }) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + if string(out) != "{}" { + t.Fatalf("EncodeDevinResult() = %s, want {}", out) + } + }) + } +} + +// Devin replaces tool_input wholesale rather than merging, so a partial rewrite +// would silently drop whatever the policy did not restate. Refuse visibly. +func TestEncodeDevinResultRefusesInputRewrite(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(hook.HookPreToolUse.String(), hook.Result{ + Decision: hook.DecisionAllow, + UpdatedInput: map[string]any{"command": "echo redacted"}, + }) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + var decoded devinHookOutput + if err := json.Unmarshal(out, &decoded); err != nil { + t.Fatalf("json.Unmarshal(%s) error = %v", out, err) + } + if decoded.Decision != "block" { + t.Fatalf("Decision = %q, want block so the request is not silently altered", decoded.Decision) + } + if !strings.Contains(decoded.Reason, "rewrite") { + t.Fatalf("Reason = %q, want it to explain the refusal", decoded.Reason) + } +} + +// An unrecognized decision value must fail closed rather than letting the call +// through. +func TestEncodeDevinResultFailsClosedOnUnknownDecision(t *testing.T) { + t.Parallel() + + out, err := EncodeDevinResult(hook.HookPreToolUse.String(), hook.Result{Decision: hook.Decision("maybe")}) + if err != nil { + t.Fatalf("EncodeDevinResult() error = %v", err) + } + var decoded devinHookOutput + if err := json.Unmarshal(out, &decoded); err != nil { + t.Fatalf("json.Unmarshal(%s) error = %v", out, err) + } + if decoded.Decision != "block" { + t.Fatalf("Decision = %q, want block", decoded.Decision) + } +} diff --git a/internal/hookruntime/runner.go b/internal/hookruntime/runner.go index 7995ece4..466659ad 100644 --- a/internal/hookruntime/runner.go +++ b/internal/hookruntime/runner.go @@ -1,6 +1,7 @@ package hookruntime import ( + "errors" "fmt" "io" @@ -21,6 +22,12 @@ func Run(stdin io.Reader, stdout, stderr io.Writer, codec Codec, evaluate func(h event, err := codec.DecodeHookEvent(input) if err != nil { + // An adapter that recognizes the event but does not translate it is not + // a failure: succeed without emitting a decision, so runtimes that read + // a non-zero exit as "block" let the tool call proceed. + if errors.Is(err, hook.ErrSkipEvent) { + return 0 + } fmt.Fprintf(stderr, "kontext: failed to decode hook input: %v\n", err) return 2 } diff --git a/internal/hookruntime/runner_test.go b/internal/hookruntime/runner_test.go index 6746a8fe..db15b600 100644 --- a/internal/hookruntime/runner_test.go +++ b/internal/hookruntime/runner_test.go @@ -2,6 +2,8 @@ package hookruntime import ( "bytes" + "errors" + "fmt" "testing" "github.com/kontext-security/kontext-cli/internal/hook" @@ -70,14 +72,62 @@ func TestRunWritesStructuredUnsupportedDecisionResult(t *testing.T) { } type stubCodec struct { - event hook.Event - out []byte + event hook.Event + out []byte + decodeErr error } func (s stubCodec) DecodeHookEvent([]byte) (hook.Event, error) { + if s.decodeErr != nil { + return hook.Event{}, s.decodeErr + } return s.event, nil } func (s stubCodec) EncodeHookResult(hook.Event, hook.Result) ([]byte, error) { return s.out, nil } + +// A skipped event is not a failure. Some agent runtimes read a non-zero hook +// exit as "block the tool call", so an event the adapter recognizes but does not +// translate must exit 0 and emit no decision. +func TestRunSucceedsSilentlyOnSkippedEvent(t *testing.T) { + t.Parallel() + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + + code := Run( + bytes.NewBufferString(`{"hook_event_name":"PostCompaction"}`), + stdout, + stderr, + stubCodec{decodeErr: fmt.Errorf("devin: event PostCompaction: %w", hook.ErrSkipEvent)}, + func(hook.Event) (hook.Result, error) { + t.Fatal("evaluate called for a skipped event") + return hook.Result{}, nil + }, + ) + + if code != 0 { + t.Fatalf("Run() exit code = %d, want 0", code) + } + if stdout.String() != "" { + t.Fatalf("stdout = %q, want no decision written", stdout.String()) + } +} + +func TestRunFailsOnDecodeErrorThatIsNotASkip(t *testing.T) { + t.Parallel() + + code := Run( + bytes.NewBufferString(`{`), + &bytes.Buffer{}, + &bytes.Buffer{}, + stubCodec{decodeErr: errors.New("malformed")}, + func(hook.Event) (hook.Result, error) { return hook.Result{}, nil }, + ) + + if code != 2 { + t.Fatalf("Run() exit code = %d, want 2", code) + } +} From a8b307bbef2c9700b7fed82e236bde232d3dde0a Mon Sep 17 00:00:00 2001 From: tumberger Date: Fri, 31 Jul 2026 11:18:52 +0200 Subject: [PATCH 2/2] feat(ledger): attribute Devin sessions to their provider hostedAgentIdentity mapped only Claude and Cowork, so a Devin session exported an empty agent_provider alongside agent "devin". The hosted ledger accepts that and the session still shows its own name, but an empty provider reads downstream as "provider unknown" rather than as the vendor it came from, and it cannot be grouped with other endpoints from the same vendor. Maps devin to ("cognition", "devin"), matching the existing shape where the provider is the vendor and the agent is the product. Verified against a running daemon: a Devin session now records ("cognition", "devin") beside a Claude session's ("anthropic", "claude_code"). Codex is still unmapped and still exports an empty provider. That is pre-existing and left alone here rather than changed as a drive-by. Co-Authored-By: Claude Opus 5 (1M context) --- internal/guard/store/sqlite/store.go | 6 ++++++ internal/guard/store/sqlite/store_test.go | 25 +++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/internal/guard/store/sqlite/store.go b/internal/guard/store/sqlite/store.go index 475c5bd0..11d5f799 100644 --- a/internal/guard/store/sqlite/store.go +++ b/internal/guard/store/sqlite/store.go @@ -1755,12 +1755,18 @@ func scanSession(scanner interface{ Scan(...any) error }) (SessionRecord, error) return record, nil } +// hostedAgentIdentity maps a local agent name to the (provider, agent) pair the +// hosted ledger records. An unmapped agent still exports its own name, but with +// an empty provider, which reads downstream as "provider unknown" rather than as +// the vendor it came from. func hostedAgentIdentity(agent string) (string, string) { switch strings.ToLower(strings.TrimSpace(agent)) { case "claude", "claude-code", "claude_code": return "anthropic", "claude_code" case "cowork", "claude-cowork", "claude_cowork": return "anthropic", "claude_cowork" + case "devin": + return "cognition", "devin" default: return "", strings.TrimSpace(agent) } diff --git a/internal/guard/store/sqlite/store_test.go b/internal/guard/store/sqlite/store_test.go index dac78e4c..3be90951 100644 --- a/internal/guard/store/sqlite/store_test.go +++ b/internal/guard/store/sqlite/store_test.go @@ -1862,3 +1862,28 @@ func assertRecordIDs(t *testing.T, records []LedgerRecord, want map[string]bool) } } } + +// The hosted ledger records a (provider, agent) pair per session. An unmapped +// agent exports an empty provider, which reads downstream as "provider unknown" +// rather than as the vendor it came from. +func TestHostedAgentIdentityMapsKnownAgents(t *testing.T) { + t.Parallel() + + for name, want := range map[string]struct{ provider, agent string }{ + "claude": {"anthropic", "claude_code"}, + "claude-code": {"anthropic", "claude_code"}, + "cowork": {"anthropic", "claude_cowork"}, + "devin": {"cognition", "devin"}, + " DeVin ": {"cognition", "devin"}, + "someoneelse": {"", "someoneelse"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + provider, agent := hostedAgentIdentity(name) + if provider != want.provider || agent != want.agent { + t.Fatalf("hostedAgentIdentity(%q) = (%q, %q), want (%q, %q)", name, provider, agent, want.provider, want.agent) + } + }) + } +}