Skip to content

merge: sync main (3.10.1) into curation-enhancement#599

Merged
danhdoan merged 1 commit intoproj/curation-enhancementfrom
chore/curation-enhancement-sync-main-3-10-1
May 4, 2026
Merged

merge: sync main (3.10.1) into curation-enhancement#599
danhdoan merged 1 commit intoproj/curation-enhancementfrom
chore/curation-enhancement-sync-main-3-10-1

Conversation

@danhdoan
Copy link
Copy Markdown
Collaborator

@danhdoan danhdoan commented May 4, 2026

Summary

  • Problem: to sync version 3.10.1 and resolve conflicts

User-visible changes

N/A

Evidence

N/A

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

None

Resolves three conflicts created by the overlap of ENG-2485 (defer summary
cascade to dream, on this branch) and ENG-2522 (detach post-curate Phase 4,
shipped on main, PR #582). The two changes touch the same lines but address
different costs — LLM cost vs user-visible latency — and compose cleanly.

curate-executor.ts
- Adopt main's {finalize, response} architecture so the daemon fires
  task:completed before Phase 4 runs.
- Inside finalize, keep cascade-defer: snapshot diff →
  enqueueStaleSummaryPaths + buildManifest. Drop main's
  propagateSummariesUnderLock — the dream lock existed to coordinate
  inline _index.md writes, which cascade-defer eliminates from curate.
- Preserve all exp 03 (cache_control) and exp 04 (recon pre-pipelining)
  changes in Phases 1-3.
- Extract propagateAndRebuild private helper alongside main's
  incrementDreamCounter.

dream-executor.ts
- Keep this branch's runStaleSummaryPropagation extracted seam (main did
  not change dream-executor; nothing lost).

curate-executor.test.ts
- Rewrite runAgentBody/finalize split assertions to expect cascade-defer
  (enqueueStaleSummaryPaths + buildManifest) instead of propagateStaleness.
- Drop the dream-lock coordination describe block — the lock is no longer
  taken in curate's Phase 4 (folder-pack still uses it).

folder-pack-executor still calls propagateSummariesUnderLock; the helper
file stays.

Verified: typecheck, lint (0 errors on resolved files), 7284 tests pass.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Claude finished @danhdoan's task in 4m 22s —— View job


PR Review — sync main (3.10.1) into curation-enhancement

  • Gather PR context and changed files list
  • Review core infrastructure changes (daemon, executors, task router)
  • Review new files (post-work-registry, propagate-summaries, status-row-classifier)
  • Review transport/handler changes
  • Review test coverage
  • Post inline comments for specific issues
  • Post overall summary with score

Overall Score: 4 / 5

This is a well-executed sync that brings substantial, production-quality features into the branch. The implementation quality is high across the board.


What's new (summary of additions)

Area Change
PostWorkRegistry Per-project serialized fire-and-forget queue with bounded shutdown drain. Cleanly solves the task:completed-before-Phase-4 ordering problem.
reviewDisabled snapshot Daemon stamps the flag once at task-create, propagates via AsyncLocalStorage and TaskExecute, so daemon (CurateLogHandler) and agent (curate-tool, dream executor) observe a single consistent value even if the user toggles mid-run.
brv review command Clean oclif command to toggle/query the HITL review log (--enable/--disable). Wired through ReviewHandler which got GET_DISABLED and SET_DISABLED transport events.
status-row-classifier Extracts isomorphic-git statusMatrix tuple classification into a single shared module, eliminating six call sites that were deriving their own projections. Well-documented with the encoding table.
git-error-messages Unified working-tree-overwrite error messages across pull/merge/checkout.
propagateSummariesUnderLock Shared Phase 4 block for FolderPackExecutor — acquires the dream lock so curate's folder-pack path doesn't race with a concurrent dream on _index.md.
Deferred stale-summary propagation CurateExecutor now enqueues changed paths into the dream-state queue instead of calling propagateStaleness inline. DreamExecutor drains the queue and merges with its own snapshot diff (A∪B) before a single propagateStaleness walk.

Strengths

  • Fail-open everywhere: snapshot failures, Phase 4 errors, lock-acquire failures, session persist failures — all caught and swallowed with appropriate logging. No critical path is gated on non-critical work.
  • Shutdown drains work: PostWorkRegistry.drain(30_000) in agent-process shutdown prevents SIGTERM truncating _index.md mid-write.
  • Clear invariant comments: The "A∪B merge before propagateStaleness" invariant in DreamExecutor, the ALS propagation comment in curate-tool-task-context.ts, and the reviewEntriesWritten double-write guard are all well-commented.
  • Test coverage: New files all have corresponding unit tests (post-work-registry.test.ts, status-row-classifier.test.ts, review-handler.test.ts, review-toggle.test.ts). Integration tests for isomorphic-git-service and status-tuple-reachability are thorough.
  • Outside-In design followed: Consumer (agent-process.ts) drives the interface of PostWorkRegistry and executors; no premature abstractions.

Issues raised (inline comments)

  1. BrvConfig.withReviewDisabled — Explicit field enumeration instead of {...this, reviewDisabled} spread; will silently drop any future new fields. (brv-config.ts:261)

  2. agent-process.ts folderPath! — Non-null assertion violates CLAUDE.md convention; should narrow with a guard and explicit TaskError. (agent-process.ts:576)

  3. propagateSummariesUnderLockDreamLockService instantiated internally, making the function hard to test in isolation; prefer optional injection with an internal default. (propagate-summaries.ts:40)

  4. handleSetDisabled — Returns the request payload instead of the persisted value; minor divergence risk if the store normalises the input. (review-handler.ts:278)

None of these are blockers. Issues 1 and 2 are straightforward fixes; 3 and 4 are quality-of-life suggestions.

Comment thread src/server/core/domain/entities/brv-config.ts
Comment thread src/server/infra/daemon/agent-process.ts
Comment thread src/server/infra/context-tree/propagate-summaries.ts
Comment thread src/server/infra/transport/handlers/review-handler.ts
Copy link
Copy Markdown
Collaborator

@ncnthien ncnthien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look very good, so nice man

@danhdoan danhdoan merged commit d8c15bc into proj/curation-enhancement May 4, 2026
6 checks passed
@danhdoan danhdoan deleted the chore/curation-enhancement-sync-main-3-10-1 branch May 4, 2026 03:33
@danhdoan danhdoan mentioned this pull request May 4, 2026
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants