feat: [ENG-2859] background sub-agent dispatch for brv curate#753
Merged
Conversation
Adds a saved `brv-curate` sub-agent that runs `brv curate` sessions
detached from the calling conversation, so substantive curates (10-60s
each) stop blocking the user's prompt. Two surface-specific definitions
ship under src/server/templates/agent/:
brv-curate.md — Claude Code: permissionMode: bypassPermissions,
tools: Bash/Read/Write/Edit/Grep/Glob,
background: true, model: inherit
brv-curate.toml — Codex: sandbox_mode = "workspace-write",
developer_instructions carrying the full body
Both carry the same operational protocol — per-fact session loop, /tmp
envelope file via --response-file, path-exists merge, retry cap, return
shape. The /tmp pin is load-bearing: background sub-agents have
permission prompts auto-denied (Claude) or are sandbox-gated (Codex),
and only the writable sandbox area (/tmp) is pre-authorized.
Skill docs updated to make dispatch the DEFAULT for substantive curates
(not opt-in):
curate.md `## Background Execution` — dispatch shape per surface +
saved-agent file walkthroughs + deployment table
SKILL.md `## Curate Dispatch` — decision table (1 / 2-5 / 6+ facts)
right after the Iron Law, so the calling agent routes
correctly without opening curate.md first
SkillConnector deploys the sub-agent file automatically on
`brv connectors install`:
Claude Code → .claude/agents/brv-curate.md
Codex → .codex/agents/brv-curate.toml
Other surfaces (Cursor, Windsurf, etc.) leave the agents dir untouched.
Mechanism: optional `agentFile: {source, target}` on SkillConnectorConfig,
set only on Claude Code and Codex. status() reports incomplete when the
agent file is missing; uninstall() removes the agent file before the
skill directory. 5 new unit tests cover deploy / no-op / partial-install /
uninstall cleanup (44/44 green).
cuongdo-byterover
requested review from
bao-byterover and
leehpham
as code owners
Contributor
|
Claude finished @cuongdo-byterover's task in 4m 1s —— View job PR Review: feat: [ENG-2859] background sub-agent dispatch for
|
| # | Severity | File | Concern |
|---|---|---|---|
| 1 | medium | templates/agent/brv-curate.md:68,144 |
"Auto-denied silently outside /tmp/" contradicts permissionMode: bypassPermissions (per curate.md:110 the mode "skips the prompts entirely"). Rationale for pinning /tmp/ should be sandbox portability + hygiene, not auto-deny. |
| 2 | medium | templates/agent/brv-curate.toml:11-17 |
Codex workspace-write permits writes to cwd + $TMPDIR. On macOS $TMPDIR is /var/folders/..., not /tmp — pinning the envelope to /tmp/ may sandbox-fail. This is one of the open "live smoke" DoD items. |
| 3 | low | test/.../skill-connector.test.ts:46 |
The existing test enumerates every templates/skill/*.md. Add a sibling that asserts every agentFile.source referenced in SKILL_CONNECTOR_CONFIGS actually exists in templates/agent/, so a future template rename can't silently break install. |
| 4 | nit | skill-connector.ts:127-142 |
Partial-install failure message is generic. A two-line note on "skill files written, agent file failed" would speed user diagnosis. The repair path already works. |
| 5 | nit | skill-connector-config.ts:15-18 |
target carries a path-with-filename while source carries a filename. Splitting into targetDir + reusing source removes the "keep the basenames in sync" invariant. Pure ergonomics. |
| 6 | nit | templates/skill/curate.md:144-154 |
Permission-rule recommendations don't clarify they're for the non-saved-agent fallback path; with bypassPermissions they're dead config. A one-line callout would prevent users from thinking the allow-list alone unlocks background dispatch. |
Security & correctness check
- No new attack surface in the connector.
agentFile.targetis read from a const config, not from user input — no path-traversal risk. - Overwrite-by-default for the agent file matches the existing skill-file pattern. Users with a hand-tuned
brv-curate.md/.tomlwill lose their changes on re-install. That's consistent with how skill files behave; documented behavior of the connector is "managed files." - HITL stays human-driven. Both agent bodies explicitly forbid
brv review approve|rejectin the hard-constraints lists. ✅ - No nested sub-agent spawning. Forbidden in the hard-constraints. ✅
--detachforbidden in the worker. ✅ — correct, since the worker is already detached and double-detach orphans.
Verification status
- ✅ Connector unit suite changes are well-scoped (5 new tests, partial-install + lifecycle covered).
⚠️ Did not runnpm test/npm run buildhere. PR body flags both as "please confirm in CI."⚠️ Live end-to-end on a fresh Claude Code (and macOS Codex) session is the one open DoD item per ENG-2859. Concerns Feat/init #1 and Feat/refactor structure #2 above are most likely to surface there.
Todos
- Read changed files
- Review
skill-connector-config.ts - Review
skill-connector.ts - Review
skill-content-loader.ts - Review agent templates
- Review skill prose
- Review test additions
- Post inline comments + summary
· Branch:feat/ENG-2859
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Collaborator
Author
Review triage — applied in
|
| # | Status | Note |
|---|---|---|
| 1 | ✅ Fixed | Reworded the auto-deny rationale in both worker templates (and the TOML top-of-file comment). See thread. |
| 2 | ✅ Fixed | ${TMPDIR:-/tmp} resolution in both worker prompts; subsequent steps reference $ENVELOPE. Live macOS-Codex smoke is still on the DoD list, but expected to pass now. See thread. |
| 3 | ✅ Fixed | Added a sibling test that enumerates every agentFile.source in SKILL_CONNECTOR_CONFIGS and asserts the template file exists under src/server/templates/agent/. 46/46 green. |
| 4 | ⏭️ Skipped | The partial-install repair path (re-install overwrites) already works correctly. Distinguishing 'skill files OK, agent file failed' in the user-facing message would complicate the install flow for a nit-level diagnosis improvement. Holding off until we see it cause real user friction. |
| 5 | ⏭️ Skipped | Leaving {source, target} — the full-path target preserves the ability to rename the file at the destination if a future surface needs a non-matching name. {source, targetDir} would drop that. See thread. |
| 6 | ✅ Fixed | Added an explicit scope callout in curate.md § Permission prerequisites — the allow-list rules are for the foreground / non-saved-agent path; bypassPermissions / workspace-write make them inert for dispatched curates. See thread. |
Diff: 4 files, +67/-33. Type-check clean, 46/46 connector tests passing locally.
Open DoD item from the original ticket (live end-to-end Claude Code session) is still pending; macOS-Codex smoke joins it now.
…rationale PR #753 review surfaced two prose drifts and a missing coverage test on the saved sub-agent templates. This commit addresses comments #1, #2, #3, #6; comments #4 and #5 are nits and left as-is (rationale in PR replies). #1 — `brv-curate.md` (Claude) claimed writes outside `/tmp/` are "auto-denied silently". That framing is from a prior iteration before we landed `permissionMode: bypassPermissions`. The real reason to pin the envelope to the temp directory is portability + hygiene, not auto-deny. Reworded the step-3 body and the matching hard-constraint bullets. #2 — Claude worker now resolves `$TMP="${TMPDIR:-/tmp}"` with Bash once, then uses `$ENVELOPE` as the literal path passed to Write and `--response-file`. Works on Linux (where $TMPDIR is unset, falls to /tmp), Claude Code (same), and macOS (where $TMPDIR is /var/folders/...). #3 — Added a sibling test in `managed skill files` that enumerates every `agentFile.source` referenced by `SKILL_CONNECTOR_CONFIGS` and asserts the file exists under `src/server/templates/agent/`. Prevents a future template rename from silently breaking install. 46/46 green. #6 — `curate.md` Permission prerequisites section now explicitly states the allow-list rules apply to the foreground / non-saved-agent path. Under `bypassPermissions` / `workspace-write` they are inert for dispatched curates; the callout prevents users from thinking the allow-list alone unlocks background dispatch. Note: the Codex `brv-curate.toml` is intentionally left at its prior state — the TMPDIR resolution and nickname_candidates that originally shipped on this branch were reverted per direct user direction. The trade-offs (no Codex panel label, /tmp pinning may bite on macOS workspace-write) are tracked as follow-ups.
Contributor
|
Claude finished @cuongdo-byterover's task in 2m 32s —— View job PR Re-Review (fixup commit
|
| # | Severity | File / lines | Concern |
|---|---|---|---|
| A | medium | brv-curate.toml:11–17, 89–93, 95–97, 110–115, 141–146, 184–185 |
Round-1 #1 + #2 not applied here. Hardcoded /tmp/… envelope path + stale "silently fail" / "auto-deny analog" framing. On macOS Codex, workspace-write whitelists cwd + $TMPDIR and $TMPDIR is /var/folders/.../T, not /tmp — the most likely failure mode for the open macOS-Codex DoD smoke. Same TMP="${TMPDIR:-/tmp}" + $ENVELOPE pattern works inside a TOML triple-quoted block with no extra escaping. (See inline.) |
| B | low / docs | PR thread vs commit message | Round-1 reply to comment #2 says "Same fix in brv-curate.toml"; the commit message says it was intentionally left out. Worth amending the reply or the commit story so the next reviewer doesn't have to reconcile them. |
Security & correctness
- Connector surface unchanged in this commit —
agentFile.targetstill const-config, no new attack surface. - HITL + no-nested-subagents + no-
--detachconstraints intact in.md. (Identical constraints in.tomluse the same stale framing as round-1 but still functionally correct.) - Test additions are self-contained, no production-code touch in this commit besides the template prose.
Verification
- ✅ Static review of the four changed files.
⚠️ Did not runnpm test/npm run typecheck/npm run buildhere (no install permission). PR author reports 46/46 connector tests green and typecheck clean locally.⚠️ Live macOS-Codex smoke is unchanged from round-1 — the.tomlrevert in this commit makes that smoke more likely to surface the/tmp↔$TMPDIRgap, not less. Either land the same TMPDIR fix on the.tomlor run the smoke before merge so the failure mode is observed rather than guessed at.
Todos
- Pull diff vs
origin/mainand the fixup commit alone - Verify TMPDIR resolution fix in both worker templates
- Verify reworded sandbox rationale in
brv-curate.md/brv-curate.toml - Verify scope callout in
curate.md§ Permission prerequisites - Verify new test enumerating
agentFile.sourcefiles - Run typecheck + connector test suite — skipped (install requires approval); relying on author's locally-clean report
- Post inline issues + final summary
· Branch:feat/ENG-2859
danhdoan
approved these changes
Jun 2, 2026
bao-byterover
approved these changes
Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
brv curateis a multi-step session (kickoff → author HTML → continuation → validate, up to 4 attempts) that BLOCKS the user's conversation for ~10-60s per topic. The earlier "instruction-only + permission allowlist" attempt failed in field tests — the main agent kept running curates inline instead of dispatching.brv-curatesub-agent for Claude Code (.claude/agents/brv-curate.md) and Codex (.codex/agents/brv-curate.toml), each carrying the full session protocol. Skill docs make background dispatch the default.brv connectors installdeploys the agent file alongside the skill markdown files.--format jsonis a separate plan). Nobrvdaemon-side or CLI core changes — this PR is templates + connector wiring only.--detachandPostWorkRegistryare untouched.Type of change
Scope (select all touched areas)
(Also: skill templates under
src/server/templates/{skill,agent}/— no checkbox above matches exactly; closest is "Hub / Connectors" since they ship throughSkillConnector.)Linked issues
Root cause (bug fixes only, otherwise write
N/A)N/A — this is a feature PR, not a bug fix.
Test plan
test/unit/infra/connectors/skill/skill-connector.test.ts(5 new tests, 44 total — all passing)install()deploys.claude/agents/brv-curate.mdfor Claude Codeinstall()deploys.codex/agents/brv-curate.tomlfor Codexinstall()does NOT create anagents/directory for surfaces without anagentFileconfig (Cursor as the reference case)status()reports not-installed when the skill files are present but the agent file has been removed (partial-install detection)uninstall()removes the agent file alongside the skill directoryUser-visible changes
brv connectors installfor Claude Code now also writes.claude/agents/brv-curate.md.brv connectors installfor Codex now also writes.codex/agents/brv-curate.toml.brv connectors uninstallremoves the agent file in addition to the skill directory.brv-curatethat runs the fullbrv curatesession detached from the calling conversation.SKILL.md/curate.md) now tells the calling agent to default to background dispatch for substantive curates.brvCLI; no config-file changes required.Evidence
Checklist
mocha skill-connector.test.ts— fullnpm testnot run by me; please confirm in CI)lint-staged+npm run lint:fixran clean during commitnpx tsc --noEmitclean)npm run build) — not run by me; please confirm in CIfeat: [ENG-2859] ...)SKILL.md,curate.md)agentFilefield onSkillConnectorConfig, two new template files, two new skill sections)main(merged main intofeat/ENG-2859—d22c1f54b)Risks and mitigations
Risk: Bypassing Claude Code's permission prompts via
permissionMode: bypassPermissionsin the saved agent could over-permit if thetoolsallowlist drifts.brv review approve|reject(HITL stays human-driven), nested sub-agents,--detach, and writes outside/tmp/. Any future widening oftoolsrequires a deliberate change to the template.Risk: Codex's
sandbox_mode = "workspace-write"is broader than a tools allowlist — the worker can write anywhere in the workspace./tmp/brv-curate-envelope-<sessionId>.jsonand forbids writes elsewhere via the hard-constraints list. Codex doesn't expose a finer-grained capability gate today.Risk: Installer silently creates
.claude/agents/or.codex/agents/directories in user repos.brv connectors install <Claude Code|Codex>.uninstallcleans the file (not the directory — left intact for the user's other agents).Risk: The live end-to-end (fresh Claude Code session, no main-agent inline rescue) has NOT been re-verified after the saved-agent restructure.
/Users/cuong/workspaces/deer-flow/.claude/agents/brv-curate.md. Manual smoke is the one open DoD item on ENG-2859 — do not merge before that passes.Risk: Skill prose tells the LLM to dispatch by default; LLMs may still occasionally run curates inline.
## Background Executionreinforces with the explicit five-exception list. If field tests show continued non-compliance, the next step is making it a hard rule rather than a default.