refactor(exec): unify channel approvals and restore routing/auth#57838
Conversation
Greptile SummaryThis PR consolidates the Discord and Telegram exec-approval flow — runtime lifecycle, native delivery routing, and channel auth — from four stacked PRs into a single mainline-safe change. A new shared Key findings:
Confidence Score: 4/5Safe to merge after fixing the misleading not-authorized error message in the plugin-only /approve fallback path. One P1 logic bug remains: in commands-approve.ts the else-if (pluginApprovalAuthorization.authorized) branch returns an authorization-flavored message when the approval is not found, even though the user is authorized. All other findings are clean — the shared runtime, delivery-plan helper, auth refactor, and both channel adapters look correct. src/auto-reply/reply/commands-approve.ts (lines 247-252 — wrong error message in plugin-only fallback)
|
| Filename | Overview |
|---|---|
| src/auto-reply/reply/commands-approve.ts | Rewrites /approve routing to use pre-computed exec/plugin authorization objects; adds Telegram-gated exec path and a new plugin-only fallback branch. Contains a P1 logic bug: the "not found" error inside the else if (pluginApprovalAuthorization.authorized) block returns the wrong error message. |
| src/infra/exec-approval-channel-runtime.ts | New shared approval runtime that centralizes gateway lifecycle, pending-state tracking, delivery/expiration races, and event dispatch. Race conditions (concurrent start/stop, delivery-before-resolution) are handled correctly. |
| src/infra/approval-native-delivery.ts | New helper to build a typed delivery plan (preferred/fallback targets, origin DM redirect) for approval notifications; deduplication and surface preference selection logic is straightforward. |
| src/infra/channel-approval-auth.ts | Adds the explicit flag to distinguish implicit allow (no channel plugin) from an explicit channel-level authorization decision. |
| src/infra/exec-approval-reply.ts | Refactors internal button-building helpers into exported buildExecApprovalActionDescriptors / buildExecApprovalCommandText, and adds parseExecApprovalCommandText for typed command parsing. |
| extensions/discord/src/monitor/exec-approvals.ts | Migrates DiscordExecApprovalHandler to the shared runtime; removes hand-rolled pending map and gateway lifecycle code; properly uses notifyOriginWhenDmOnly from the delivery plan. |
| extensions/telegram/src/exec-approvals-handler.ts | Migrates TelegramExecApprovalHandler to the shared runtime; removes hand-rolled pending map and gateway lifecycle; adds PluginApprovalRequest support by converting to exec-like requests where needed. |
| extensions/telegram/src/exec-approval-resolver.ts | New one-shot gateway client that resolves a Telegram approval; properly guards the ready-promise against duplicate settlement via readySettled. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/auto-reply/reply/commands-approve.ts
Line: 247-252
Comment:
**Wrong error message when approval not found in plugin-only path**
In this branch `pluginApprovalAuthorization.authorized === true` is guaranteed (we're inside `else if (pluginApprovalAuthorization.authorized)`), which means `reason` is typically `undefined`. So when `plugin.approval.resolve` throws an "unknown or expired approval id" error, the user receives:
```
❌ You are not authorized to approve this request.
```
…even though they ARE authorized — the approval simply doesn't exist or has expired.
Compare with the parallel exec-approval-not-found path (line ~223) which correctly returns `❌ Failed to submit approval: ${String(err)}`.
Suggested fix:
```suggestion
if (isApprovalNotFoundError(err)) {
return {
shouldContinue: false,
reply: { text: `❌ Failed to submit approval: ${String(err)}` },
};
}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(exec): align approval rebase followu..." | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a44eed09e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d363db2914
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd236fdf10
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 312f23b4ab
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52d410d056
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93d5e28415
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
…nclaw#57838) * fix(exec): add shared approval runtime * fix(exec): harden shared approval runtime * fix(exec): guard approval expiration callbacks * fix(exec): handle approval runtime races * fix(exec): clean up failed approval deliveries * fix(exec): restore channel approval routing * fix(exec): scope telegram legacy approval fallback * refactor(exec): centralize native approval delivery * fix(exec): harden approval auth and account routing * test(exec): align telegram approval auth assertions * fix(exec): align approval rebase followups * fix(exec): clarify plugin approval not-found errors * fix(exec): fall back to session-bound telegram accounts * fix(exec): detect structured telegram approval misses * test(exec): align discord approval auth coverage * fix(exec): ignore discord dm origin channel routes * fix(telegram): skip self-authored message echoes * fix(exec): keep implicit approval auth non-explicit
…nclaw#57838) * fix(exec): add shared approval runtime * fix(exec): harden shared approval runtime * fix(exec): guard approval expiration callbacks * fix(exec): handle approval runtime races * fix(exec): clean up failed approval deliveries * fix(exec): restore channel approval routing * fix(exec): scope telegram legacy approval fallback * refactor(exec): centralize native approval delivery * fix(exec): harden approval auth and account routing * test(exec): align telegram approval auth assertions * fix(exec): align approval rebase followups * fix(exec): clarify plugin approval not-found errors * fix(exec): fall back to session-bound telegram accounts * fix(exec): detect structured telegram approval misses * test(exec): align discord approval auth coverage * fix(exec): ignore discord dm origin channel routes * fix(telegram): skip self-authored message echoes * fix(exec): keep implicit approval auth non-explicit
…nclaw#57838) * fix(exec): add shared approval runtime * fix(exec): harden shared approval runtime * fix(exec): guard approval expiration callbacks * fix(exec): handle approval runtime races * fix(exec): clean up failed approval deliveries * fix(exec): restore channel approval routing * fix(exec): scope telegram legacy approval fallback * refactor(exec): centralize native approval delivery * fix(exec): harden approval auth and account routing * test(exec): align telegram approval auth assertions * fix(exec): align approval rebase followups * fix(exec): clarify plugin approval not-found errors * fix(exec): fall back to session-bound telegram accounts * fix(exec): detect structured telegram approval misses * test(exec): align discord approval auth coverage * fix(exec): ignore discord dm origin channel routes * fix(telegram): skip self-authored message echoes * fix(exec): keep implicit approval auth non-explicit
…nclaw#57838) * fix(exec): add shared approval runtime * fix(exec): harden shared approval runtime * fix(exec): guard approval expiration callbacks * fix(exec): handle approval runtime races * fix(exec): clean up failed approval deliveries * fix(exec): restore channel approval routing * fix(exec): scope telegram legacy approval fallback * refactor(exec): centralize native approval delivery * fix(exec): harden approval auth and account routing * test(exec): align telegram approval auth assertions * fix(exec): align approval rebase followups * fix(exec): clarify plugin approval not-found errors * fix(exec): fall back to session-bound telegram accounts * fix(exec): detect structured telegram approval misses * test(exec): align discord approval auth coverage * fix(exec): ignore discord dm origin channel routes * fix(telegram): skip self-authored message echoes * fix(exec): keep implicit approval auth non-explicit
…nclaw#57838) * fix(exec): add shared approval runtime * fix(exec): harden shared approval runtime * fix(exec): guard approval expiration callbacks * fix(exec): handle approval runtime races * fix(exec): clean up failed approval deliveries * fix(exec): restore channel approval routing * fix(exec): scope telegram legacy approval fallback * refactor(exec): centralize native approval delivery * fix(exec): harden approval auth and account routing * test(exec): align telegram approval auth assertions * fix(exec): align approval rebase followups * fix(exec): clarify plugin approval not-found errors * fix(exec): fall back to session-bound telegram accounts * fix(exec): detect structured telegram approval misses * test(exec): align discord approval auth coverage * fix(exec): ignore discord dm origin channel routes * fix(telegram): skip self-authored message echoes * fix(exec): keep implicit approval auth non-explicit
…nclaw#57838) * fix(exec): add shared approval runtime * fix(exec): harden shared approval runtime * fix(exec): guard approval expiration callbacks * fix(exec): handle approval runtime races * fix(exec): clean up failed approval deliveries * fix(exec): restore channel approval routing * fix(exec): scope telegram legacy approval fallback * refactor(exec): centralize native approval delivery * fix(exec): harden approval auth and account routing * test(exec): align telegram approval auth assertions * fix(exec): align approval rebase followups * fix(exec): clarify plugin approval not-found errors * fix(exec): fall back to session-bound telegram accounts * fix(exec): detect structured telegram approval misses * test(exec): align discord approval auth coverage * fix(exec): ignore discord dm origin channel routes * fix(telegram): skip self-authored message echoes * fix(exec): keep implicit approval auth non-explicit
…nclaw#57838) * fix(exec): add shared approval runtime * fix(exec): harden shared approval runtime * fix(exec): guard approval expiration callbacks * fix(exec): handle approval runtime races * fix(exec): clean up failed approval deliveries * fix(exec): restore channel approval routing * fix(exec): scope telegram legacy approval fallback * refactor(exec): centralize native approval delivery * fix(exec): harden approval auth and account routing * test(exec): align telegram approval auth assertions * fix(exec): align approval rebase followups * fix(exec): clarify plugin approval not-found errors * fix(exec): fall back to session-bound telegram accounts * fix(exec): detect structured telegram approval misses * test(exec): align discord approval auth coverage * fix(exec): ignore discord dm origin channel routes * fix(telegram): skip self-authored message echoes * fix(exec): keep implicit approval auth non-explicit
Summary
This PR replaces the old stacked approval work in #57655, #57649, #57516, and #57650 with one reviewable channel-approval change.
It is both a refactor and a bug fix:
/approve, native callbacks, plugin fallback, and account-scoped routingThe refactor is not incidental here. It is the mechanism that removes the duplicated approval paths that were causing the routing and auth drift.
Problem
Before this branch:
/approveand native callback resolution did not share the same fallback and auth rulesThat made both review and maintenance worse: the code paths diverged, and the bug fixes could not really be validated without mentally composing the whole stack.
What changed
Shared approval runtime and helpers
src/infra/exec-approval-channel-runtime.tssrc/infra/exec-approval-reply.tssrc/plugin-sdk/infra-runtime.tsNative delivery unification
src/infra/approval-native-delivery.tssrc/plugin-sdk/approval-delivery-helpers.tsextensions/discord/src/approval-native.tsandextensions/telegram/src/approval-native.tsRouting and resolution fixes
/approveresolution and callback handling across exec and plugin approvals/approveAuth and account-scoping hardening
src/infra/channel-approval-auth.tsReview follow-ups included in this PR
turnSourceAccountIdis missingAPPROVAL_NOT_FOUNDgateway envelopes before Telegram falls back toplugin.approval.resolve/approvetest registry with the unified native approval auth model after rebasing to currentmainWhy this is one PR
All of these changes sit on the same lifecycle:
request -> delivery -> reply/callback -> auth -> resolution -> final status updateSplitting them left overlapping diffs in the same files and made the later fixes depend on intermediate refactor branches. Keeping the whole channel approval path together is the cleanest review boundary.
Out of scope
Shell-side
/approvecommand rejection and wrapper parsing are not in this PR. That work is isolated in #57839.Verification
pnpm plugin-sdk:api:checkpnpm buildpnpm test -- src/auto-reply/reply/commands.test.tsSupersedes