feat: [ENG-2065] dream state + log services by RyanNg1403 · Pull Request #378 · campfirein/byterover-cli

RyanNg1403 · 2026-04-13T08:28:38Z

Summary

Problem: ByteRover has no persistence layer for the dreaming (background memory consolidation) system — no way to track dream state, acquire execution locks, or log dream operations.
Why it matters: All other dream tasks (consolidate, synthesize, prune, trigger, executor) depend on these foundational services.
What changed: Added 5 new source files under src/server/infra/dream/ — Zod schemas for state and log entries, a fail-open state service, a PID-based lock service, and a file-based log store. Added DREAM_LOG_DIR and DREAM_LOG_ID_PREFIX to src/server/constants.ts.
What did NOT change (scope boundary): No existing services modified. No CLI commands added. No daemon wiring. This is purely the persistence layer.

Type of change

Scope (select all touched areas)

Linked issues

Closes ENG-2065
Related ENG-2059

Root cause (bug fixes only, otherwise write `N/A`)

N/A

Test plan

Coverage added:
- Unit test
- Integration test
- Manual verification only
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
Key scenario(s) covered:
- Schema validation (valid payloads, rejection of invalid data, discriminated unions)
- State service: fail-open read on missing/corrupt files, atomic write, incrementCurationCount
- Lock service: acquire/release/rollback, dead PID reclaim, stale lock detection
- Log store: save/getById roundtrip, list with filters, pruning, stale processing resolution, path traversal prevention

User-visible changes

None

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording

79 tests passing, 0 type errors, lint clean.

Checklist

Tests added or updated and passing (npm test)
Lint passes (npm run lint)
Type check passes (npm run typecheck)
Build succeeds (npm run build)
Commits follow Conventional Commits format
Documentation updated (if applicable)
No breaking changes (or clearly documented above)
Branch is up to date with main

Risks and mitigations

Risk: DreamLockService.tryAcquire() has a TOCTOU race window in the write-then-verify pattern.
- Mitigation: Spec-approved design trade-off. Single-daemon architecture makes collision near-impossible. Worst case (two dreams run) is non-destructive.
Risk: DreamStateService.incrementCurationCount() is not atomic under concurrent writers.
- Mitigation: Project task queue enforces single-writer (max concurrency = 1 per project). Documented in code.

Add dream infrastructure persistence layer: - dream-state-schema.ts: DreamStateSchema, PendingMergeSchema, EMPTY_DREAM_STATE - dream-log-schema.ts: DreamOperationSchema (CONSOLIDATE/SYNTHESIZE/PRUNE), DreamLogEntrySchema (completed/partial/error/processing/undone) - dream-state-service.ts: fail-open read, atomic write, incrementCurationCount - dream-lock-service.ts: PID-based lock with stale detection and write-then-verify - dream-log-store.ts: atomic save, Zod validation, monotonic IDs, stale processing resolution, pruning beyond maxEntries 79 tests passing, 0 type errors.

github-actions · 2026-04-13T08:28:51Z

PR Review: feat: [ENG-2065] dream state + log services

Tasks

Gather context and read changed files
Review source files (src/server/infra/dream/, src/server/constants.ts)
Review test files
Post inline comments
Post overall review summary

View job run

github-actions · 2026-04-13T08:31:37Z

PR Review: feat: [ENG-2065] dream state + log services

Tasks

Gather context and understand PR scope
Read source files (src/server/infra/dream/, src/server/constants.ts)
Read test files
Analyze for correctness, security, and code quality
Post inline comments
Post overall review summary

View job run

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ebf4ff97c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

- DreamStateService.read(): spread pendingMerges as new array to avoid mutating EMPTY_DREAM_STATE module constant - DreamLockService.tryAcquire(): add mkdir guard before writeFile to handle fresh projects where baseDir doesn't exist yet

github-actions · 2026-04-13T08:34:26Z

Claude finished @RyanNg1403's task in 4m 15s —— View job

PR Review: feat: [ENG-2065] dream state + log services

Tasks

Gather context and understand PR scope
Read source files (src/server/infra/dream/, src/server/constants.ts)
Read test files
Analyze for correctness, security, and code quality
Post inline comments
Post overall review summary

Overall: 4/5

Well-scoped persistence layer — clean atomic writes, solid fail-open semantics, good path-traversal prevention, and the two correctness bugs from the first review pass (pendingMerges shallow copy, missing mkdir guard in tryAcquire) have been addressed in commit c73de6c. The foundational work is trustworthy.

What's still open

