fix: queue-based working memory capture (#179)

[dean0x]

Summary

• Replaces broken transcript extraction with queue-based turn capture from hook inputs
• UserPromptSubmit (preamble) captures user prompts to .memory/.pending-turns.jsonl
• Stop hook captures assistant_message (on end_turn only) to same queue, decouples throttle from capture
• Background updater uses mv-based atomic handoff with crash recovery via .pending-turns.processing

What was wrong

The previous implementation extracted user/assistant messages from session transcript JSONL, but most entries are tool_result/tool_use with no text content. In a typical session (89 "user" messages, only 8 with actual text), the tail -3 windowing almost always missed real content.

Design

Hook inputs already provide both data points directly — no transcript needed:

• UserPromptSubmit → prompt (user's text)
• Stop → assistant_message (full response) + stop_reason

Queue accumulates turns; throttle only gates background processing (not capture). 10-turn batch cap, overflow safety, crash recovery.

Test plan

• 5 new queue behavior tests in shell-hooks.test.ts
• 618/618 tests passing
• bash -n syntax validation on all 3 modified hooks
• 16 acceptance scenarios (stop_reason filtering, dual format, capture before throttle, DEVFLOW_BG_UPDATER guard, crash recovery, missing .memory/)
• Evaluator: 5/5 plan requirements aligned

Closes #179

[dean0x]

🔍 Code Review: Consolidated High-Confidence Findings (≥80%)

Overview

9 blocking/should-fix issues identified across 4 files. Multiple reviewers flagged the same concerns, indicating consistent patterns. Below, I've deduped and consolidated findings by location.

---

🔴 BLOCKING Issues (Fix Before Merge)

1. scripts/hooks/preamble — Stale Header Comment (95% confidence)

Line 5

The file header still claims:

# Zero file I/O beyond stdin — static injection only.

But this PR adds file I/O at lines 23–39 (appends to .memory/.pending-turns.jsonl). Misleading documentation that will confuse future maintainers.

Fix:

# Injects a detection-only preamble. Classification rules only — skill mappings live in devflow:router.
# Also captures user prompts to .memory/.pending-turns.jsonl for working memory.

---

2. scripts/hooks/preamble & stop-update-memory — Raw JSON Construction Instead of Helper (85% confidence)

Preamble lines 31–36 and stop-update-memory lines 74–79

Both scripts construct JSON objects using raw jq -n -c --arg patterns. The codebase already has a json_construct helper in the json-parse library that wraps exactly this pattern (with jq/node dual fallback).

Problem: Duplicated abstraction layer. If JSON construction changes are needed, multiple locations must be updated in sync.

Fix:

# In preamble (lines 31-36):
json_construct --arg role "user" --arg content "$_TRUNCATED_PROMPT" --argjson ts "$_TS" >> "$CWD/.memory/.pending-turns.jsonl"

# In stop-update-memory (lines 74-79):
json_construct --arg role "assistant" --arg content "$ASSISTANT_MSG" --argjson ts "$TS" >> "$QUEUE_FILE"

---

3. scripts/hooks/stop-update-memory — Mtime Logic Duplicated (92% confidence)

Lines 96–100

The stop hook inlines the Linux/macOS stat mtime detection, while background-memory-update defines a reusable get_mtime() function for the same operation. This pattern also appears in session-start-memory.

Problem: Two competing approaches for the identical operation. If stat detection needs a fix (new platform), both locations must be updated in sync.

Fix: Extract get_mtime() into a shared sourced file, or reuse the existing function from background-memory-update.

---

4. scripts/hooks/stop-update-memory — Queue Overflow Logic Duplicated (85% confidence)

Lines 82–89 (also in background-memory-update lines 107–111)

The overflow safety pattern (check >200 lines, truncate to 100 via tail/mv) appears in both scripts with identical thresholds and idioms (wc -l | tr -d ' ', tail > .tmp && mv).

Problem: DRY violation. If thresholds need tuning, two locations must be updated.

Fix: Extract a shared cap_file_lines() function into a sourced helper.

---

5. scripts/hooks/background-memory-update — Per-Line Subprocess Spawning (95% confidence)

Lines 148–181

The turn-parsing while-read loop calls json_field twice per JSONL line (role and content). Each invocation spawns a jq or node subprocess. With the 20-line cap (10 turns × 2 lines), this creates 40 subprocess spawns in sequence.

Problem: This is the exact pattern documented in PF-006 (per-line jq spawning adds 1–3s latency). Reintroducing it adds measurable latency and extends lock hold time.

Fix: Use single-pass jq -s (slurp) to extract all roles/contents at once:

if [ "$_HAS_JQ" = "true" ]; then
  PARSED=$(echo "$ENTRIES" | jq -r '[.role, .content] | @tsv')
else
  PARSED=$(echo "$ENTRIES" | node -e "d='';process.stdin.on('data',c=>d+=c);process.stdin.on('end',()=>{d.trim().split('\n').filter(Boolean).forEach(l=>{try{const o=JSON.parse(l);console.log(o.role+'\t'+o.content)}catch(e){}})})")
fi

CURRENT_USER=""
while IFS=$'\t' read -r ROLE CONTENT; do
  # ... same pairing logic, but ROLE/CONTENT already extracted
done <<< "$PARSED"

---

6. tests/shell-hooks.test.ts — Shell Injection Risk in execSync (82% confidence)

Multiple test blocks: lines 772, 791, 816, 840

The test pattern echo '${input.replace(/'/g, "'\\''")}' | bash escapes single quotes but doesn't guard against shell metacharacters in dynamically constructed JSON (backticks, $(), newlines).

