Skip to content

Fix subagent announce + add promptMode for token efficiency#944

Closed
tyler6204 wants to merge 13 commits intoopenclaw:mainfrom
tyler6204:fix/subagent-announce
Closed

Fix subagent announce + add promptMode for token efficiency#944
tyler6204 wants to merge 13 commits intoopenclaw:mainfrom
tyler6204:fix/subagent-announce

Conversation

@tyler6204
Copy link
Copy Markdown
Member

@tyler6204 tyler6204 commented Jan 15, 2026

Summary

Fix subagent announce flow (was completely broken) and address all review feedback.

Original Changes

  • Fix subagent announce routing and format improvements
  • Simplify: main agent summarizes subagent results in its own voice (no extra LLM step)
  • Add chat.inject gateway method to inject messages into session transcripts
  • Add promptMode to reduce subagent token usage (~25% reduction)
  • Expand subagent tool deny list
  • Fix session store race condition that caused ~67% of concurrent subagent results to be lost

Review Feedback Fixes

  • Fix delete semantics: Replace delete store[key] with store[key] = undefined for merge-safe writes in all 6 flagged locations (legacy alias migrations now work correctly with concurrent writes)
  • Add chat.inject bridge handler: Bridge clients can now call chat.inject
  • Add tests for merge-delete semantics: 6 new tests covering concurrent writes, deletion via undefined/null, legacy key migration
  • Add tests for chat.inject: 4 new tests for append/broadcast path, error cases
  • Fix subagent parallelism: Subagents now run in parallel (was serialized due to lane concurrency defaulting to 1, now unlimited by default)
  • Add tests for lane concurrency: 2 new tests for Infinity and limited concurrency
  • Update docs/types/schema: Reflect new unlimited default for agents.defaults.subagents.maxConcurrent

Session Store Race Condition Fix

When multiple subagents spawn simultaneously, concurrent calls to saveSessionStore would overwrite each other's changes (classic read-modify-write race). Fixed by re-reading the store inside the lock and merging before writing:

await withSessionStoreLock(storePath, async () => {
  const current = loadSessionStore(storePath);  // Re-read inside lock
  const merged = { ...current, ...store };      // Merge (incoming wins)
  // Handle explicit deletions
  for (const key of Object.keys(store)) {
    if (store[key] === undefined || store[key] === null) {
      delete merged[key];
    }
  }
  await saveSessionStoreUnlocked(storePath, merged);
});

Subagent Parallelism Fix

Changed default maxConcurrent from 1 to unlimited (Infinity) in:

  • server-lanes.ts (initial apply)
  • server-reload-handlers.ts (hot reload)

Users can still cap via agents.defaults.subagents.maxConcurrent config if needed.

Test Plan

  • All 3261 tests pass
  • Build passes
  • Lint clean
  • Manual test: spawn subagent, verify main agent summarizes
  • Manual test: spawn multiple subagents, verify all run in parallel

🤖 Generated with Claude Code

@tyler6204 tyler6204 marked this pull request as draft January 15, 2026 11:59
@tyler6204 tyler6204 force-pushed the fix/subagent-announce branch from 0aed539 to 0dd3d20 Compare January 15, 2026 20:13
@tyler6204 tyler6204 marked this pull request as ready for review January 15, 2026 21:02
@steipete steipete self-assigned this Jan 15, 2026
@steipete
Copy link
Copy Markdown
Contributor

  Findings

  - High: saveSessionStore now merges inside lock; any top‑level
    delete store[...] no longer removes keys, so alias migrations +
    deletes will resurrect entries. Switch to store[key] = undefined
    (or adjust merge logic) everywhere you delete keys. Refs: src/
    gateway/server-methods/sessions.ts:115, src/gateway/server-
    methods/sessions.ts:164, src/gateway/server-methods/
    sessions.ts:228, src/gateway/server-methods/sessions.ts:303, src/
    gateway/server-bridge-methods-sessions.ts:134, src/gateway/server-
    bridge-methods-sessions.ts:269.
  - Medium: chat.inject added to protocol/server methods but no bridge
    handler → bridge clients can’t call it. Add handler or explicit
    rejection. Ref: src/gateway/server-bridge-methods-chat.ts:32.
  - Low: Missing tests for merge‑delete semantics + chat.inject
    append/broadcast path.

  Questions / assumptions

  - Is chat.inject meant to be bridge‑accessible? If yes, add bridge
    path.

Always do a codex review. You'll be surprised what it finds.

@tyler6204
Copy link
Copy Markdown
Member Author

Thanks appreciate the feedback, I'll update and check with codex. I also found another issue where subagents appear to be serialized instead of running in parallel. tested by spawning a 30 second wait subagent, then 5 seconds later spawning one that should reply instantly. the instant one didnt start until the slow one finished, confirmed by timestamps. the second subagent showed 0 tokens and empty session while the first was running, then executed right after. will include this fix as well

