Proj/dreaming

[danhdoan]

Summary

• Problem: ByteRover's context tree accumulates duplicates, stale drafts, and missed cross-domain patterns over time. No automated maintenance existed - users had to manually curate/review to keep the tree healthy.
• Why it matters: Over multiple curation cycles, content quality degrades. The agent's retrieval gets noisier, review queues grow, and the tree drifts from the knowledge it should represent.
• What changed: Ships the full dreaming feature — a 3-operation background pipeline (consolidate, synthesize, prune) triggered from CLI or idle-agent that runs an LLM to clean up the context tree with human-review gates on destructive actions.
• What did NOT change (scope boundary): No changes to brv query, brv search, brv curate, or the review system's core. Dreaming extends the review queue (dual-write pattern) but doesn't modify review command behavior.

Type of change

• Bug fix
• New feature
• Refactor (no behavior change)
• Documentation
• Test
• Chore (build, dependencies, CI)

Scope (select all touched areas)

• TUI / REPL
• Agent / Tools
• Server / Daemon
• Shared (constants, types, transport events)
• CLI Commands (oclif)
• Hub / Connectors
• Cloud Sync
• CI/CD / Infra

Linked issues

• Closes ENG-2060 (consolidate operation)
• Closes ENG-2061 (synthesize operation)
• Closes ENG-2062 (prune operation)
• Closes ENG-2063 (idle trigger + human review wiring)
• Closes ENG-2065 (dream state + log services)
• Closes ENG-2066 (LLM response parser + schemas)
• Closes ENG-2067 (dream trigger — four-gate eligibility)
• Closes ENG-2068 (dream executor + CLI + daemon wiring)
• Closes ENG-2070 (dream undo)
• Closes ENG-2121 (TUI display improvements)
• Closes ENG-2122 (--detach flag)
• Closes ENG-2126 (4 spec-violation gaps post-merge)
• Closes ENG-2128 (inline file contents in operation prompts)

Root cause (bug fixes only)

N/A - new feature.

Test plan

• Coverage added:
  • Unit test (dream-state, dream-lock, dream-log, trigger, executor, all 3 operations, undo)
  • Integration test
  • Manual verification via auto-test smoke suite (local-auto-test/dreaming/)
• Test file(s):
  • test/unit/infra/dream/dream-state-schema.test.ts
  • test/unit/infra/dream/dream-log-schema.test.ts
  • test/unit/infra/dream/dream-state-service.test.ts
  • test/unit/infra/dream/dream-lock-service.test.ts
  • test/unit/infra/dream/dream-log-store.test.ts
  • test/unit/infra/dream/dream-trigger.test.ts
  • test/unit/infra/dream/parse-dream-response.test.ts
  • test/unit/infra/dream/dream-undo.test.ts
  • test/unit/infra/dream/operations/consolidate.test.ts
  • test/unit/infra/dream/operations/synthesize.test.ts
  • test/unit/infra/dream/operations/prune.test.ts
  • test/unit/infra/executor/dream-executor.test.ts
• Key scenario(s) covered:
  • Four-gate eligibility (time, activity, queue, lock)
  • All three operations produce correct DreamOperation entries
  • Atomic read-modify-write for dream-state (no lost increments under concurrent curations)
  • PID-based lock with stale detection and rollback-on-error
  • Undo reverses all operation types (MERGE, TEMPORAL_UPDATE, SYNTHESIZE, ARCHIVE, SUGGEST_MERGE)
  • Cross-cycle pendingMerges consumption (prune writes → next consolidate reads)
  • Review queue dual-write (dream operations with needsReview: true surface in brv review pending)
  • Idle trigger dispatches dream automatically from brv-server.ts before killing idle agent
  • Auto-test smoke suite: 24/24 pass with real LLM (Google Gemini), including force scenarios for
    each operation type

User-visible changes

• brv dream [--force] [--detach] [--undo] [--format text|json] [--timeout <s>] —
  new CLI command. Runs three operations in sequence: consolidate, synthesize, prune.
• Idle agents automatically dispatch dream when eligibility gates pass (time +
  activity + queue + lock).
• brv review pending now includes dream-generated operations flagged for review.
• New files on disk per project:
  • .brv/dream-state.json — checkpoint state (lastDreamAt, curationsSinceDream, pendingMerges)
  • .brv/dream.lock — PID lock with mtime-as-checkpoint
  • .brv/dream-log/drm-*.json — per-dream operation history

