fix: learning system empty-field validation + debug log relocation

[dean0x]

Summary

Fixes the learning system's background-learning script where Sonnet returns observations with empty fields, causing a self-matching feedback loop. Adds field validation, transcript quality gate, EXISTING_OBS filtering, debug mode, and relocates debug logs to user scope (~/.devflow/logs/).

Changes

• Validate observation fields (rule, pattern, workflow) before storage
• Skip transcripts with <3 observations from Sonnet (quality gate)
• Filter EXISTING_OBS from considerations when generating new
• Add DEBUG env var support for detailed logging
• Move debug logs from project to user scope for persistent troubleshooting
• Update CLAUDE.md and README.md with learning debug guidance

Related Issues

Relates to learning system validation and debug infrastructure

Testing

• Verify background-learning script with DEBUG=1 enabled
• Check logs appear in ~/.devflow/logs/ not .memory/
• Validate empty fields are filtered from observations

[dean0x]

Validation mismatch: Weaker check in migration purge

The migration auto-purge uses obj.id && obj.type && obj.pattern (JS truthiness check), but isLearningObservation() is much stricter — it validates:

• confidence is a number
• observations is a number
• status is one of three enum values
• evidence is an array
• details is a string
• Fields are non-empty strings (not just truthy)

Result: Entries like {"id":"x","type":"invalid","pattern":"y"} survive migration but get rejected by --purge, creating inconsistent cleanup.

Fix: Import and reuse parseLearningLog from learn.ts instead of the inline filter.

---

Blocking issue (HIGH: 90% confidence) — flagged by architecture, consistency, complexity, and TypeScript reviewers.

[dean0x]

Test writes to real ~/.devflow/ directory instead of sandbox

This test calls migrateMemoryFiles() which internally resolves getDevFlowDirectory() to the actual ~/.devflow/ path. While cleanup runs after the test, this creates risk:

• Test crashes before cleanup → orphaned files in user home
• CI environments may have different permissions/constraints
• Parallel test runs could collide on the same path

Fix: Inject the devflow directory path or refactor migrateMemoryFiles to accept an optional devflowDir parameter for testability.

---

Blocking issue (HIGH: 90% confidence) — flagged by tests reviewer.

[dean0x]

Log files created with world-readable permissions

The mkdir -p call creates directories with default umask (typically 022), making the log files world-readable (mode 644) on multi-user systems. These logs contain session IDs, timestamps, and (in debug mode) excerpts of session content.

Fix: Add restrictive permissions after directory creation:

mkdir -p "$_LOG_DIR"
chmod 700 "$_LOG_DIR"

Apply across all four hook scripts (background-learning, background-memory-update, stop-update-learning, stop-update-memory).

---

Should-fix issue (MEDIUM: 82-85% confidence) — flagged by security reviewer.

[dean0x]

rotate_log inconsistency between background-learning and background-memory-update

background-learning now has debug-aware log rotation (500/250 lines when DEBUG=true, 100/50 otherwise), but background-memory-update still uses hardcoded 100/50. Both scripts write to the same ~/.devflow/logs/ directory, so users enabling debug mode would expect consistent retention behavior.

Fix: Either apply the same debug-aware rotation to both scripts, or extract rotate_log into the shared json-parse source file. At minimum, add the early-return guard to background-memory-update:

rotate_log() {
  if [ ! -f "$LOG_FILE" ]; then return; fi
  # ... rest of rotation logic
}

---

Should-fix issue (MEDIUM: 85% confidence) — flagged by consistency and architecture reviewers.

[dean0x]

Code Review Summary: #161 - Learning System Validation & Debug Logs

Inline Comments (High Confidence: ≥80%)

4 inline comments created above for distinct blocking and should-fix issues:

1. Validation mismatch in migration (90% confidence) — post-install.ts:623 blocks merge
2. Migration test filesystem access (90% confidence) — memory.test.ts:360 blocks merge
3. Log file permissions (82-85% confidence) — background-learning:21 should-fix
4. rotate_log divergence (85% confidence) — Hook scripts should-fix

Lower-Confidence Findings (60-79%)

The following issues were found during review but fall below the 80% inline-comment threshold:

Code Quality

• learn.ts:287 — learnCommand.action() handler exceeds 200 lines with 7 flag branches
• learn.ts:289-345 — Duplicated raw-line-count + invalid-detection pattern (3 occurrences)
• background-learning:398-410 — Validation logic is untestable (embedded in 661-line shell script, PF-004)
• learn.ts:457-478 — purge command lacks confirmation prompt before destructive write

Type Safety

• learn.ts:44,46 — Type guard chains .length on unknown type without explicit cast

Documentation & Testing

• tests/learn.test.ts — loadLearningConfig() tests lack debug field assertions in override scenarios
• src/cli/commands/learn.ts — Missing integration test for --purge CLI behavior
• CLAUDE.md:43 — Self-Learning paragraph outdated
• scripts/hooks/background-learning:91-92 — SYNC comments lack field count hint

Skipped Findings (Pre-existing / Tracked)

• Project slug duplication (5 locations) — Architectural debt, tracked as PF-005
• Shell/TypeScript validation divergence — obs_ prefix check inconsistency, tracked as PF-005
• background-learning script size (661 lines) — Pre-existing PF-004, not worsened meaningfully
• Per-line subprocess spawning — Pre-existing PF-006, not modified

---

4 inline comments created. Generated by Claude Code review system.