Audit: fix-thought-content-nonempty

Task slug: fix-thought-content-nonempty Branch: feature/fix-thought-content-nonempty Worktree: E:/AMS/.worktrees/claude/fix-thought-content-nonempty Round: none (post-R83 hygiene from a code review) Base: origin/main @ 86e430fb Tier: T3 executor


1. Problem statement (review finding #3)

The β writeback gate (enforceWriteback at src/domains/tasks/writeback.ts:97) verifies only the presence of a thought_record row for the task_id — it runs SELECT COUNT(*), not a content-quality check. Combined with the ζ schema declaring content: z.string() (no .min(1)), an executor can satisfy the writeback contract by inserting a thought with content: "". The schema docstring at src/domains/trail/schema.ts:71-78 even endorses this: “empty allowed; a zero-content thought is valid if unusual”.

The contract today enforces presence, not quality. That is gameable.

2. Inventory of the surface

2.1 Schema layer (the canonical fix point)

File: src/domains/trail/schema.ts

Line Item Current Required change
71-78 TSDoc field invariants “content — string (empty allowed; a zero-content thought is valid if unusual).” Replace with non-empty rationale
85 Zod field content: z.string(), content: z.string().min(1, 'thought content must be non-empty')
170-188 computeHash body hashes content as a UTF-8 string DO NOT TOUCH — see §4

2.2 Repository tool input schema

File: src/domains/trail/repository.ts

Line Item Current Required change
114-119 CreateThoughtRecordInputSchema content: z.string(), content: z.string().min(1, '…')
122 ThoughtRecordToolInputSchema re-export unchanged (inherits from above)

2.3 Tests touching empty content

src/__tests__/trail-schema.test.ts:166   — accepts empty-string content (must be inverted)
src/__tests__/trail-schema.test.ts:333   — computeHash handles '' content (keep — hash function unchanged)
src/__tests__/domains/trail/repository.test.ts:160 — accepts empty content via createThoughtRecord (must be inverted)
src/__tests__/domains/tasks/writeback.test.ts      — uses 'test writeback content' (already non-empty; no changes needed)

The writeback tests insert via raw SQL, bypassing the Zod schema. They will continue to pass because they already use a non-empty literal ('test writeback content' at line 75). No semantic change needed there.

2.4 Documentation surface

File Line Note
src/db/migrations/003_thought_records.sql 12 Comment “(empty string allowed)” — update
docs/audits/p0-7-1-trail-schema-audit.md (search) Verify any “empty allowed” claim
docs/contracts/p0-7-2-thought-crud-contract.md 321 Test row 8: “empty-string content works” — invert to “rejected”
docs/packets/p0-7-1-trail-schema-packet.md 138, 271 Update spec + test 13 description
docs/verification/p0-7-1-trail-schema-verification.md 92 Historical run output — leave as historical record (verification file at SHA describes prior state). Do NOT rewrite past verification.
docs/agents/writeback-protocol.md Already prescribes non-empty multi-line content per §2.x; add a one-line cross-ref to the schema-level .min(1)

2.5 NOT in scope

  • computeHash in schema.ts:170 — bytewise stable, must not change. Existing chains in data/colibri.db and data/ams.db rely on the hash function being identity-stable for any input including ''. The new constraint blocks NEW empty thoughts at validation time; existing rows with content='' (if any) still re-hash to the same value when verified.
  • prev_hash semantics or ZERO_HASH constant.
  • .max() length policy (deferred).
  • Live Phase 0 DB content. We do not migrate or rewrite existing rows.

3. Risk assessment

Risk Severity Mitigation
Existing chains with content='' rows fail re-verification None — hash function unchanged; the row’s own hash still matches its inputs. The schema’s .min(1) only fires at insert time, not at verify time. n/a
Test fixture using content: '' to assert “accepts” inverts meaning Medium Replace with explicit “rejects” tests + add 2 new positive tests (schema, repository)
Writeback test loses gate-bypass scenario Low Writeback test never used schema path — uses raw SQL. Still tests the COUNT gate. Add a NEW test asserting that createThoughtRecord rejects empty content BEFORE the gate is reachable.
Donor heritage data/ams.db chains None Hash function untouched; legacy reads still verify.

4. Why the hash function is untouched (load-bearing fact)

computeHash accepts a content: string and runs sha256(canonical_JSON({...content...})). For any input (empty or non-empty), it is deterministic. The integrity of EVERY existing row in thought_records (Phase 0 colibri.db and donor ams.db) depends on this function being byte-identical to its prior implementations.

The fix is a validation-layer change, not a hash-layer change. Records that exist with content='' (if any) verify exactly as they did before — the schema’s .min(1) only blocks NEW writes.

5. Inventory results — file change list

Source code (3 files):

  1. src/domains/trail/schema.ts — TSDoc + .min(1)
  2. src/domains/trail/repository.ts — input schema .min(1)
  3. src/db/migrations/003_thought_records.sql — comment update only (no DDL change)

Tests (3 files):

  1. src/__tests__/trail-schema.test.ts — invert “accepts empty” → “rejects empty” + keep computeHash empty test (hash function unchanged)
  2. src/__tests__/domains/trail/repository.test.ts — invert “accepts empty” → “rejects empty”
  3. src/__tests__/domains/tasks/writeback.test.ts — add ONE new test: empty content rejected at thought-record-create time

Docs (3 files):

  1. docs/contracts/p0-7-2-thought-crud-contract.md — invert row 8
  2. docs/packets/p0-7-1-trail-schema-packet.md — update lines 138 + 271
  3. docs/agents/writeback-protocol.md — add one cross-ref note (optional)

Verification file (p0-7-1-trail-schema-verification.md) is a historical run record — not retroactively edited.

6. Out-of-scope observations

  • src/domains/proof/retention.ts:34 mentions EMPTY_CONTENT_HASH in a docstring but no such constant is exported. Pre-existing doc drift; not in this task’s scope.
  • Mock-content patterns elsewhere in the codebase use placeholder strings; none use empty strings outside the trail tests above. Survey complete.

Audit complete. Proceed to contract.


Back to top

Colibri — documentation-first MCP runtime. Apache 2.0 + Commons Clause.

This site uses Just the Docs, a documentation theme for Jekyll.