fix: route Codex telemetry hooks through wrapper#3116
Conversation
🦋 Changeset detectedLatest commit: 5549aa0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc7cf24895
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🚀 Preview Environment (PR #3116)Preview URL: https://pr-3116.dev.getgram.ai
Gram Preview Bot |
There was a problem hiding this comment.
2 issues found across 4 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7ba7f6f8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
2 issues found across 12 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
- Pipe the enriched Codex hook payload to curl via stdin (--data-binary @-) instead of passing it as an argv, avoiding ARG_MAX truncation for large PostToolUse events. - Refresh the trusted_hash on existing Gram-managed [hooks.state] entries during install so upgrades that change a hook command (hook.sh -> hook_async.sh) don't get flagged as modified and silently stop telemetry. - Consult the session metadata cache regardless of payload user_email, filling each field only when empty, to avoid redundant user lookups. - Restore the trailing newline on generated openapi3.yaml to fix the "Check for dirty files" CI step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tigation-investigate-async-hook-support-in-codex # Conflicts: # server/internal/plugins/generate.go
- Regenerate the internal SDK and bundled OpenAPI so the codex hook user_email field is reflected (Check SDK is up-to-date). - Replace forbidden t.Skip calls in the install-script regression test with require.NoError (forbidigo GG004). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
hook_async.shwrapper so telemetry events return immediately without using unsupportedhooks.jsonasync fields.hook.shand update trusted hash generation to match the per-event command path.Linear
Test plan
go test ./server/internal/pluginsSummary by cubic
Routes Codex telemetry-only hooks through a background wrapper and adds
user_emailattribution so events return fast and map to the right user. Also pipes payloads via stdin and refreshes per-event trusted hashes on upgrade to keep telemetry reliable. Addresses AGE-2563.SessionStart,PostToolUse, andStopviahook_async.sh; others stay onhook.sh.hook_async.shthat copies stdin, forwards tohook.shin the background, and exits 0; ensures it’s executable in the ZIP.user_emailto the API/payload;hook.shderives it onSessionStartfrom local Codex auth, server caches session metadata, resolves user, and applies it across events.--data-binary @-) to avoid ARG_MAX truncation on large events.[hooks.state]trusted_hashduring install so upgrades don’t silently drop telemetry.user_email.Written for commit 5549aa0. Summary will update on new commits.