fix: hook output was 10KB into a 2KB delivery window

docs/architecture.md

@@ -53,10 +53,11 @@ session continue rather than failing it.
 Tools compute results and hand them over; **nothing modifies code,
 files, or state behind the agent's back**. The write hook does not
 format your file — it hands the agent the formatted content to review
-and write. `templates`, `agents`, `spec template`, `todo add` all print
-content for the agent to write. The two exceptions are Procoder's own
-state (`todo close` flips a Status line; the index refreshes itself),
-never your code.
+and write, or, when that content is too large to deliver intact, the
+command that prints it. `templates`, `agents`, `spec template`,
+`todo add` all print content for the agent to write. The two exceptions
+are Procoder's own state (`todo close` flips a Status line; the index
+refreshes itself), never your code.
 
 Why: an agent that experiences its tools as collaborators uses them; an
 agent that gets silently overridden routes around its harness. And every
@@ -104,10 +105,23 @@ platform tool gaps), the fix lands in the shared path.
 ## The write hook, end to end
 
 `PostToolUse` on every Write/Edit: payload on stdin → format verdict
-(fixed content included if unformatted) → lint findings for that file →
-markdown/doc checks if prose → secrets scan → doc-drift notes → index
-refresh. Answer in the same turn, file untouched. Cost in practice:
-sub-second per write.
+(formatted content included if it fits the budget below, a pointer to
+`procoder format` if not) → lint findings for that file → markdown/doc
+checks if prose → secrets scan → doc-drift notes → index refresh. Answer
+in the same turn, file untouched. Cost in practice: sub-second per write.
+
+**The payload is budgeted, because delivery is.** A host does not inline
+unbounded hook output: past roughly two kilobytes Claude Code writes the
+output to a file and inlines only a preview of the first 2KB. So the hook
+keeps its whole message under that, gives the formatted body a bounded
+share of it — the only part that can be arbitrarily large — and DROPS any
+part that will not fit rather than truncating it. Half a finding is a
+finding whose meaning cannot be trusted, and half a formatted file is one
+somebody may write back over the whole thing.
+
+What was dropped is counted in the output, with `procoder check` named as
+the way to see all of it. A hook that quietly delivered four findings out
+of seven would be the silent green this tool exists to remove.
 
 ## Testing philosophy
 

docs/lifecycle.md

@@ -35,6 +35,7 @@ it carries; the repository is judged on all of it.
 <li><span class="procoder-verdict procoder-verdict--info">Info</span><span><b>state of play</b><i>Branch against the default, dirty count, active sprint, open stories, unlearned lessons, index freshness — inside a hard three-second budget.</i></span></li>
 <li><span class="procoder-verdict procoder-verdict--info">Info</span><span><b>deferred suites</b><i>Names the test suites the gate will not run here, so a green gate is never mistaken for a suite that passed.</i></span></li>
 <li><span class="procoder-verdict procoder-verdict--info">Info</span><span><b>version check</b><i>Asks GitHub off the critical path — a slow network cannot hold a session open.</i></span></li>
+<li><span class="procoder-verdict procoder-verdict--info">Info</span><span><b>receipt check</b><i>The payload opens with an instruction and closes with an end marker. A host that inlines only the first 2KB and writes the rest to a file leaves a reader holding a preview — the marker is how it can tell, and the instruction says to go and read the remainder.</i></span></li>
 </ul>
 </div>
 </section>

internal/hook/budget_ask_test.go

