A faulty tools/post-execute listener can make every tool appear failed after execution

[Tobi1chi]

Summary

A faulty in-process third-party plugin can currently make unrelated tools appear to fail after they have already executed.

In the incident that prompted this discussion, @deepseek-ai/dsh-tool-indexed-search registered a global tools/post-execute listener intended to process locate results. The listener did not first check exec.name === 'locate', so it attempted to access value.results.length for results from unrelated tools such as read, pwsh, glob, skill, Cordis, and job_list. Those values do not necessarily have a results property, producing:

Cannot read properties of undefined (reading 'length')

This is directly a plugin bug, not a claim that DSH core tools are broken. However, it exposes a fault-isolation, diagnostics, and side-effect ambiguity gap at the trusted in-process post-execute extension point.

Minimal reproduction

A listener shaped like this is sufficient:

ctx.on('tools/post-execute', async (exec, result, next) => {
  const decision = await next()
  if (decision.kind !== 'accept') return decision

  const value = result.value as LocateValue | undefined
  if (value === undefined || value.results.length <= caps.maxResults || value.truncated === false) {
    return decision
  }

  // ...
})

If it is registered globally and omits an exec.name === 'locate' guard, a non-locate result can cause the listener to throw.

Observed behavior

• After the plugin was hot-mounted through its profile patch, the next read call immediately failed at tools/post-execute.
• Subsequent calls to unrelated tools (including pwsh, glob, skill, job_list, and Cordis) all returned the same error; more than 20 calls were affected.
• The harness itself remained alive. The exception was confined to an individual tool result, but the same failing global listener participated in every later call, making the tool surface effectively unavailable.
• In packages/core/tools/src/index.ts, finalizeScheduledExecution / postExecute converts a throwing post-execute listener into an isError result. packages/core/tools/tests/tools.spec.ts explicitly tests this behavior.

The most concerning consequence is semantic: the tool body may already have produced side effects before post-execute changes the result to an error. An agent can reasonably interpret that result as “the tool did not execute” and retry, duplicating a write, command, or other side effect.

Why simply swallowing the exception is unsafe

Post-execute hooks may implement permission checks, policy enforcement, or other security-sensitive controls. Silently ignoring an exception would risk fail-open behavior. The existing fail-closed direction is understandable; the issue is that it currently provides little attribution or containment when a global third-party listener is defective.

Possible improvements

1. Include listener provenance in post-execute failures: plugin identity, scope, and registration source where available.
2. Provide diagnostics and a controlled circuit-breaker mechanism for repeated failures, while keeping enforcement/policy listeners fail-closed.
3. Distinguish policy/enforcement listeners from decoration/observation listeners, with deliberately different isolation semantics.
4. Offer tool-specific result-processing APIs so plugins do not need a broad global hook for a single tool type.
5. Add plugin development guidance and integration tests that mount a plugin and then execute an unrelated tool.
6. Consider an explicit result state for “tool body completed, post-processing failed,” so agents do not mistake it for a non-executed operation and retry side effects.

Environment

Windows; DSH 0.1.0-rc.5.

Would maintainers be open to discussing the best combination of provenance, fail-closed containment, and post-execution outcome semantics here?