Problem: While tests have controlled inputs today, the pattern is fragile. Test data evolution could cause unexpected failures.

Fix: Use safer input option:

execSync(`bash "${STOP_HOOK}"`, {
  input: JSON.stringify({ cwd: tmpDir, stop_reason: 'end_turn', assistant_message: 'test' }),
  stdio: ['pipe', 'pipe', 'pipe'],
});

---

🟡 SHOULD-FIX Issues (Gaps in Coverage/Documentation)

7. tests/shell-hooks.test.ts — Missing Content Array Format Test (85% confidence)

The stop hook handles two assistant_message formats: plain string and content array ([{type: "text", text: "..."}]). Tests only cover the string format.

Fix: Add test for content array:

it('stop_reason end_turn with content array — extracts text blocks', () => {
  const input = JSON.stringify({
    cwd: tmpDir,
    session_id: 'test-session',
    stop_reason: 'end_turn',
    assistant_message: [
      { type: 'text', text: 'First' },
      { type: 'tool_result', content: 'ignored' },
      { type: 'text', text: 'Second' },
    ],
  });
  // ... run hook and verify both text blocks in queue
});

---

8. tests/shell-hooks.test.ts — Missing Queue Overflow Truncation Test (82% confidence)

The stop hook implements queue overflow safety (>200 lines → truncate to 100). This has zero test coverage. Bugs in overflow handling would go undetected.

Fix: Pre-populate queue with 201 lines, trigger stop hook, verify truncation to ≤100 lines.

---

9. CLAUDE.md — Hook Count and Description Outdated (82–85% confidence)

Line 41: Says "Three shell-script hooks" but now involves four:

1. preamble (user prompt capture)
2. stop-update-memory (assistant response capture)
3. background-memory-update (queue processing)
4. session-start-memory (memory injection)

Line 108: Says WORKING-MEMORY.md is "overwritten each session" — now queue-based, so it accumulates turns across multiple stops.

Fix:

• Line 41: Update to "Four shell-script hooks" or rephrase to avoid counting
• Line 108: Update to "Auto-maintained by background updater (queue-fed by preamble + stop hooks)"

---

📊 Summary

Category	Count	Severity
Blocking	6	HIGH
Should-Fix	3	MEDIUM
Dedup Rate	4/13	—

Key Pattern: DRY violations (json_construct, mtime, queue overflow logic) + PF-006 reintroduction (subprocess spawning) + incomplete test coverage. Multiple reviewers flagged the same issues, indicating consistent concerns.

Recommendation: Address the 6 blocking issues before merge. They involve straightforward refactoring (extract helpers, replace duplicates) and test additions. The PF-006 pattern is the most complex fix but unlocks the biggest latency win.

---

Review compiled from 9 reviewer reports. Inline comments can be created on specific files if needed.

[dean0x]

Review Findings: High-Confidence Issues (≥80%)