Evidence

Unit tests: 6169 passing, 0 failing
Build: clean
Lint: 0 errors (warnings pre-existing)

Auto-test (real LLM via Google Gemini): 24/24 scenarios pass, including:

• force-merge: two near-duplicate files → MERGE executed, one file merged
• force-synthesize: three domains mentioning FlushCoordinator → SYNTHESIZE operation
  writes a type: synthesis file to the correct domain
• force-archive: draft with importance: 10 and "TODO... safe to delete" body → ARCHIVE
  operation moves the file to _archived/ with stub + full preserved

Auto-test suite lives at /Users/danh/workspace/projects/local-auto-test/dreaming/
and is not part of this PR (pre-release smoke test, not CI).

Checklist

• Tests added or updated and passing (npm test)
• Lint passes (npm run lint)
• Type check passes (tsc -b)
• Build succeeds (npm run build)
• Commits follow Conventional Commits format
• Documentation updated (dream flags documented via oclif --help)
• No breaking changes
• Branch is up to date with main

Risks and mitigations

• Risk: Dream triggers automatically on idle agents — could surprise users who don't know the feature exists.
  • Mitigation: Dream runs silently by default (--detach behavior from idle trigger). Operations requiring review (needsReview: true) surface in brv review pending before taking effect. User sees pending reviews and decides.
• Risk: LLM cost. Each dream invokes the LLM up to 3 times (one per operation). Idle triggers could run dreams without explicit user action.
  • Mitigation: Activity gate requires ≥3 curations since last dream. Time gate requires 12h between dreams. Auto-test smoke run costs ~10-15 LLM calls total; production cost scales with user activity, not calendar time.
• Risk: Wrong LLM output corrupts the context tree.
  • Mitigation: (1) MERGE and TEMPORAL_UPDATE operations preserve previousTexts in the dream log for brv dream --undo reversal. (2) All destructive operations require review before being applied (review backup store). (3) Outer catch {} ensures partial failures don't abort the whole dream.
• Risk: Inlined file contents in LLM prompts could exceed model context limits.
  • Mitigation (consolidate): capPayloadSize() enforces MAX_PAYLOAD_CHARS = 200_000 with changed-files-first eviction.
  • Mitigation (prune): max 20 candidates × 1000-char preview (~20KB ceiling).
  • Gap (synthesize): no explicit cap on inlined _index.md content - relies on summaries being small. Tracked as a follow-up to add a cap similar to consolidate's.
• Risk: Silent catch {} blocks hide real errors (e.g., LLM auth failures). Dream log shows errors: 0 even when every LLM call failed.
  • Mitigation: Not addressed in this PR. Tracked as a follow-up - summary.errors should reflect caught failures so users can see when LLM calls fail.

[github-actions]

Claude finished @danhdoan's task in 3m 59s —— View job

---

PR Review: Proj/dreaming

Tasks

• Gather context and understand PR scope
• Review core dream infrastructure (state, lock, log)
• Review operations (consolidate, synthesize, prune)
• Review executor and trigger
• Review CLI command and daemon wiring
• Review test coverage
• Post inline comments and final summary

---

Overall Assessment: 4/5

This is a well-engineered, well-tested feature. The architecture is sound — the four-gate eligibility model, the dual-write pattern for surfacing dream ops into brv review pending, and the PID-based lock with rollback-on-error are all correctly designed. The atomic tmp→rename write pattern is used consistently. Test coverage is extensive (12 test files, 6169 passing). Outside-in structure is followed throughout.

---

Critical Issue

writePendingMerge bypasses the per-file mutex (prune.ts:406)

PruneDeps.dreamStateService only exposes read() and write() — not update(). The function performs read() → mutate → write() without the mutex, which can silently overwrite a concurrent incrementCurationCount() from a curate task. The DreamStateService.write() doc explicitly warns about this pattern. The fix is to add update() to PruneDeps.dreamStateService and replace the RMW pair with a single update() call containing the dedup logic inside the updater. See inline comment for the suggested rewrite.

---

Other Issues

