Skip to content

[pull] main from danny-avila:main#60

Merged
pull[bot] merged 1 commit intoinnFactory:mainfrom
danny-avila:main
Apr 12, 2026
Merged

[pull] main from danny-avila:main#60
pull[bot] merged 1 commit intoinnFactory:mainfrom
danny-avila:main

Conversation

@pull
Copy link
Copy Markdown

@pull pull Bot commented Apr 12, 2026

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 : )

* 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).
@pull pull Bot locked and limited conversation to collaborators Apr 12, 2026
@pull pull Bot added the ⤵️ pull label Apr 12, 2026
@pull pull Bot merged commit fd87491 into innFactory:main Apr 12, 2026
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant