Skip to content

feat: [ENG-2153] Curate command blocks execution and prevents concurrent actions#479

Merged
bao-byterover merged 2 commits intomainfrom
feat/ENG-2153
Apr 21, 2026
Merged

feat: [ENG-2153] Curate command blocks execution and prevents concurrent actions#479
bao-byterover merged 2 commits intomainfrom
feat/ENG-2153

Conversation

@bao-byterover
Copy link
Copy Markdown
Collaborator

Summary

  • Problem: Agents were defaulting to brv curate --detach and then immediately reading the data back, violating the "you cannot query data that does not yet exist" invariant. This led to stale or missing context in downstream query / search / follow-up curate calls. --detach errors are also silent, so failures went undetected.
  • Why it matters: Curate → query is the core agent loop. Premature detach breaks recall correctness — the exact regression we reverted earlier in the curate execution history (see ticket background). Keeping the status quo means the flag exists but agents can't use it safely.
  • What changed: Agent-facing template docs now mandate blocking by default and gate --detach behind two conjunctive conditions — (a) nothing later in the turn reads/builds on the data, AND (b) the user explicitly said not to wait. Explicitly rejects common self-justifications (size, "looks like last step", "can't think of a dependency"). Requires reporting "queued" (not "saved") after detach and verifying via brv curate view <logId> before any downstream read. Updated files: brv-instructions.md, command-reference.md, workflow.md, SKILL.md.
  • What did NOT change (scope boundary): No runtime/CLI/daemon behavior changes. The brv curate command, its --detach flag, the curate executor, and any stores are untouched. Only agent-facing guidance changes. No TypeScript, no tests, no transport schemas.

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
  • LLM Providers
  • Server / Daemon
  • Shared (constants, types, transport events)
  • CLI Commands (oclif)
  • Hub / Connectors
  • Cloud Sync
  • CI/CD / Infra

(Server / Daemon is checked because the templates live under src/server/templates/ and are injected by the daemon; no daemon code path changes.)

Linked issues

  • Closes #ENG-2153
  • Related #

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

  • Root cause: N/A
  • Why this was not caught earlier: N/A

Test plan

  • Coverage added:
    • Unit test
    • Integration test
    • Manual verification only
  • Test file(s): None — markdown template changes only; no executable code paths changed.
  • Key scenario(s) covered:
    • Start brv REPL on a fresh project, ask agent to save learnings → agent uses brv curate "..." (no --detach) and waits.
    • Instruct agent "don't wait, fire and forget" on a curate where nothing downstream reads it → agent uses --detach, reports "queued" with logId.
    • Instruct agent to detach, then immediately ask it to query the just-curated data → agent runs brv curate view <logId> first and waits for status: completed.
    • Ambiguous phrasing ("run it in background", "run async") → agent still blocks (these are excluded triggers).

User-visible changes

  • No CLI surface, flag, or output format changes. Agent behavior shifts: curate is blocking by default, detach usage requires explicit user instruction, and the agent now reports "queued" (not "saved") with a logId whenever detach is used.

If none, write None.

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets (agent transcript showing blocking-by-default behavior and brv curate view <logId> verification step before follow-up query — attach before merge)
  • Screenshot/recording

Checklist

  • Tests added or updated and passing (npm test) (N/A — no code changes; existing suite unaffected, confirm green)
  • 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) (this PR is the documentation update)
  • No breaking changes (or clearly documented above)
  • Branch is up to date with main (merge commit 6e09622)

Risks and mitigations

  • Risk: Agents over-block on safe detach-eligible work, increasing latency in interactive sessions.
    • Mitigation: Guidance still permits --detach with explicit user signal; users can always invoke brv curate --detach directly on the CLI where they control the execution mode.
  • Risk: Agents misread the dual-gate and detach anyway, hiding silent curate failures.
    • Mitigation: Guidance mandates brv curate view <logId> verification before trusting detached data; if drift is observed in transcripts, tighten SKILL.md with more explicit examples.
  • Risk: Guidance conflicts with or drifts from future curate UX changes (e.g., if default changes at the CLI level).
    • Mitigation: Docs live in src/server/templates/ alongside other agent guidance; revisit in lockstep with any curate CLI behavior change.

@bao-byterover bao-byterover requested a review from leehpham as a code owner April 21, 2026 02:54
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 21, 2026

Claude finished @bao-byterover's task in 2m 5s —— View job


PR Review: feat: [ENG-2153] Curate command blocks execution and prevents concurrent actions

  • Gather context and understand the PR
  • Read changed files
  • Analyze documentation changes for correctness and clarity
  • Provide review feedback

Summary

Score: 4/5

This is a well-scoped, high-quality documentation-only PR that addresses a real behavioral regression in the agent loop. The dual-gate mechanism for --detach is logically sound and the language is deliberately strong ("wait", "block", "BOTH") to resist agent rationalization. The "mirror-priming loop" rationale in SKILL.md is especially insightful — it explains why "run in background" is excluded rather than just listing it.

The main concerns are minor wording inconsistencies between the three files carrying the same dual-gate rules, a missing JSON output example for the verification step, and the evidence transcript not yet attached per the PR's own checklist.


Issues

suggestion (brv-instructions.md, line 101): The verification step here lacks --format json, making it harder for agents to programmatically parse status. SKILL.md includes this flag but brv-instructions.md doesn't. Since agents load different template files, the guidance should be consistent.

- Before querying data from a prior `--detach`, run `brv curate view <logId>` and wait for `status: completed`
+ Before querying data from a prior `--detach`, run `brv curate view <logId> --format json` and wait for `status: completed`

Fix this →


suggestion (SKILL.md, line 104–110): The cross-turn hygiene block instructs agents to parse status: completed from JSON output, but doesn't show what that JSON looks like. A one-line sample makes it unambiguous and avoids agents having to guess the field path.

 Only proceed when `status: completed`. If `processing`, wait or tell the user. If `error`/`cancelled`, report and consider re-curate.
+
+Sample output: `{"logId": "cur-xxx", "status": "completed", "summary": "..."}`

Fix this →


nit (across files — gate 1 wording): Gate 1 in brv-instructions.md and workflow.md uses reads/queries/references but SKILL.md adds search explicitly: "query, search, read, or reference". The SKILL.md version is more complete — the other two files miss brv search as a downstream dependency. Worth making them consistent.

Files affected:

  • src/server/templates/sections/brv-instructions.md line 87
  • src/server/templates/sections/workflow.md line 33

Fix this →


praise: The explicit enumeration of "NOT reasons to --detach" in brv-instructions.md is excellent defensive writing. Pre-empting "I can't think of a dependency" and "looks like the last step" directly counters the most common failure modes in agent reasoning loops.


praise: Verifying the implementation — brv curate view --format json is a real, supported flag (src/oclif/commands/curate/view.ts:41-44), so the SKILL.md verification step is grounded in actual CLI behavior. Good.


nitpick (PR checklist): The Evidence section is marked [x] but with a note "attach before merge" — no transcript has been attached. This is the only checklist item flagged as incomplete. Not a blocker for the logic of the change, but worth following through before merging.


out-of-scope observation: src/server/templates/sections/mcp-workflow.md is a separate minimal workflow template for MCP-integrated agents that has no execution mode guidance at all. Since MCP uses the brv-curate tool (not CLI flags), --detach is N/A there — so the omission is correct.

@bao-byterover
Copy link
Copy Markdown
Collaborator Author

@DatPham-6996 please help me review this PR, thanks so much.

Copy link
Copy Markdown
Collaborator

@DatPham-6996 DatPham-6996 left a comment

Choose a reason for hiding this comment

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

LGTM

@bao-byterover bao-byterover merged commit 1c61ea5 into main Apr 21, 2026
6 checks passed
@bao-byterover bao-byterover deleted the feat/ENG-2153 branch April 21, 2026 15:29
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.

3 participants