Skip to content

feat: hooks POC#1679

Open
chase-crumbaugh wants to merge 21 commits intomainfrom
chase/hooks
Open

feat: hooks POC#1679
chase-crumbaugh wants to merge 21 commits intomainfrom
chase/hooks

Conversation

@chase-crumbaugh
Copy link
Member

@chase-crumbaugh chase-crumbaugh commented Feb 23, 2026

An initial pass at Hooks. Pending further testing and productionization work


Open with Devin

@vercel
Copy link

vercel bot commented Feb 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gram-docs-redirect Ready Ready Preview, Comment Feb 27, 2026 10:56pm

Request Review

@changeset-bot
Copy link

changeset-bot bot commented Feb 23, 2026

🦋 Changeset detected

Latest commit: ba5daaf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
dashboard Patch
@gram/client Patch
server Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Feb 24, 2026

atlas migrate lint on server/migrations

Status Step Result
No migration files detected  
ERD and visual diff generated View Visualization
No issues found View Report
Read the full linting report on Atlas Cloud

@github-actions
Copy link
Contributor

github-actions bot commented Feb 24, 2026

atlas migrate lint on server/clickhouse/migrations

Status Step Result
1 new migration file detected 20260226021100_add-tool-event-source-cols.sql
ERD and visual diff generated View Visualization
No issues found View Report
Read the full linting report on Atlas Cloud

@blacksmith-sh

This comment has been minimized.

@chase-crumbaugh chase-crumbaugh changed the title wip: hooks feat: hooks POC Feb 24, 2026
@chase-crumbaugh chase-crumbaugh marked this pull request as ready for review February 24, 2026 20:29
@chase-crumbaugh chase-crumbaugh requested a review from a team as a code owner February 24, 2026 20:29
devin-ai-integration[bot]

This comment was marked as resolved.

@blacksmith-sh

This comment has been minimized.

Copy link
Contributor

@tgmendes tgmendes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good overall, but we should separate the migrations to it's own PR for isolation.

Also it feels like there are really 2 different changes going on in this PR which I would've consider separating:

  1. Tool name renaming (not using gram URN)
  2. Actual hooks work

Either way as long as the migrations are in a separate PR I think this should be good to go soon

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 19 additional findings in Devin Review.

Open in Devin Review

validate_env_vars() {
local hook_event_name="$1"

echo "RUNNING HOOK $hook_event_name"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Debug echo to stdout corrupts JSON hook response

The validate_env_vars function prints echo "RUNNING HOOK $hook_event_name" to stdout (line 13). Since Claude Code hooks expect the command's stdout to be a valid JSON response, this debug line prepends non-JSON text before the actual JSON output from output_success_json or output_block_json, producing malformed output like:

RUNNING HOOK PreToolUse
{
  "hookSpecificOutput": { ... }
}
Root Cause and Impact

The echo on line 13 of hooks/core/common.sh writes to stdout instead of stderr. All three hook scripts (pre_tool_use.sh, post_tool_use.sh, post_tool_use_failure.sh) call validate_env_vars before call_gram_api, so every hook invocation prepends this debug text to stdout.

For the standalone install (configured in hooks/install.sh:84-120), hooks are synchronous (no async: true), meaning Claude Code reads stdout to parse the JSON response. The malformed output will cause parse failures for:

  • PreToolUse: Could cause the permission decision to fail, potentially blocking all tool execution.
  • PostToolUse / PostToolUseFailure: Could cause Claude Code to misinterpret the hook result.

For the plugin (hooks/plugin-claude/hooks/hooks.json), hooks are marked "async": true, so stdout may be ignored — limiting the impact to the standalone install.

The error output in call_gram_api correctly uses >&2 (stderr) at hooks/core/common.sh:103-105, showing the intent was to keep stdout clean for JSON.

Suggested change
echo "RUNNING HOOK $hook_event_name"
echo "RUNNING HOOK $hook_event_name" >&2
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@blacksmith-sh
Copy link
Contributor

blacksmith-sh bot commented Feb 27, 2026

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
github.com/speakeasy-api/gram/server/internal/telemetry/
TestSearchToolCalls_AggregatesByTraceID
View Logs
github.com/speakeasy-api/gram/server/internal/telemetry/
TestSearchToolCalls_FilterByGramURN
View Logs
github.com/speakeasy-api/gram/server/internal/telemetry/
TestSearchToolCalls_FilterByGramURN/exact_match_returns_single_result
View Logs
github.com/speakeasy-api/gram/server/internal/telemetry/
TestSearchToolCalls_FilterByGramURN/no_match_returns_empty
View Logs
github.com/speakeasy-api/gram/server/internal/telemetry/
TestSearchToolCalls_FilterByGramURN/partial_match_on_source_returns_multiple_results
View Logs
github.com/speakeasy-api/gram/server/internal/telemetry/
TestSearchToolCalls_FilterByGramURN/partial_match_on_tool_name
View Logs

Fix in Cursor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants