feat(channels): add typed cross-surface approval actions#103679
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 11, 2026, 8:56 PM ET / July 12, 2026, 00:56 UTC. Summary Reproducibility: yes. from current-head source. A typed Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Route valid typed Telegram callbacks through the dedicated approval sequential lane, preserve the explicit-kind and legacy compatibility paths, land the reconciled registry base first, and then prove same-topic and cross-surface first-answer behavior on the exact head. Do we have a high-confidence way to reproduce the issue? Yes, from current-head source. A typed Is this the best way to solve the issue? No, not yet. The typed transport-private action model is the right maintainable boundary, but Telegram ingress must recognize the new callback envelope before this is a safe cross-surface implementation. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d05391b9b3c5. Label changesLabel changes:
Label justifications:
Evidence reviewedAcceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (5 earlier review cycles)
|
Squash-rebased #103679 segment onto the durable-approval-registry tip on current main. Typed approval/command/select presentation actions replace raw-string inference across slack/telegram/discord/matrix/imessage/whatsapp, approval.resolve carries an explicit kind, and channel adapters map native callback envelopes through the typed action registry. Drift reconciliation: deprecated buildExecApprovalInteractiveReply assertions dropped (#104650 removed the shims); worker_environments bootstrap-column migration kept alongside the approval resolution_ref backfill; plugin-sdk API baseline regenerated. (cherry picked from commit 68765a5d39d2118c88a7a54d00387337912d4494) (cherry picked from commit 8642ac12af142e4b751f4f30d4b114615e7e5f66) (cherry picked from commit 036c4bc39499925fc03de16ec9302e346769350a) (cherry picked from commit 19dc350d6bc34e29a5169c6bc80971b0ad12adde) (cherry picked from commit fc978b0bad86aef421c79f6a211b25cc1b743c01) (cherry picked from commit 10de4d1ed5071f9be6ad1ee5d1e32c0fa8c9d11c) (cherry picked from commit 9a664ced1b1fa740172b258f355f1a82925ae41c) (cherry picked from commit c5ff69abbf444139e9e007bfa45beb0f00ffea54)
d5ed331 to
d466a80
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d466a80795
ℹ️ 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 encode = (approvalId: string) => | ||
| `${TELEGRAM_APPROVAL_CALLBACK_PREFIX}${kind}:${decision}:${approvalId}`; |
There was a problem hiding this comment.
Route typed Telegram approval callbacks off the chat lane
When these new tga1: callback_data values are clicked from the same Telegram chat/topic whose agent turn is waiting for the approval, they do not match the existing approval-lane check in extensions/telegram/src/sequential-key.ts (it only treats parseExecApprovalCommandText(callbackData) as :approval). The pending turn holds the normal chat lane while awaiting the decision, so the typed button handler is queued behind the waiter and the approval can deadlock until timeout; add this reserved prefix to the approval sequential key path.
Useful? React with 👍 / 👎.
d466a80 to
e7f87ad
Compare
Squash-rebased #103679 segment onto the durable-approval-registry tip on current main. Typed approval/command/select presentation actions replace raw-string inference across slack/telegram/discord/matrix/imessage/whatsapp, approval.resolve carries an explicit kind, and channel adapters map native callback envelopes through the typed action registry. Drift reconciliation: deprecated buildExecApprovalInteractiveReply assertions dropped (#104650 removed the shims); worker_environments bootstrap-column migration kept alongside the approval resolution_ref backfill; plugin-sdk API baseline regenerated. (cherry picked from commit 68765a5d39d2118c88a7a54d00387337912d4494) (cherry picked from commit 8642ac12af142e4b751f4f30d4b114615e7e5f66) (cherry picked from commit 036c4bc39499925fc03de16ec9302e346769350a) (cherry picked from commit 19dc350d6bc34e29a5169c6bc80971b0ad12adde) (cherry picked from commit fc978b0bad86aef421c79f6a211b25cc1b743c01) (cherry picked from commit 10de4d1ed5071f9be6ad1ee5d1e32c0fa8c9d11c) (cherry picked from commit 9a664ced1b1fa740172b258f355f1a82925ae41c) (cherry picked from commit c5ff69abbf444139e9e007bfa45beb0f00ffea54) (cherry picked from commit d466a80) (cherry picked from commit f5b4fe40dd5c961322f8553cc80b2fdfb3f6503e) (cherry picked from commit 7340b4749a4cc4c72f7a41cce1bc9cb550cae038) (cherry picked from commit a151f41808f23ae60b10305ccd2bc959b9169a86)
e7f87ad to
60397dd
Compare
…-ups Follow-up delta to the merged #103579 head, rebased onto current main: - gateway-protocol wire types derive from owner-module schema consts (types.ts tombstone) and ProtocolSchemas leaves the package index so the public plugin-sdk d.ts graph tree-shakes the registry declaration - approval access authority follows the operator.approvals scope tier with reviewerDeviceIds as the opt-in restriction (cross-surface first-answer-wins; requester identity gates only legacy adapters) - plugin node.invoke approvals register directly so unrenderable presentations fail closed before request routing - exec-approval manager reconciliation with #103515 revocation hardening (resolution source attribution, one-shot ask-fallback consumption) - surface-report pins and plugin-sdk API baseline refreshed; Swift models regenerated
Squash-rebased #103679 segment onto the durable-approval-registry tip on current main. Typed approval/command/select presentation actions replace raw-string inference across slack/telegram/discord/matrix/imessage/whatsapp, approval.resolve carries an explicit kind, and channel adapters map native callback envelopes through the typed action registry. Drift reconciliation: deprecated buildExecApprovalInteractiveReply assertions dropped (#104650 removed the shims); worker_environments bootstrap-column migration kept alongside the approval resolution_ref backfill; plugin-sdk API baseline regenerated. (cherry picked from commit 68765a5d39d2118c88a7a54d00387337912d4494) (cherry picked from commit 8642ac12af142e4b751f4f30d4b114615e7e5f66) (cherry picked from commit 036c4bc39499925fc03de16ec9302e346769350a) (cherry picked from commit 19dc350d6bc34e29a5169c6bc80971b0ad12adde) (cherry picked from commit fc978b0bad86aef421c79f6a211b25cc1b743c01) (cherry picked from commit 10de4d1ed5071f9be6ad1ee5d1e32c0fa8c9d11c) (cherry picked from commit 9a664ced1b1fa740172b258f355f1a82925ae41c) (cherry picked from commit c5ff69abbf444139e9e007bfa45beb0f00ffea54) (cherry picked from commit d466a80) (cherry picked from commit f5b4fe40dd5c961322f8553cc80b2fdfb3f6503e) (cherry picked from commit 7340b4749a4cc4c72f7a41cce1bc9cb550cae038) (cherry picked from commit a151f41808f23ae60b10305ccd2bc959b9169a86)
Dependency graph guard clearedThis PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh
|
|
Land-ready verification for Improvements made during review:
Proof:
Known proof gap: Blacksmith Testbox leases were unavailable during the final local rerun, so trusted AWS Crabbox supplied the final full changed gate. The repo-native prepare gate separately accepted hosted evidence for this exact published tree. |
|
Merged via squash.
|
Related: #103505
Stacked on #103579. Review this branch from
codex/durable-approval-registry.What Problem This Solves
Approval controls in chat currently encode product semantics as slash-command strings or infer the approval owner from an ID prefix. Long IDs can exceed transport callback limits, and a click on one surface can report the attempted decision instead of the canonical result already chosen elsewhere.
Why This Change Was Made
This adds an explicit approval action to the portable presentation contract, a fixed-size durable resolution reference for constrained callbacks, and kind-aware canonical resolution across bundled approval channels. URL and Web App actions are typed at the same boundary; shipped command and legacy button inputs remain deprecated compatibility paths.
Channel plugins remain transport-only: core supplies the approval ID, owner kind, and decision; Telegram, Discord, Slack, Google Chat, Matrix, QQ, iMessage, Signal, and WhatsApp encode only private transport locators and render the canonical first-answer result. Durable cross-message terminal reconciliation is intentionally deferred to the later lifecycle stack described in
docs/refactor/operator-approvals.md.The canonical resolver responds with committed SQLite truth before detached channel, push, and legacy-event follow-up. Slow or failed secondary delivery therefore cannot delay the winning surface or hide the recorded result.
User Impact
Operators can answer the same pending approval from supported chat surfaces without approval-ID truncation or owner inference. Concurrent answers converge on the SQLite-backed first answer, and the clicked surface shows that canonical terminal outcome even when another surface won.
Evidence
approval.resolvereturns the committed CAS result before best-effort fanout completes.