Skip to content

fix(opencode): fire read hooks for image and binary file parts#30694

Open
ulises-jeremias wants to merge 1 commit into
anomalyco:devfrom
ulises-jeremias:fix/opencode-read-hook-image-binary
Open

fix(opencode): fire read hooks for image and binary file parts#30694
ulises-jeremias wants to merge 1 commit into
anomalyco:devfrom
ulises-jeremias:fix/opencode-read-hook-image-binary

Conversation

@ulises-jeremias
Copy link
Copy Markdown
Contributor

Issue for this PR

Closes #30648

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Fixes a hook consistency gap for file parts in prompt resolution.

Previously, when resolving non-text/non-directory file parts (e.g. image/binary attachments), SessionPrompt.resolvePart read the file directly and created a data URL without invoking plugin read hooks. As a result, tool.execute.before did not fire for image/binary file reads.

This PR updates that branch to trigger Read-equivalent hooks:

  • tool.execute.before with args: { filePath }
  • file read execution
  • tool.execute.after with a minimal output payload (Binary read successfully)

Behavior is otherwise preserved: attachments are still forwarded as data URLs.

Also included:

  • preserve error observability parity in this branch (log.error + Session.Event.Error)
  • regression test in packages/opencode/test/session/prompt.test.ts verifying both tool.execute.before and tool.execute.after fire for image file parts and receive expected arguments.

How did you verify your code works?

  • Added regression test:
    • fires tool.execute.before for image file parts
  • Attempted to run targeted test from packages/opencode:
    • bun test test/session/prompt.test.ts -t "fires tool.execute.before for image file parts"
  • In this environment test execution is blocked by pre-existing workspace/module resolution errors:
    • Cannot find module '@opencode-ai/llm' from packages/core/src/session/event.ts

Screenshots / recordings

Not included.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@ulises-jeremias
Copy link
Copy Markdown
Contributor Author

Self-review summary:

  • ✅ Root cause addressed in the exact bypass path (SessionPrompt.resolvePart binary/file branch).
  • ✅ Hook parity restored for image/binary reads by invoking:
    • tool.execute.before with { filePath }
    • tool.execute.after with output metadata
  • ✅ Existing behavior preserved: file attachment is still added as data URL.
  • ✅ Error observability preserved in this branch (log.error + Session.Event.Error) for consistency with text/directory read paths.
  • ✅ Added regression test proving plugin hooks execute for image file parts and receive expected args/output.
  • ✅ Change scope is minimal and limited to prompt file-part resolution + focused test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tool.execute.before hook does not fire for image/binary file reads

1 participant