@@ -0,0 +1,78 @@
+package hook
+
+import (
+	"bytes"
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// TestQuestionsSurviveTheBudgetAsOneLineEach pins the flattening at its call
+// site, which the unit test of oneLine does not.
+//
+// Dropping oneLine does not make the payload oversized — fit simply discards
+// the whole ask part instead, and the budget assertion elsewhere still
+// passes. What is lost is the questions: the coder stops being told a human
+// decision is pending, which is the one thing askPart exists to do.
+//
+// So the assertion is that they SURVIVE, and survive as one line each.
+//
+// proved by: replacing `oneLine(q.Text, 160)` with `q.Text` makes the ask
+// part too large to fit, fit drops it, and this fails on the missing q&a
+// section.
+func TestQuestionsSurviveTheBudgetAsOneLineEach(t *testing.T) {
+	// No gofmt skip on purpose. Whatever verdict format.Check reaches
+	// without it, the questions still have to survive the budget — and
+	// without this, a machine with no gofmt has nothing exercising Run
+	// against the budget at all.
+	root := t.TempDir()
+
+	// A CLEAN file, so the format part contributes nothing and the ask queue
+	// is competing only with itself.
+	p := filepath.Join(root, "a.go")
+	if err := os.WriteFile(p, []byte("package probe\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.MkdirAll(filepath.Join(root, ".procoder", "ask"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	var dec strings.Builder
+	for i := 0; i < 6; i++ {
+		dec.WriteString("## A question with a body?\n\n")
+		dec.WriteString(strings.Repeat("Context that runs on and on. ", 60))
+		dec.WriteString("\n\n- one option\n- another option\n\n")
+	}
+	if err := os.WriteFile(filepath.Join(root, ".procoder", "ask", "decisions.md"),
+		[]byte(dec.String()), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	var out bytes.Buffer
+	if code := Run(payloadFor(t, p), &out); code != 0 {
+		t.Fatalf("hook exit %d — a hook must never fail the session", code)
+	}
+	if out.Len() == 0 {
+		t.Fatal("pending questions produced no payload at all")
+	}
+	var resp hookOutput
+	if err := json.Unmarshal(out.Bytes(), &resp); err != nil {
+		t.Fatal(err)
+	}
+	ctx := resp.HookSpecificOutput.AdditionalContext
+
+	if !strings.Contains(ctx, "q&a:") {
+		t.Fatalf("the questions did not survive the budget — the coder is not told a "+
+			"human decision is pending, which is the one thing this part exists for:\n%s", ctx)
+	}
+	for _, line := range strings.Split(ctx, "\n") {
+		if !strings.HasPrefix(line, "  - ") {
+			continue
+		}
+		if len([]rune(line)) > 220 {
+			t.Fatalf("a question line is %d runes — it was not flattened to a summary:\n%s",
+				len([]rune(line)), line)
+		}
+	}
+}

internal/hook/budget_run_test.go

@@ -0,0 +1,88 @@
+package hook
+
+import (
+	"bytes"
+	"encoding/json"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// TestRunNeverExceedsTheDeliveryBudget is the end-to-end half, and it exists
+// because the unit test of fit does not catch the mutation that matters:
+// putting strings.Join back at the call site leaves fit correct, tested and
+// unused, with every payload oversized again.
+//
+// The fixture is tuned so the parts genuinely overflow — a formatted body
+// just under its share, so it inlines rather than being pointed at, plus a
+// full ask queue. Without that the payload lands under budget on its own and
+// the test passes whether Run calls fit or not, which is the shape of a test
+// that proves nothing.
+//
+// proved by: replacing `fit(parts)` in Run with `strings.Join(parts, "\n\n")`
+// fails this on the budget assertion.
+func TestRunNeverExceedsTheDeliveryBudget(t *testing.T) {
+	if _, err := exec.LookPath("gofmt"); err != nil {
+		t.Skip("gofmt not installed")
+	}
+	root := t.TempDir()
+
+	// An unformatted file whose FORMATTED form lands just under the share,
+	// so it is inlined and takes up most of the budget.
+	var src strings.Builder
+	src.WriteString("package probe\n")
+	for i := 0; i < 40; i++ {
+		src.WriteString("func  Fn( ) {}\n")
+	}
+	p := filepath.Join(root, "a.go")
+	if err := os.WriteFile(p, []byte(src.String()), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	// A decisions ledger with more questions than askPart shows, each with a
+	// long body — the shape that put eight kilobytes of ask queue into every
+	// write in this repository.
+	if err := os.MkdirAll(filepath.Join(root, ".procoder", "ask"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	var dec strings.Builder
+	for i := 0; i < 8; i++ {
+		dec.WriteString("## Question ")
+		dec.WriteString(strings.Repeat("with a very long heading ", 12))
+		dec.WriteString("?\n\n")
+		dec.WriteString(strings.Repeat("Context that runs on and on. ", 40))
+		dec.WriteString("\n\n- one option\n- another option\n\n")
+	}
+	if err := os.WriteFile(filepath.Join(root, ".procoder", "ask", "decisions.md"),
+		[]byte(dec.String()), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	var out bytes.Buffer
+	if code := Run(payloadFor(t, p), &out); code != 0 {
+		t.Fatalf("hook exit %d — a hook must never fail the session", code)
+	}
+	if out.Len() == 0 {
+		t.Fatal("the fixture produced no payload, so this test asserts nothing")
+	}
+	var resp hookOutput
+	if err := json.Unmarshal(out.Bytes(), &resp); err != nil {
+		t.Fatalf("hook output is not the host's JSON shape: %v\n%s", err, out.String())
+	}
+	ctx := resp.HookSpecificOutput.AdditionalContext
+
+	if len(ctx) > maxContextBytes {
+		t.Fatalf("payload is %d bytes, past the %d budget the host actually delivers — "+
+			"everything after the first 2KB reaches the agent as a file path nothing makes it read",
+			len(ctx), maxContextBytes)
+	}
+	// The fixture is built to overflow. If nothing was dropped it stopped
+	// overflowing, and this test went back to proving nothing — fail loudly
+	// rather than pass quietly.
+	if !strings.Contains(ctx, "omitted") {
+		t.Fatalf("the fixture no longer overflows the budget, so this test no longer "+
+			"exercises fit — re-tune it:\n%s", ctx)
+	}
+}

internal/hook/budget_test.go

@@ -0,0 +1,149 @@
+package hook
+
+import (
+	"strings"
+	"testing"
+	"unicode/utf8"
+
+	"procoder/internal/format"
+)
+
+// proved by: joining every part unconditionally — what this did before —
+// produces a payload the host truncates, and what is lost is whatever
+// finding happened to be built last.
+func TestFitStopsAtTheDeliveryBudget(t *testing.T) {
+	big := strings.Repeat("x", maxContextBytes)
+	got := fit([]part{{text: "first"}, {text: big}, {text: "third"}})
+
+	if !strings.Contains(got, "first") {
+		t.Fatal("the first part was dropped")
+	}
+	if strings.Contains(got, big) {
+		t.Fatal("a part that did not fit was included anyway")
+	}
+	// No tolerance: the notice is reserved INSIDE the budget, so the budget
+	// is the budget. A +200 slack here is what let a 2144-byte payload pass
+	// against a 2000-byte constant.
+	if len(got) > maxContextBytes {
+		t.Fatalf("payload is %d bytes, past the %d budget", len(got), maxContextBytes)
+	}
+	if !strings.Contains(got, "omitted") {
+		t.Fatalf("findings were dropped and the output does not say so:\n%s", got)
+	}
+}
+
+// proved by: truncating a part instead of dropping it hands the agent half
+// a finding — and half a formatted file is one somebody may write back over
+// the whole file.
+func TestFitNeverTruncatesAPart(t *testing.T) {
+	a := strings.Repeat("a", 500)
+	b := strings.Repeat("b", 500)
+	c := strings.Repeat("c", 500)
+	d := strings.Repeat("d", 500)
+	got := fit([]part{{text: a}, {text: b}, {text: c}, {text: d}})
+	for _, p := range []string{a, b, c, d} {
+		if strings.Contains(got, p[:50]) && !strings.Contains(got, p) {
+			t.Fatal("a part appears in the output but not in full")
+		}
+	}
+	// The positive half. Without it this passes on an empty return, or on a
+	// return carrying nothing but the notice — a test that cannot fail.
+	for _, p := range []string{a, b, c} {
+		if !strings.Contains(got, p) {
+			t.Fatal("a part that fits was dropped")
+		}
+	}
+	if strings.Contains(got, d) {
+		t.Fatal("a part past the budget was kept")
+	}
+	if !strings.Contains(got, "omitted") {
+		t.Fatal("a part was dropped and the output does not say so")
+	}
+}
+
+// proved by: ordering the parts by consequence is one fix for the
+// starvation; a must-keep set is the other. Whichever is in use, a secret
+// must not be the finding the budget discards.
+func TestASecretSurvivesTheBudget(t *testing.T) {
+	filler := strings.Repeat("f", maxContextBytes-100)
+	secret := "== secrets: 3 finding(s) in the file you just wrote"
+	got := fit([]part{{text: filler}, {text: secret, keep: true}})
+
+	if !strings.Contains(got, secret) {
+		t.Fatalf("the secret finding was dropped to make room for a larger part:\n%s", got)
+	}
+	if len(got) > maxContextBytes {
+		t.Fatalf("keeping it pushed the payload to %d, past the %d budget", len(got), maxContextBytes)
+	}
+}
+
+// proved by: slicing a byte index through an em-dash returns invalid UTF-8,
+// which json.Marshal replaces with U+FFFD — the question reaches the agent
+// as mojibake. The ASCII-only case cannot catch it.
+func TestOneLineDoesNotSplitARune(t *testing.T) {
+	got := oneLine(strings.Repeat("é", 400), 160)
+	if !utf8.ValidString(got) {
+		t.Fatalf("invalid UTF-8: %q", got)
+	}
+	if n := len([]rune(got)); n != 160 {
+		t.Fatalf("truncated to %d runes, want 160", n)
+	}
+}
+
+// proved by: leaving q.Text as written lets one decision record carry its
+// whole body — blank lines and all — into a line the code calls a summary.
+// Five of those ran to eight kilobytes in this repository and crowded every
+// other finding out of the payload.
+func TestOneLineFlattensAndTruncates(t *testing.T) {
+	in := "a question\n\nwith a body\nover several lines"
+	got := oneLine(in, 160)
+	if strings.Contains(got, "\n") {
+		t.Fatalf("still multi-line: %q", got)
+	}
+	if got != "a question with a body over several lines" {
+		t.Fatalf("got %q", got)
+	}
+	if long := oneLine(strings.Repeat("z", 400), 160); len([]rune(long)) != 160 {
+		t.Fatalf("not truncated to the limit: %d runes", len([]rune(long)))
+	}
+}
+
+// proved by: the old 48KB threshold — twenty-four times the host's delivery
+// budget — let a formatted body be inlined under an instruction to write it
+// back, with only its first 2KB actually arriving.
+func TestFormattedBodyHasABoundedShare(t *testing.T) {
+	if maxInlineFormatted >= maxContextBytes {
+		t.Fatal("the formatted body may consume the whole payload, starving every finding after it")
+	}
+	msg := message(format.Result{
+		Verdict:   format.Unformatted,
+		File:      "big.go",
+		Tool:      "gofmt",
+		Formatted: []byte(strings.Repeat("x", maxInlineFormatted+1)),
+	})
+	if strings.Contains(msg, strings.Repeat("x", 100)) {
+		t.Fatal("a body over the share was inlined instead of pointed at")
+	}
+	if !strings.Contains(msg, "procoder format") {
+		t.Fatalf("no pointer to the full result:\n%s", msg)
+	}
+}
+
+// proved by: a keep part larger than the whole budget being dropped
+// silently — the must-keep finding is exactly the one that then vanishes,
+// and an omission nobody was told about is the silent green this tool
+// exists to remove.
+func TestAnOversizedKeepPartIsNamedInTheOmission(t *testing.T) {
+	oversized := strings.Repeat("s", maxContextBytes+100)
+	got := fit([]part{{text: "a small finding"}, {text: oversized, keep: true}})
+
+	if strings.Contains(got, oversized) {
+		t.Fatal("a part larger than the whole budget was delivered anyway")
+	}
+	if len(got) > maxContextBytes {
+		t.Fatalf("the payload is %d bytes against a %d budget", len(got), maxContextBytes)
+	}
+	if !strings.Contains(got, "1 finding(s) omitted") {
+		t.Fatalf("the oversized keep part vanished without being named:\n%s", got)
+	}
+}