This batch of inline comments flags blocking and should-fix issues from the code review. Lower-confidence suggestions (60-79%) are included in the summary comment below.

Issues Found

Total inline comments: 6

• Idempotency regression in addMemoryHooks (Consistency, 85%)
• Path resolution assumes project root (TypeScript, 85%)
• Double JSON parsing (TypeScript, 80%)
• Per-prompt subprocess overhead in hook (Performance, 85%)
• get_mtime duplication across hooks (Architecture, 85%)
• Preamble hook subprocess optimization (Performance, 80%)

---

Lower-Confidence Suggestions (60-79%)

These findings are consolidated here as they're architectural rather than blocking:

Queue Cleanup Logic Duplication (Complexity/Architecture, 70%)

Locations: src/cli/commands/init.ts:1413-1415, src/cli/commands/memory.ts:213-215

The .pending-turns.jsonl and .pending-turns.processing cleanup is implemented identically in two places. Extract to a shared utility in src/cli/utils/post-install.ts:

export async function cleanupQueueFiles(memoryDir: string): Promise<{ queue: boolean; processing: boolean }> {
  const queue = await fs.unlink(path.join(memoryDir, '.pending-turns.jsonl')).then(() => true).catch(() => false);
  const processing = await fs.unlink(path.join(memoryDir, '.pending-turns.processing')).then(() => true).catch(() => false);
  return { queue, processing };
}

Overflow Threshold Magic Numbers (Complexity, 85%)

Locations: scripts/hooks/stop-update-memory:92-98, scripts/hooks/background-memory-update:115-119

Both scripts use hardcoded 200/100 thresholds (200 lines trigger cleanup, keep 100). Define once:

QUEUE_MAX_LINES=200
QUEUE_TRIM_LINES=100

The cross-referencing # NOTE: comments are good but constants would be more robust.

Queue File Concurrency Assumption (Architecture, 70%)

Locations: scripts/hooks/prompt-capture-memory:37, scripts/hooks/stop-update-memory:67

Two hooks (prompt-capture-memory and stop-update-memory) both append to .pending-turns.jsonl. The >> append is atomic on POSIX for lines under PIPE_BUF, but a design comment would clarify this concurrency assumption:

# Atomic append (POSIX: lines under PIPE_BUF are atomic)
echo "$TURN_JSON" >> "$CWD/.memory/.pending-turns.jsonl"

Hook Lifecycle Abstraction Opportunity (Architecture, 65%)

Pattern: addMemoryHooks/removeMemoryHooks in memory.ts vs learning hooks in learn.ts

Both follow an identical pattern: iterate config map, match by marker substring, add/remove/count. A generic HookManager class could eliminate structural duplication. Lower priority as the current duplication is minor.

---

Summary by Category

Reviewer	Finding	Confidence	Action
Consistency	changed variable removal in addMemoryHooks	85%	Inline comment
TypeScript	process.cwd() path resolution	85%	Inline comment
TypeScript	Double JSON.parse()	80%	Inline comment
Performance	Per-prompt subprocess overhead	85%	Inline comment
Architecture	get_mtime duplication	85%	Inline comment
Performance	Preamble hook subprocess calls	80%	Inline comment
Complexity	Queue cleanup duplication	70%	Summary
Complexity	Magic number thresholds	85%	Summary
Architecture	Queue concurrency assumption	70%	Summary
Architecture	Hook lifecycle abstraction	65%	Summary

---

Deduplication Note

The following issues were reported by multiple reviewers and consolidated:

• get_mtime duplication: Architecture (85%), Complexity (90%), Consistency (70%)
• Queue overflow thresholds: Complexity (85%), Consistency (65%)
• Queue cleanup duplication: Architecture (85%), Complexity (70%)
• process.cwd() path resolution: TypeScript (85%), Architecture (82%)
• Per-prompt subprocess overhead: Performance (85%)
• Idempotency regression: Consistency (85%), TypeScript (82%)

Each deduplicated finding appears once in this comment at highest confidence.

---

Next Steps

1. Review inline comments and address blocking issues
2. Address should-fix items (double-parse, preamble optimization)
3. Consolidate lower-confidence findings as appropriate

Recommendation: Approve with conditions (fix blocking items, address should-fix items before merge).

---

Created by Claude Code git agent for devflow PR review. Findings ≥80% confidence elevated to inline comments.