Severity	File	Issue
suggestion	dream-undo.ts:120	write() used directly (cross-process undo — OK, but merits a comment explaining why)
nitpick	dream-undo.ts:36	DreamUndoResult uses interface — should be type per CLAUDE.md (data-only shape)
suggestion	parse-dream-response.ts:14	Strategy 1 only matches ```json fences; unlabeled ``` fences fall through to Strategy 2 which can misfire on multi-JSON responses
tracking	dream-executor.ts:305	errors always 0 in computeSummary, making the summary.errors > 0 branch in formatResult unreachable (acknowledged in PR risks — noting here for follow-up)
tracking	synthesize.ts:53	No payload cap on inlined _index.md content (acknowledged in PR risks)
nitpick	synthesize.ts:228	Path traversal guard uses startsWith() instead of the shared isDescendantOf() used elsewhere
observation	dream-log-store.ts:87	Filtered list reads all entries before applying limit — harmless given maxEntries=50 cap

---

Strengths

• Concurrency model is well-reasoned. The two-phase loadAndClearPendingMerges design (unguarded read for hints, mutex-guarded update() for clear) is explicitly documented with the right rationale.
• Lock lifecycle is correct. release() on success, rollback(priorMtime) on error — the time gate can't be fooled by a failed dream.
• getById path traversal guard in DreamLogStore (ID_PATTERN.test(id)) is a clean defense against crafted IDs.
• Partial run preservation. allOperations is hoisted and pushed incrementally so a timeout or error mid-dream still captures completed work in the audit log and exposes it to --undo.
• reviewEntriesWritten flag prevents duplicate review entries when step 7 throws after step 6b succeeds — a subtle correctness detail handled correctly.
• Test coverage for the four-gate eligibility, atomic RMW, stale-lock detection, cross-cycle pendingMerges, and undo reversal for all operation types is comprehensive.

[github-actions]

Claude finished @danhdoan's task in 1m 18s —— View job

---

PR Review (Follow-up): Proj/dreaming

Tasks

• Understand trigger context (PR synchronize — new commits pushed addressing review feedback)
• Diff recent commits to identify what changed since last review
• Verify critical bug fix: writePendingMerge mutex bypass
• Check other review items were addressed
• Post updated review summary

---

Follow-up Review: All Addressed Items Verified ✓

Commit ea45b0a ("fix: resolve Review comments") addressed every actionable item from the first review. Here's a verification pass:

---

Critical Bug Fix — Verified ✓

writePendingMerge mutex bypass (prune.ts:406)

Fixed correctly. PruneDeps.dreamStateService now exposes update(), and writePendingMerge was rewritten to perform the dedup check inside the updater callback — matching the reviewer's suggested approach exactly. The atomic read-modify-write is now protected by the file mutex.

// Before (racy RMW pair):
const dreamState = await deps.dreamStateService.read()
// ...mutate...
await deps.dreamStateService.write({...dreamState, pendingMerges})

// After (correct atomic update):
await deps.dreamStateService.update((state) => {
  const alreadySuggested = ...
  if (alreadySuggested) return state
  return { ...state, pendingMerges: [...pendingMerges, {...}] }
})

Test stubs in prune.test.ts were updated to exercise the update() path — including the dedup case, which now correctly passes the pre-populated state into the updater and asserts the result is unchanged.

---

Other Issues — All Resolved ✓

Original Issue	Status
DreamUndoResult uses interface instead of type	Fixed — now export type DreamUndoResult = {...}
write() in dream-undo.ts:125 lacks explanation	Fixed — clear comment added explaining cross-process undo rationale and when update() would be needed
parse-dream-response.ts Strategy 1 misses unlabeled fences	Fixed — regex updated to /```(?:json)?\s*([\s\S]*?)```/
synthesize.ts path traversal used startsWith() instead of shared isDescendantOf()	Fixed — now uses isDescendantOf(absPath, contextTreeDir) with correct import

---

Remaining Tracking Items (acknowledged in PR risks, not blocking)

Item	Status
dream-executor.ts:305 — errors always 0, summary.errors > 0 branch unreachable	Tracked follow-up — no change needed for this PR
synthesize.ts:53 — No payload cap on inlined _index.md content	Tracked follow-up — no change needed for this PR
dream-log-store.ts:87 — Reads all entries before applying limit when filter is active	Harmless given maxEntries=50 cap — no change needed

---

Overall Assessment: 5/5

All critical and actionable issues from the first review are correctly resolved. The fix for writePendingMerge follows the suggested rewrite precisely, the test coverage was extended to verify the update() path and dedup invariant, and the stylistic nitpicks are clean. The two acknowledged follow-up items are correctly scoped to future PRs.

This PR is ready to merge.

[bao-byterover]

LGTM