Skip to content

Add managed hook suppression#21577

Closed
abhinav-oai wants to merge 1 commit intomainfrom
abhinav/managed-hook-suppress
Closed

Add managed hook suppression#21577
abhinav-oai wants to merge 1 commit intomainfrom
abhinav/managed-hook-suppress

Conversation

@abhinav-oai
Copy link
Copy Markdown
Collaborator

@abhinav-oai abhinav-oai commented May 7, 2026

Why

Managed hooks from requirements can be intentionally noisy when they run every turn, and the current runtime always surfaces their lifecycle notifications to clients. We want admins to be able to suppress only those managed hook notifications without giving user config a way to hide local/plugin hooks.

What

  • add a managed-requirements-only suppress field for command hooks
  • fail open when suppress appears in user hook config by ignoring it and emitting a warning
  • carry suppression through hook dispatch and omit hook/started + hook/completed notifications for suppressed runs while still recording metrics/analytics
  • keep the app-server requirements API shape unchanged for now; Codex app inherits suppression automatically because it renders from lifecycle notifications

@abhinav-oai abhinav-oai force-pushed the abhinav/managed-hook-suppress branch from 9a2cf45 to b1dec90 Compare May 7, 2026 19:34
@abhinav-oai abhinav-oai force-pushed the abhinav/managed-hook-suppress branch from b1dec90 to 1b164ba Compare May 7, 2026 19:47
Copy link
Copy Markdown
Contributor

@eternal-openai eternal-openai left a comment

Choose a reason for hiding this comment

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

Hey I know I'm reviewing this too early since it's still a draft, but there are some issues:

  • the mechanism of just dropping the notifications doesn't really work, as elegant as that would be. consider the case where the hooks still blocks or crashes - the UI would need to then make what happened visible, rather than just stopping the thread without explanation. there's probably other things that still need visual treatment, like stop-block continuation prompts originating from hidden hooks
  • so we actually do need to thread hook visibility through the notifications, and just have the UIs decide to hide them more intelligently
  • some of the renames here feel like a bit more churn than is necessary
  • i think the naming could be better. How about something like visibility_hint: "default" | "hidden" ? my concern is that the current naming overlaps too much with the spec's suppressOutput which means something different

@abhinav-oai
Copy link
Copy Markdown
Collaborator Author

abhinav-oai commented May 8, 2026

Hey I know I'm reviewing this too early since it's still a draft, but there are some issues:

  • the mechanism of just dropping the notifications doesn't really work, as elegant as that would be. consider the case where the hooks still blocks or crashes - the UI would need to then make what happened visible, rather than just stopping the thread without explanation. there's probably other things that still need visual treatment, like stop-block continuation prompts originating from hidden hooks
  • so we actually do need to thread hook visibility through the notifications, and just have the UIs decide to hide them more intelligently
  • some of the renames here feel like a bit more churn than is necessary
  • i think the naming could be better. How about something like visibility_hint: "default" | "hidden" ? my concern is that the current naming overlaps too much with the spec's suppressOutput which means something different

hmm that's a great point

visibility_hint is good, I like the enumerated settings as well so we can expand in the future too.

another idea that could be interesting is exposing as part of normal config but still have requirements clamp the value. I think normal users would also like this config as I've seen a few related issuesL

will also take a pass and clean it up overall!

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.

2 participants