Dynamic instanceof via catch-block import is fragile

BeforeLlmCallBlockError already carries the sentinel property isBeforeLlmCallBlock = true for exactly this situation. Using instanceof via a dynamic import in a catch block has a subtle failure mode: if a bundler or loader ever creates two separate module instances of hook-stream-wrapper.js (e.g., different chunks or a dual-package hazard), the class references won't be the same object and instanceof will return false. The block error would then fall through to the else branch, setting promptError and surfacing a run failure instead of the intended graceful termination.

The sentinel property is already there — prefer checking it directly:

This removes the dynamic import entirely and is immune to the dual-instance problem.

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/run/attempt.ts
Line: 1817-1818

Comment:
**Dynamic `instanceof` via catch-block import is fragile**

`BeforeLlmCallBlockError` already carries the sentinel property `isBeforeLlmCallBlock = true` for exactly this situation. Using `instanceof` via a dynamic import in a `catch` block has a subtle failure mode: if a bundler or loader ever creates two separate module instances of `hook-stream-wrapper.js` (e.g., different chunks or a dual-package hazard), the class references won't be the same object and `instanceof` will return `false`. The block error would then fall through to the `else` branch, setting `promptError` and surfacing a run failure instead of the intended graceful termination.

The sentinel property is already there — prefer checking it directly:

```suggestion
          const isBlockError =
            err instanceof Error &&
            (err as { isBeforeLlmCallBlock?: boolean }).isBeforeLlmCallBlock === true;
          if (isBlockError) {
```

This removes the dynamic import entirely and is immune to the dual-instance problem.

How can I resolve this? If you propose a fix, please make it concise.

Iteration counter is likely off-by-one for the first LLM call

hookIterationRef starts at 0, and the subscription increments it on every turn_start event. In the agent loop, turn_start fires before the LLM call (streamFn) in each turn. This means:

1. hookIterationRef.current = 0 (initial)
2. turn_start fires → hookIterationRef.current = 1
3. Wrapped streamFn is called → event.iteration = 1 ← first call

The tests in hook-stream-wrapper.test.ts manually set iterationRef.current = 0 for the first call and = 1 for the second, establishing the expected contract that the first LLM call maps to iteration: 0. In production the first call will be iteration: 1, and subsequent calls will be iteration: N+1 relative to test expectations.

If turn_start does fire before streamFn, this is an off-by-one. Two straightforward fixes:

• Option A: Initialise hookIterationRef = { current: -1 } so that after the first turn_start increment the first LLM call sees 0.
• Option B: Keep { current: 0 } and increment after the LLM call (e.g. on turn_end or llm_response).

It's worth verifying the turn_start / streamFn ordering in pi-agent-core to confirm the direction of the fix.

This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/run/attempt.ts
Line: 1532-1538

Comment:
**Iteration counter is likely off-by-one for the first LLM call**

`hookIterationRef` starts at `0`, and the subscription increments it on every `turn_start` event. In the agent loop, `turn_start` fires **before** the LLM call (`streamFn`) in each turn. This means:

1. `hookIterationRef.current = 0` (initial)
2. `turn_start` fires → `hookIterationRef.current = 1`
3. Wrapped `streamFn` is called → `event.iteration = 1` ← first call

The tests in `hook-stream-wrapper.test.ts` manually set `iterationRef.current = 0` for the first call and `= 1` for the second, establishing the expected contract that the first LLM call maps to `iteration: 0`. In production the first call will be `iteration: 1`, and subsequent calls will be `iteration: N+1` relative to test expectations.

If `turn_start` does fire before `streamFn`, this is an off-by-one. Two straightforward fixes:

- **Option A:** Initialise `hookIterationRef = { current: -1 }` so that after the first `turn_start` increment the first LLM call sees `0`.
- **Option B:** Keep `{ current: 0 }` and increment *after* the LLM call (e.g. on `turn_end` or `llm_response`).

It's worth verifying the `turn_start` / `streamFn` ordering in `pi-agent-core` to confirm the direction of the fix.

How can I resolve this? If you propose a fix, please make it concise.

Avoid reusing prior assistant output when hook blocks call

When before_llm_call blocks, this branch only logs and leaves promptError unset, so the attempt is treated as successful. In that path assistantTexts stays empty, and payload building falls back to lastAssistant from session history (src/agents/pi-embedded-runner/run/payloads.ts:195-251), which can resend a stale prior assistant reply if the conversation already had one. That turns a blocked call into incorrect user-visible output instead of a clean no-response outcome.

Useful? React with 👍 / 👎.

Enforce allowPromptInjection for before_llm_call hooks

This introduces before_llm_call as a fully valid typed hook, but it is not covered by the prompt-injection policy gate. registerTypedHook only enforces plugins.entries.<id>.hooks.allowPromptInjection=false for names in isPromptInjectionHookName (src/plugins/registry.ts:519), and that set still excludes before_llm_call (src/plugins/types.ts:380-383), so a plugin can still mutate messages/systemPrompt through this hook even when operators explicitly disable prompt injection for that plugin.

Useful? React with 👍 / 👎.