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
computeHashinschema.ts:170— bytewise stable, must not change. Existing chains indata/colibri.dbanddata/ams.dbrely on the hash function being identity-stable for any input including''. The new constraint blocks NEW empty thoughts at validation time; existing rows withcontent=''(if any) still re-hash to the same value when verified.prev_hashsemantics orZERO_HASHconstant..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):
src/domains/trail/schema.ts— TSDoc +.min(1)src/domains/trail/repository.ts— input schema.min(1)src/db/migrations/003_thought_records.sql— comment update only (no DDL change)
Tests (3 files):
src/__tests__/trail-schema.test.ts— invert “accepts empty” → “rejects empty” + keep computeHash empty test (hash function unchanged)src/__tests__/domains/trail/repository.test.ts— invert “accepts empty” → “rejects empty”src/__tests__/domains/tasks/writeback.test.ts— add ONE new test: empty content rejected at thought-record-create time
Docs (3 files):
docs/contracts/p0-7-2-thought-crud-contract.md— invert row 8docs/packets/p0-7-1-trail-schema-packet.md— update lines 138 + 271docs/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:34mentionsEMPTY_CONTENT_HASHin a docstring but no such constant is exported. Pre-existing doc drift; not in this task’s scope.- Mock-
contentpatterns elsewhere in the codebase use placeholder strings; none use empty strings outside the trail tests above. Survey complete.
Audit complete. Proceed to contract.