#	File	Severity	Issue
1	`dream-lock-service.ts:95-102`	must-fix	`isProcessAlive` catches `EPERM` and returns `false` — a live process owned by another user would be treated as dead, allowing lock theft. See inline.
2	`dream-log-schema.ts:21`	should-fix	`confidence: z.number()` has no `[0, 1]` range constraint — invalid probabilities pass schema validation silently. See inline.
3	`test/unit/infra/dream/dream-state-service.test.ts`	nitpick	No test pins the `pendingMerges` mutation-isolation fix — a future refactor that reverts to bare `{...EMPTY_DREAM_STATE}` would silently regress. See inline.
4	`dream-log-store.ts:88`	suggestion	When any filter is active, `limit` is not applied until after all files are read. Since timestamps are embedded in filenames, a cheap pre-filter can push `after`/`before` down before I/O. See inline.
5	`dream-log-store.ts:59`	nitpick	`getNextId` is `async` but contains no `await`. Sync signature is more accurate and avoids misleading callers. See inline.

What's good

Atomic writes (tmp → rename) in both DreamStateService and DreamLogStore are correct and consistent.
Fail-open reads in DreamStateService handle missing files, corrupt JSON, and schema failures separately — the three distinct error paths are all exercised in tests.
Path traversal prevention in getById via ID_PATTERN.test(id) before constructing any file path is exactly right.
Stale-processing resolution in resolveStale is a smart UX touch; fire-and-forget save is the right call since the in-memory state is already correct.
Lock rollback (priorMtime = 0 → unlink, otherwise restore mtime) correctly handles the "never existed" case without leaking state.
Test structure is thorough — discriminated-union variants, pruning, monotonic ID ordering, and path traversal are all directly exercised.
DREAM_LOG_DIR / DREAM_LOG_ID_PREFIX follow the existing CURATE_LOG_* constant pattern exactly.

Minor observations (no action required)

dream-state-schema.test.ts and dream-log-schema.test.ts both have inner describe blocks that are not indented inside the outer describe('...', () => { wrapper. No behavioral impact but makes the nesting harder to read at a glance.
DreamLogEntry.startedAt is z.number() with no lower-bound guard. A z.number().int().positive() would be more defensive, though in practice the store only writes values sourced from Date.now().
release() has no mkdir guard. Technically correct (you can only release after a successful tryAcquire, which already creates the directory), but a misuse would throw ENOENT rather than silently no-op.

- isProcessAlive: treat EPERM as alive (process exists, lacking permission) - SynthesizeOperationSchema: constrain confidence to [0, 1]

github-actions · 2026-04-13T08:40:49Z

Claude finished @RyanNg1403's task in 2m 41s —— View job

PR Review: feat: [ENG-2065] dream state + log services

Tasks

Gather context and understand PR scope
Read source files to assess latest state
Review test files
Analyze what was fixed vs. what remains open
Post inline comments
Post overall review summary

Overall: 4.5/5

The two must-fix issues from the prior review round have both landed cleanly in c73de6c and 9e7a085. The persistence layer is in good shape to build on.

What was fixed since last review

#	Commit	Issue
✅	`9e7a085`	`isProcessAlive` now returns `true` on `EPERM` (live process owned by another user is no longer treated as dead)
✅	`9e7a085`	`confidence` constrained to `[0, 1]` via `.min(0).max(1)`
✅	`c73de6c`	`pendingMerges` shallow-copy fixed — both fail-open paths now return `{...EMPTY_DREAM_STATE, pendingMerges: []}`
✅	`c73de6c`	`tryAcquire` now calls `mkdir(dirname(lockFilePath), {recursive: true})` before writing

Still open (nitpicks / suggestions only)

#	File	Severity	Issue
1	`dream-log-store.ts:59`	nitpick	`getNextId` is `async` with no `await` — sync signature is more accurate. See inline.
2	`dream-log-store.ts:87-88`	suggestion	`limit` not applied until after all files are read when any filter is active — timestamp pre-filter at filename level is cheap. See inline.
3	`test/unit/infra/dream/dream-state-service.test.ts:66`	nitpick	No regression test pinning the `pendingMerges` mutation-isolation fix — one `push`+re-read test would lock this in permanently. See inline.
4	`test/unit/infra/dream/dream-log-schema.test.ts:101`	nitpick	`confidence` boundary tests missing — schema constraint is enforced but not exercised at `-0.1`/`1.1`/`0`/`1`. See inline.

None of the above block merge. All are carryovers from the prior review; none are regressions introduced by the fixup commits.

What's solid

Atomic writes (tmp → rename) in both DreamStateService and DreamLogStore are correct and consistent.
Fail-open reads cover all three failure paths (missing file, corrupt JSON, schema failure) and each is exercised in tests.
Path-traversal prevention in getById via ID_PATTERN.test(id) before constructing any file path is exactly right.
EPERM fix is correctly scoped — only EPERM is treated as alive; all other errors (including ESRCH for no-such-process) still return false.
mkdir guard placement (before the write, after the stale-check branch) is correct.
Lock rollback (priorMtime === 0 → unlink, otherwise restore mtime) correctly handles the "never existed" case without leaking state.
Test structure is thorough — discriminated-union variants, pruning, monotonic ID ordering, and path traversal are all directly exercised.

danhdoan

lgtm

RyanNg1403 requested review from bao-byterover and leehpham as code owners April 13, 2026 08:28