[pull] main from danny-avila:main#60
Merged
pull[bot] merged 1 commit intoinnFactory:mainfrom Apr 12, 2026
Merged
Conversation
* feat: wire run-level hooks into processStream (phase 1/2) Fires four hook lifecycle events from `Run.processStream`, making the inert hooks module from PR #87 live. Hosts pass a pre-constructed `HookRegistry` via `RunConfig.hooks`; when absent, all hook dispatch is skipped (zero overhead for non-adopters). ## Integration points - `RunConfig.hooks?: HookRegistry` — new optional field, mirrors the existing `customHandlers` pattern - `Graph.hookRegistry` — field alongside `handlerRegistry`, cleared in `clearHeavyState()` - `Run` constructor wires the registry onto both `this.hookRegistry` and `this.Graph.hookRegistry` ## Hook fire points in `processStream` 1. **RunStart** — after config setup, before stream creation. Receives `inputs.messages`, `runId`, `threadId`, `agentId`. 2. **UserPromptSubmit** — immediately after RunStart. Extracts prompt text from the last human message. If the hook returns `{decision: 'deny'}` or `{decision: 'ask'}`, the run aborts early and returns `undefined`. The `ask` case is an abort in v1 — the SDK returns the decision and the host decides how to implement interactive approval (see Phase 2 plan in the skills design report). 3. **Stop** — at the end of the try block, after the for-await loop. Receives the accumulated messages from `Graph.getRunMessages()`. Only fires on successful completion (not on error). 4. **StopFailure** — in a new catch block between try and finally. Receives the error message and last assistant message. Hook errors are swallowed (`.catch(() => {})`) so the original error propagates. ## Session teardown `hookRegistry.clearSession(runId)` is called in the finally block, guaranteeing cleanup even on error. Fulfills §3.9 rule #5 from the design report. ## Public API `src/hooks/` is now re-exported from `src/index.ts`. Hosts can import `HookRegistry`, `executeHooks`, and all hook types directly from `@librechat/agents`. ## Helper functions Three module-level functions added to `run.ts`: - `findLastHumanMessage` — backward scan for `getType() === 'human'` - `findLastAssistantMessage` — backward scan for `getType() === 'ai'` - `extractPromptText` — handles string and complex content arrays ## Tests 10 new integration tests using `Run.create` + `overrideTestModel` + `processStream` (same pattern as `custom-event-await.test.ts`): - RunStart fires with correct fields - UserPromptSubmit extracts prompt, deny aborts, ask aborts - Stop fires on success, does not fire on error - StopFailure fires on error, preserves original exception - Session cleared in finally (both success and error paths) - No-hooks baseline works identically Total: 81 existing + 10 integration = 91 passing. * fix(hooks): address PR 2 review — DRY helpers, test coverage, guards Resolves all findings from the comprehensive review on #88. - #1 MAJOR: Add 3 tests for `extractPromptText` — multi-part content (text + image blocks yields 'hello\nworld'), image-only content (yields ''), and empty content array (yields ''). - #2 MINOR: Merge `findLastHumanMessage`/`findLastAssistantMessage` into parameterized `findLastMessageOfType(messages, type)`. - #3 MINOR: Rewrite `extractPromptText` array path from `.filter().map().join()` triple-pass to single `for...of` loop accumulating into `parts[]` then `.join('\n')`. - #4 MINOR: Add `config.callbacks = undefined` on deny/ask early return path so Langfuse handler reference is cleaned up even though no stream ran. - #5 MINOR: Add comment on `UserPromptSubmit` fire point noting `attachments` is not yet wired (Phase 2). - #6 MINOR: Strengthen `StopFailure` test assertion from `toBeTruthy()` to `typeof === 'string'` + `length > 0`. - #7 MINOR: Add test for empty-content human message — verifies `UserPromptSubmit` fires with `prompt: ''`. - #8 NIT: Add inline comment on `stopHookActive: false` explaining it will be `true` in Phase 2 when stop is triggered by a hook. - #9 NIT: Use `hasHookFor('Stop', sessionId)` and `hasHookFor('StopFailure', sessionId)` guards before `getRunMessages()` to avoid the array copy when no hooks are registered for the event. - CL-2: Wrap Stop hook dispatch in its own `.catch()` so an infrastructure error in `executeHooks` (theoretical — it catches internally) cannot masquerade as a stream failure in the catch block. Tests: 91 -> 94 (+3 for extractPromptText coverage).
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )