Refactor adapter handlers and cmdHook to accept io.Reader/io.Writer params

[gregology]

Fixes #77

Problem

cmdHook — the function that runs every single time an agent triggers a hook — had zero test coverage. The individual handlers (HandleClaudeHook, HandlePiHook) were well-tested, but the dispatch logic connecting them was a blind spot. A regression in IsPiHook detection could silently misroute all input and we'd never know until a user hit it.

Approach

Thread io.Reader/io.Writer params through the full call chain: cmdHook -> HandleClaudeHook/HandlePiHook. This is Approach B from the issue discussion.

Every other cmd function (cmdContext, cmdDecisions, cmdValidate, cmdInit) already accepts io.Writer params. The adapter handlers were the odd ones out, writing directly to os.Stdout/os.Stderr. Now they're consistent with everything else.

With explicit writers, testing cmdHook dispatch is just bytes.NewReader for input and bytes.Buffer for output. No pipe gymnastics, no os.Stdout swapping. The existing captureStdout helper (which did pipe redirection) is gone — all ~24 adapter tests now use buffers too.

Why not the smaller approaches

Approach A (only refactor cmdHook, leave handlers alone) would've gotten dispatch coverage but left the handlers inconsistent — still writing to os.Stdout directly while every cmd function uses injected writers. Half the job.

Approach C (pipe-based tests, no prod changes) works but pipe manipulation in tests is fragile and harder to read. The existing captureStdout helper was already a code smell — it swapped a global, which is inherently racy even if the tests happen to pass today.

New tests

TestCmdHook covers the dispatch paths that were previously untested:

• Claude JSON -> routes to Claude handler, produces Claude-format output
• Pi JSON -> routes to Pi handler, produces Pi-format output
• Empty input -> graceful error
• Malformed JSON -> graceful error
• "source": "pi" with Claude-shaped fields -> still routes to Pi handler

Dev Ghost metrics

Total duration: 9m 30s
Turns: 125
Tool calls: 94
Tokens: 2,222,208 input / 21,350 output

Stage	Model	Duration	Turns	Tool calls	Tokens (in/out)	Cache read	Cache creation
triage	claude-opus-4-6	1m 47s	38	36	45,028 / 1,225	37,566	7,458
implementation	claude-opus-4-6	5m 12s	57	36	1,436,037 / 16,803	1,374,330	61,629
evaluation	claude-opus-4-6	0m 48s	9	7	56,545 / 1,125	46,078	10,463
docs_review	claude-opus-4-6	1m 14s	20	14	529,755 / 1,559	460,043	69,696
craft_pr	claude-opus-4-6	0m 27s	1	1	154,843 / 638	82,587	72,253

Resolves #77

[gregology]

Threads io.Reader/io.Writer through the cmdHook -> HandleClaudeHook/HandlePiHook chain, bringing the hook path in line with every other cmd function. Removes the racy captureStdout pipe-swap helper and replaces it with plain bytes.Buffer across all ~24 adapter tests. Adds TestCmdHook to cover the previously-untested dispatch logic.

---

Warnings

cmd/sctx/main_test.go ~line 492 — test assertions are too weak

The TestCmdHook cases check for JSON field names as substrings (hookSpecificOutput, additionalContext), but those strings appear in every successful response regardless of content. The "claude hook dispatches to claude handler" case would pass even if the handler injected completely wrong context — hookSpecificOutput is just a struct key.

To actually verify dispatch, unmarshal the output into the typed struct and assert on real content — e.g. check that additionalContext contains "hook test context" from the fixture AGENTS.yaml, or that permissionDecision is "allow". That lines up with the project's "every test represents a real user scenario" philosophy.

internal/adapter/claude.go line 88, pi.go line 100 — silent write error discard

_, _ = fmt.Fprintln(errOut, w)

The blank identifiers swallow write errors on the new errOut writer. Warnings are best-effort so this is probably fine in practice, but a short comment like // best-effort; write failures non-fatal would save the next reader from wondering if it's an oversight.

---

Notes

cmd/sctx/main_test.go ~line 501 — notWantOut unused on the routing edge case

The "pi source with claude-shaped payload" case is a good one, but it only checks that additionalContext is present — it doesn't check that Claude-format output is absent. The test struct already has a notWantOut field sitting right there. Adding notWantOut: "hookSpecificOutput" would make this actually prove routing went to Pi and not Claude.

cmd/sctx/main_test.go — errOut never inspected

No test case checks the errOut buffer. A case that triggers a warning (unresolvable path, etc.) and asserts it shows up in errOut would exercise the full plumbing.

internal/adapter/claude.go line 55 — exported signature change

HandleClaudeHook and HandlePiHook signatures changed, which would be breaking if anyone imported the package externally. Since it's under internal/, Go prevents that — just flagging it so nobody worries about it during review.

---

Clean refactor overall. The prod code changes are straightforward and correct. The main thing worth addressing is the test assertions — right now they prove "something was written" but not "the right thing was written," which undercuts the value of the new TestCmdHook suite.

Dev Ghost metrics

Total duration: 2m 42s
Turns: 38
Tool calls: 32
Tokens: 219,243 input / 3,877 output

Stage	Model	Duration	Turns	Tool calls	Tokens (in/out)	Cache read	Cache creation
analyze	claude-opus-4-6	1m 22s	27	25	62,729 / 1,139	46,129	16,596
review_findings	claude-opus-4-6	0m 52s	10	6	99,685 / 1,934	79,949	19,731
draft	claude-opus-4-6	0m 26s	1	1	56,829 / 804	34,308	22,518

[gregology]

Address these issues

cmd/sctx/main_test.go ~line 492 — test assertions are too weak

The TestCmdHook cases check for JSON field names as substrings (hookSpecificOutput, additionalContext), but those strings appear in every successful response regardless of content. The "claude hook dispatches to claude handler" case would pass even if the handler injected completely wrong context — hookSpecificOutput is just a struct key.

To actually verify dispatch, unmarshal the output into the typed struct and assert on real content — e.g. check that additionalContext contains "hook test context" from the fixture AGENTS.yaml, or that permissionDecision is "allow". That lines up with the project's "every test represents a real user scenario" philosophy.

internal/adapter/claude.go line 88, pi.go line 100 — silent write error discard

_, _ = fmt.Fprintln(errOut, w)

The blank identifiers swallow write errors on the new errOut writer. Warnings are best-effort so this is probably fine in practice, but a short comment like // best-effort; write failures non-fatal would save the next reader from wondering if it's an oversight.

[gregology]

Addressed both requested changes:

• TestCmdHook now unmarshals output into ClaudeHookOutput / PiHookOutput and checks that AdditionalContext contains "hook test context" from the fixture AGENTS.yaml. This proves correct dispatch and content, not just field name presence. Extracted a runHook helper to keep cognitive complexity under the linter threshold.
• Added // best-effort; write failures non-fatal inline comments on the errOut writes in both claude.go and pi.go.

The three informational notes (unused notWantOut, uninspected errOut buffer, exported signature under internal/) weren't explicitly requested as changes so I left them alone.

Dev Ghost metrics

Total duration: 2m 43s
Turns: 38
Tool calls: 28
Tokens: 308,071 input / 7,524 output

Stage	Model	Duration	Turns	Tool calls	Tokens (in/out)	Cache read	Cache creation
analyze_feedback	claude-opus-4-6	0m 44s	13	12	60,058 / 806	48,946	11,108
implement	claude-opus-4-6	1m 41s	24	15	194,045 / 6,447	178,520	15,501
craft_update	claude-opus-4-6	0m 17s	1	1	53,968 / 271	26,839	27,126