tyler6204 and others added 9 commits January 15, 2026 14:20
- Fix session entry merging bug where entries with same canonical key
  would overwrite each other, losing lastChannel/lastTo routing info
- Track lastChannel, lastTo, lastAccountId in session entries for
  announce target resolution
- Use compact format for external channels (telegram, whatsapp) with
  clean result text only; verbose Stats format for internal/dashboard
- Inject announce into main session context via enqueueSystemEvent so
  main agent sees subagent completion
- Skip Skills, Memory, Heartbeats, Silent Replies, Messaging sections
  in system prompt for subagents to reduce token usage
- Update tests for compact format on external channels

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace the announce LLM step with a simpler report_back tool that
subagents can use to send results directly to the main session.

Key changes:
- Add report_back tool: subagents call this to send findings to main agent
- Add chat.inject gateway method: injects messages into session + broadcasts to webchat
- Remove announce LLM step: subagents now report explicitly, main agent summarizes
- Update subagent system prompt to explain report_back usage
- Registry now only does cleanup (label patch + optional delete), not announce

Benefits:
- Simpler flow: subagent uses report_back when done, no extra LLM call
- Main agent summarizes in its own voice (using SOUL.md)
- Silent subtasks work naturally (just don't call report_back)
- Results show in gateway dashboard via chat.inject broadcast
- System events still inject into main session context

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…cript

- Only register report_back tool for subagent sessions (not main agent)
- Fix chat.inject to write to transcript file (was only broadcasting)
- Use role: "assistant" instead of "system" so webchat displays the message
- Add stopReason and usage fields for proper Pi AI format

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Flow:
1. Subagent calls report_back("findings")
2. chat.inject writes to main session transcript (shows in webchat)
3. Triggers main agent via agent gateway call
4. Main agent sees subagent report in transcript, responds to user
5. Response delivered to external channel (Telegram, etc.)

Main agent summarizes in its own voice - no extra user message needed.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove extra announce LLM step (runAgentStep)
- Send instructional message to main agent instead of direct send
- Main agent summarizes in its own voice (uses SOUL.md, identity)
- Include stats (runtime, tokens, cost) in trigger message
- Remove report_back tool entirely
- Update subagent system prompt to remove report_back references
- Add promptMode support for subagents (minimal system prompt)
- Main agent can respond with NO_REPLY if silent

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When multiple subagents spawn simultaneously, concurrent calls to
saveSessionStore would overwrite each other's changes. Fixed by
re-reading the store inside the lock and merging before writing.

Before: ~33% success rate (1-2 of 3-5 subagents persisted)
After: 100% success rate (all subagents persist correctly)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Parallel subagent spawns were losing session entries due to read-modify-write
race conditions. The fix modifies saveSessionStore to re-read and merge inside
the lock, and updates deletion patterns to use undefined instead of delete
operator so merges can detect and honor deletions.

Files changed:
- store.ts: Re-read and merge inside lock in saveSessionStore
- agent-runner-execution.ts: Use undefined for deletion signaling
- session.ts: Use undefined for legacy key migration deletion
- sessions.ts: Use undefined for RPC session deletion
- Fix delete semantics: use store[key] = undefined for merge-safe deletes
  in session store (legacy alias migrations now work with concurrent writes)
- Add chat.inject bridge handler for bridge client accessibility
- Add tests for merge-delete semantics in session store
- Add tests for chat.inject append/broadcast path
- Fix subagent parallelism: increase default lane concurrency from 1 to 5

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@tyler6204 tyler6204 force-pushed the fix/subagent-announce branch from 9b0901b to 7d7e30e Compare January 15, 2026 22:32
tyler6204 and others added 4 commits January 15, 2026 14:40
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Users can still cap via agents.defaults.subagents.maxConcurrent if needed.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@tyler6204
Copy link
Copy Markdown
Member Author

Addressed all review feedback

Fixed:

  • ✅ Delete semantics: store[key] = undefined for merge-safe deletes (6 locations)
  • ✅ Added chat.inject bridge handler
  • ✅ Added tests for merge-delete semantics (6 tests)
  • ✅ Added tests for chat.inject (4 tests)
  • ✅ Fixed subagent parallelism: now unlimited by default (was serialized due to maxConcurrent: 1)
  • ✅ Added tests for lane concurrency (2 tests)

Additional changes:

  • Updated docs/types/schema to reflect unlimited default for agents.defaults.subagents.maxConcurrent

All 3261 tests pass, build clean, lint clean. Ready for re-review!

@steipete
Copy link
Copy Markdown
Contributor

Landed on main with commits: 688a0ce, a4b347b, 10eb1be. Tests: pnpm lint && pnpm build && pnpm test.

1 similar comment
@steipete
Copy link
Copy Markdown
Contributor

Landed on main with commits: 688a0ce, a4b347b, 10eb1be. Tests: pnpm lint && pnpm build && pnpm test.

@steipete steipete closed this Jan 15, 2026
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