fix: classify chat lifecycle hook denials as policy decisions - #27552
fix: classify chat lifecycle hook denials as policy decisions#27552ibetitsmike wants to merge 2 commits into
Conversation
Map the structured 403 denial body to the hook_denied kind so an existing chat titles it 'Blocked by policy' instead of 'Request failed', and give the create form dedicated branches for both hook outcomes so a policy decision no longer renders as a developer error with a stack trace.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37a65f2d11
ℹ️ 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".
| const kind = isChatHookDeniedResponse(error.response?.data) | ||
| ? "hook_denied" |
There was a problem hiding this comment.
When a lifecycle hook rejects a send or edit in an existing chat, this new branch changes the rendered title to Blocked by policy, but the added stories cover only AgentCreateForm; the existing AgentChatPage story exercises hook_dispatch_failed and has no hook_denied scenario. Add an AgentChatPage.stories.tsx play story that submits a message, returns the structured 403 response, and asserts the policy title and message so this behavior is covered as required by FE1.
AGENTS.md reference: site/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
1 participant
Stack Context
Top of the chat lifecycle hooks stack (#27401 → #27428 → #27429 → #27430 → #27551). Fixes the last two UAT findings, which are coupled because both need a way to tell a policy decision apart from a failure.
What?
Adds a
hook_deniedchat error kind and a structured 403 denial response, then uses it on both surfaces that render hook outcomes.Blocked by policyinstead ofRequest failed.Go to workspacesaction.Why?
The dispatch-failure path already returns a structured
ChatHookDispatchFailedResponsecarryingkind: "hook_dispatch_failed", which the frontend uses to title itLifecycle hook failed. The denial path wrote a barecodersdk.Responsewith nokind, so the frontend could not classify it, fell back togeneric, and renderedRequest failed. A policy decision is not a failure, and the old title carried no signal about what happened.This needs a backend discriminator rather than a frontend-only fix. Matching on
status === 403alone would misclassify ordinary permission errors, which reach the same create-form branch and must keep their existing treatment. That case is pinned by the pre-existingForbiddenErrorWithRolestory, which still passes unchanged.No migration:
ChatErrorKindis persisted only inside the JSONBchats.last_errorcolumn, and the decoder accepts unknown kinds and defaults only blank ones.Testing
kind.Stack TraceandResponse data.make genconfirmed idempotent,golangci-lint,pnpm lint:types,pnpm lint:compiler,pnpm run lint-docs.