Skip to content

fix: skip before_prompt_build hooks for subagent sessions (#601)#613

Merged
rwmjhb merged 8 commits intoCortexReach:masterfrom
jlin53882:fix/issue601-subagent-only
Apr 15, 2026
Merged

fix: skip before_prompt_build hooks for subagent sessions (#601)#613
rwmjhb merged 8 commits intoCortexReach:masterfrom
jlin53882:fix/issue601-subagent-only

Conversation

@jlin53882
Copy link
Copy Markdown
Contributor

@jlin53882 jlin53882 commented Apr 14, 2026

Summary

Fixes Issue #601 — sub-agent sessions cause gateway blocking by skipping expensive before_prompt_build hooks for :subagent: sessions.

When a sub-agent session runs a before_prompt_build hook, it performs LanceDB I/O (auto-recall + reflection-injection) sequentially on the same thread, blocking all other user sessions. Since sub-agent sessions already inherit their full context from the parent agent, these operations are redundant and can be safely skipped.


Problem

Root cause: The memory-lancedb-pro plugin runs two expensive hooks on every before_prompt_build event:

  1. auto-recallstore.get() + LanceDB embedding/rerank
  2. reflection-injector (inheritance + derived) → loadAgentReflectionSlices() + store.get()

For sub-agent sessions, these operations are redundant because:

  • Sub-agent context already comes from the parent agent's conversation
  • The I/O runs sequentially on the gateway thread
  • While the sub-agent does I/O, all user sessions are blocked

Reproduction: Spawn a sub-agent via sessions_spawn(runtime="subagent"). The sub-agent's sessionKey contains :subagent: (e.g., agent:main:channel:123:subagent:def456). During the sub-agent's first prompt build, the gateway thread is occupied with LanceDB I/O, causing all other sessions to wait.


Solution

Add a pre-check guard at the top of each before_prompt_build hook:

const sessionKey = typeof ctx.sessionKey === "string" ? ctx.sessionKey : "";
if (sessionKey.includes(":subagent:")) return;

Why it's safe:

  • Sub-agent sessions get context from parent — skipping does NOT lose information
  • User sessions are unaffected — their sessionKey does not contain :subagent:
  • This is a pre-check early return, not a change to hook behavior
  • Guard is placed before all expensive operations (verified by automated test)

Changes

index.ts — 3 sub-agent skip guards

# Hook Feature Line (approx.)
1 before_prompt_build auto-recall ~2224
2 before_prompt_build reflection-injector (inheritance) ~3089
3 before_prompt_build reflection-injector (derived) ~3118

Each hook adds the same guard pattern:

const sessionKey = typeof ctx.sessionKey === "string" ? ctx.sessionKey : "";
if (sessionKey.includes(":subagent:")) return;

SessionKey format: agent:main:subagent:... (confirmed from openclaw hooks source:
"Sub-agents have sessionKey patterns like 'agent:main:subagent:...'")

test/issue598_smoke.mjs — Smoke test

test/issue601_behavioral.mjs — Behavioral test (new)

Addresses AliceLJY's review comment: the smoke test is source-string matching, not behavioral testing.

Test 1 — Guard logic (13 cases):

  • 4 sub-agent sessionKey formats → guard returns true
  • 9 normal/edge cases → guard returns false (type-safe: null, undefined, "", numeric)

Test 2 — Guard placement (3 hooks):
Verifies :subagent: guard appears before all expensive operations:

  • store.get() — Hook 1, 2
  • store.update() — Hook 1
  • loadAgentReflectionSlices() — Hook 2, 3
  • recallWork() — Hook 1

Test 3 — Behavioral simulation:

  • Sub-agent sessionKey (agent:main:channel:123:subagent:def456) → hook returns early, no expensive ops called
  • Normal sessionKey (agent:main:channel:123) → hook proceeds with expensive ops

Testing

# Smoke test
node test/issue598_smoke.mjs
# Expected: PASS — subagent sessions skipped before async work

# Behavioral test
node test/issue601_behavioral.mjs
# Expected: ALL PASSED
#   - Guard logic: 13 cases
#   - Guard placement: 3 hooks verified
#   - Behavioral simulation: 4 cases

Both tests must pass.


Reviews

Codex Adversarial Review ✅

  • Guard logic: 13 edge cases all correct
  • Guard placement: guard precedes all expensive ops in all 3 hooks
  • Behavioral simulation: sub-agent bypass verified, normal session unaffected
  • Type-safe: null/undefined/numeric/empty all handled correctly

Style note (resolved)

  • Before: Hook 1 used ctx?.sessionKey, Hook 2/3 used ctx.sessionKey
  • After: All 3 hooks unified to ctx.sessionKey (commit 4e80f01)

Commits

SHA Description
ebf4fb7 fix: increase guard pattern distance from 200 to 2000 chars
9e5411f test: add behavioral test for subagent skip guards
4e80f01 fix: unify sessionKey guard to ctx.sessionKey (no optional chaining)

Related


Closes #601

…#598, CortexReach#601)

Cherry-pick of PR CortexReach#603 (memory leak fixes) + Nit fixes from AliceLJY review:
- store.ts: tail-reset semaphore replaces unbounded promise chain
- access-tracker.ts: separate retryCount map + Nit#4 merge delta on retry race
- embedder.ts: TTL eviction on set() when near capacity
- retrieval-stats.ts: ring buffer + Nit#6 inline iteration (no intermediate array)
- noise-prototypes.ts: DEDUP_THRESHOLD 0.95->0.90
- index.ts: subagent skip guards for before_prompt_build hooks
- test/issue598_smoke.mjs: Nit#1 hardcoded path -> import.meta.url relative path
…h#601)

Prevents LanceDB I/O from blocking user gateway sessions.
- index.ts: 3 subagent guard checks added to before_prompt_build hooks
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 386ef4aa9a

ℹ️ 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".

Comment thread test/issue598_smoke.mjs Outdated
}

// Verify: before_prompt_build hook exists and has the subagent guard
const hookGuardPattern = /before_prompt_build[\s\S]{0,200}:subagent:/;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Widen subagent-hook regex to avoid false smoke-test failures

Running node test/issue598_smoke.mjs currently exits with FAIL: before_prompt_build hook is missing ':subagent:' guard even though the guards were added, because this pattern only allows 200 characters between before_prompt_build and :subagent:. In index.ts, the nearest matches are farther apart (about 236+ chars), so the new verification script is a false negative and blocks the commit’s documented test workflow.

Useful? React with 👍 / 👎.

…#601)

- Mirrors exact guard logic from index.ts (sessionKey.includes(':subagent:'))
- SessionKey format confirmed from openclaw hooks source: agent:main:subagent:...
- Test 1: Guard logic (13 cases - 4 subagent keys + 9 normal/edge/type-safe)
- Test 2: Guard placement (verifies :subagent: guard precedes all expensive ops in all 3 hooks)
- Test 3: Behavioral simulation (subagent bypasses vs normal proceeds)
- Smoke test (issue598_smoke.mjs) still passes alongside this
jlin53882 added a commit to jlin53882/memory-lancedb-pro that referenced this pull request Apr 14, 2026
rwmjhb pushed a commit that referenced this pull request Apr 15, 2026
* fix: 5 memory leaks + subagent skip + AliceLJY nit fixes (#598, #601)

Cherry-pick of PR #603 (memory leak fixes) + Nit fixes from AliceLJY review:
- store.ts: tail-reset semaphore replaces unbounded promise chain
- access-tracker.ts: separate retryCount map + Nit#4 merge delta on retry race
- embedder.ts: TTL eviction on set() when near capacity
- retrieval-stats.ts: ring buffer + Nit#6 inline iteration (no intermediate array)
- noise-prototypes.ts: DEDUP_THRESHOLD 0.95->0.90
- index.ts: subagent skip guards for before_prompt_build hooks
- test/issue598_smoke.mjs: Nit#1 hardcoded path -> import.meta.url relative path

* fix: logger.error optional chaining to prevent TypeError when logger only has warn

* fix: route destroy() flush through flush() to avoid concurrent writes + catch unhandled rejection

* fix: clear pending synchronously in destroy() before async flush + remove unused this.logger.error

* fix: resolve 5 memory leak issues causing heap OOM (#598)

Cherry-pick from PR #603:
- store.ts: tail-reset semaphore replaces unbounded promise chain
- access-tracker.ts: separate retryCount map, maxRetries=5, destroy() with 3s timeout
- embedder.ts: TTL eviction on set() when near capacity
- retrieval-stats.ts: ring buffer replaces Array.shift() - O(1) write
- noise-prototypes.ts: DEDUP_THRESHOLD 0.95->0.90
- test/issue598_smoke.mjs: smoke test for subagent guards

* test: add regression tests for Issue #598 memory leak fixes

- store-serialization.test.mjs: verifies runSerializedUpdate executes sequentially
- access-tracker-retry.test.mjs: verifies retry delta does not amplify, max retries drops
- embedder-cache.test.mjs: verifies embedder config accepts TTL params

Refs: issues #598

* test: improve regression tests per Codex review feedback

- store-serialization: add in-flight concurrency check + exception release test
- access-tracker-retry: add new writes during retry test + precise delta check
- embedder-cache: add actual TTL config verification

Refs: issues #598

* test: precise metadata count verification for access-tracker + improved embedder TTL test

- access-tracker: add precise metadata count check using parseAccessMetadata
  (verifies final accessCount=3 matches expected 1+2 delta merge)
- embedder-cache: clarify TTL test with config acceptance + OLLAMA note
- pre-seed memory in retry test so getById returns data (not null)

Refs: Codex Round 2 feedback

* test: fix embedder-cache to not use non-existent config fields

- Remove maxCacheSize and cacheTtlMinutes from test (they don't exist in EmbeddingConfig)
- Document that TTL eviction uses hardcoded defaults (256, 30 min)
- The fix is _evictExpired() on set(), not configurable params

Ref: Codex Round 3 feedback

* test: rename embedder-cache to smoke test, clarify coverage limits

- Rename from 'regression test' to 'smoke test' per Codex Round 4 feedback
- Document that _evictExpired() on set() requires OLLAMA server for full test
- Fix passes now shows actual cache size/hits/misses verification

Ref: Codex Round 4

* ci: add Issue #598 regression tests to CI manifest

- test/store-serialization.test.mjs: store.ts tail-reset serialization
- test/access-tracker-retry.test.mjs: access-tracker delta amplification
- test/embedder-cache.test.mjs: embedder TTL eviction (smoke)

Ref: issues #598

* test: remove issue598_smoke.mjs from PR #611 (belongs to PR #613 subagent skip)

* fix(cli-smoke): add missing store.count() mock (fixes pre-existing bug from 0988a46)

* revert: undo cli-smoke fix (belongs to dedicated issue)
@rwmjhb rwmjhb merged commit 1278dfa into CortexReach:master Apr 15, 2026
6 of 7 checks passed
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.

[BUG] Sub-agent sessions cause gateway blocking — before_prompt_build hooks starve other sessions

2 participants