core: dedupe approval not-found handling by gumadeiras · Pull Request #60932 · openclaw/openclaw

gumadeiras · 2026-04-04T15:50:50Z

Summary

Problem: approval-not-found detection was duplicated in core /approve handling and the Telegram resolver.
Why it matters: duplicated gateway-error parsing drifts easily and makes future approval-path fixes more expensive.
What changed: moved approval-not-found classification into one shared infra helper and updated the core + Telegram callers to use it.
What did NOT change (scope boundary): No Matrix UX changes, no approval prompt rendering changes, no protocol changes.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes #
Related #
This PR fixes a bug or regression

Root Cause (if applicable)

Root cause: the same approval-not-found error decoding logic lived in multiple places instead of one shared helper.
Missing detection / guardrail: no shared utility enforced consistent handling across approval submitters.
Contributing context (if known): approval resolution spans legacy and structured gateway errors, so drift is easy when each caller hand-rolls the parser.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: existing approval submitter tests plus shared helper call sites
Scenario the test should lock in: structured and legacy unknown/expired approval errors are classified consistently across callers.
Why this is the smallest reliable guardrail: the behavior is pure error classification with thin caller wiring.
Existing test that already covers this (if any): Telegram approval resolver coverage exercises not-found fallback behavior.
If no new test is added, why not: this split keeps the already-covered shared-core extraction small and behavior-preserving.

User-visible / Behavior Changes

None

Diagram (if applicable)

N/A

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS
Runtime/container: local Node/pnpm dev env
Model/provider: N/A
Integration/channel (if any): core approval routing + Telegram approval resolver
Relevant config (redacted): N/A

Steps

Trigger an approval resolution path that receives a structured or legacy unknown/expired approval error.
Observe fallback/handling in the caller.

Expected

Callers classify the error consistently using one shared helper.

Actual

Fixed by this PR.

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

Verified scenarios: build passes; static checks pass; shared helper matches the prior caller behavior.
Edge cases checked: structured INVALID_REQUEST + APPROVAL_NOT_FOUND, direct APPROVAL_NOT_FOUND, legacy message-text fallback.
What you did not verify: full targeted Vitest reruns in this local environment beyond prior focused coverage.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No
If yes, exact upgrade steps:

Risks and Mitigations

Risk: subtle mismatch with one callers previous local implementation.
- Mitigation: the shared helper is a direct extraction of the existing logic and build/static checks passed.

greptile-apps · 2026-04-04T15:52:42Z

Greptile Summary

This PR deduplicates the isApprovalNotFoundError classification logic that previously existed independently in src/auto-reply/reply/commands-approve.ts and extensions/telegram/src/exec-approval-resolver.ts, consolidating it into a new src/infra/approval-errors.ts helper and re-exporting it from openclaw/plugin-sdk/infra-runtime. The extraction is faithful — the hardcoded string constants match the ErrorCodes enum values exactly — and call-site behavior is unchanged.

isApprovalNotFoundError is now part of the public openclaw/plugin-sdk/infra-runtime surface but docs/.generated/plugin-sdk-api-baseline.sha256 was not regenerated; pnpm plugin-sdk:api:check (enforced in workflow-sanity.yml) will fail in CI. Run pnpm plugin-sdk:api:gen and commit the updated hash file.

Confidence Score: 4/5

Safe to merge after regenerating the Plugin SDK API baseline hash; the logic change itself is correct and behavior-preserving.

One P1 finding: the public Plugin SDK surface was expanded but docs/.generated/plugin-sdk-api-baseline.sha256 was not updated, which will break the pnpm plugin-sdk:api:check CI gate. All other aspects of the refactor are correct.

src/plugin-sdk/infra-runtime.ts — needs accompanying plugin-sdk-api-baseline.sha256 regeneration

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/plugin-sdk/infra-runtime.ts
Line: 17

Comment:
**Missing Plugin SDK API baseline update**

`isApprovalNotFoundError` is now re-exported from the public `openclaw/plugin-sdk/infra-runtime` subpath, which expands the tracked Plugin SDK surface. The `docs/.generated/plugin-sdk-api-baseline.sha256` hash file was not regenerated, so `pnpm plugin-sdk:api:check` (which runs in `workflow-sanity.yml`) will fail in CI.

Per the repo guidelines, after changing the public Plugin SDK surface you must run `pnpm plugin-sdk:api:gen` and commit the updated `.sha256` file alongside the change.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "core: dedupe approval not-found handling" | Re-trigger Greptile}

Copilot

Pull request overview

This PR removes duplicated “approval not found” error classification logic by extracting it into a shared infra helper, then wiring both the core /approve handler and the Telegram approval resolver to use that helper. This reduces drift risk between approval submitters that need to interpret both structured gateway errors and legacy message-text errors.

Changes:

Added isApprovalNotFoundError() in a new shared module (src/infra/approval-errors.ts).
Updated core /approve handling to use the shared helper instead of in-file parsing.
Updated the Telegram exec approval resolver to import the shared helper via the plugin SDK infra runtime barrel.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/plugin-sdk/infra-runtime.ts	Re-exports the shared approval error helper for plugin/extension consumers.
src/infra/approval-errors.ts	Introduces centralized detection for “approval not found” across structured + legacy error shapes.
src/auto-reply/reply/commands-approve.ts	Removes duplicated parsing and uses `isApprovalNotFoundError()` for fallback routing/handling.
extensions/telegram/src/exec-approval-resolver.ts	Removes duplicated parsing and uses `isApprovalNotFoundError()` for plugin fallback decisions.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e9942a423d

ℹ️ 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".

gumadeiras · 2026-04-04T17:24:01Z

Merged via squash.

Prepared head SHA: 108221fdfe0e84c486df21effc9d81318df85674
Merge commit: e627f53d24a957b80ced740a2acfdfdfbcbe6b05

Thanks @gumadeiras!

@gumadeiras

Merged via squash. Prepared head SHA: 108221f Co-authored-by: gumadeiras <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras

@gumadeiras

Merged via squash. Prepared head SHA: 108221f Co-authored-by: gumadeiras <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras

@gumadeiras

Merged via squash. Prepared head SHA: 108221f Co-authored-by: gumadeiras <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras

@gumadeiras

Merged via squash. Prepared head SHA: 108221f Co-authored-by: gumadeiras <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras

@gumadeiras

Merged via squash. Prepared head SHA: 108221f Co-authored-by: gumadeiras <[email protected]> Co-authored-by: gumadeiras <[email protected]> Reviewed-by: @gumadeiras

Copilot AI review requested due to automatic review settings April 4, 2026 15:50

openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: S maintainer Maintainer-authored PR labels Apr 4, 2026

Copilot started reviewing on behalf of gumadeiras April 4, 2026 15:51 View session

greptile-apps Bot reviewed Apr 4, 2026

View reviewed changes

Comment thread src/plugin-sdk/infra-runtime.ts Outdated

Copilot AI reviewed Apr 4, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Apr 4, 2026

View reviewed changes

Comment thread src/plugin-sdk/infra-runtime.ts Outdated

gumadeiras self-assigned this Apr 4, 2026

openclaw-barnacle Bot added the docs Improvements or additions to documentation label Apr 4, 2026

gumadeiras force-pushed the codex/approval-error-dedupe branch 2 times, most recently from 0014b56 to 9e21f1b Compare April 4, 2026 16:33

gumadeiras added 5 commits April 4, 2026 13:22

core: dedupe approval not-found handling

32d388b

fix(approvals): narrow shared error runtime seam

7486fcc

fix(approvals): place error seam changelog entry under fixes

0fb8252

fix(approvals): move changelog entry to unreleased

b1ccfa3

docs(plugin-sdk): clarify error-runtime helper scope

108221f

gumadeiras force-pushed the codex/approval-error-dedupe branch from d29827b to 108221f Compare April 4, 2026 17:23

gumadeiras merged commit e627f53 into main Apr 4, 2026
8 checks passed

gumadeiras deleted the codex/approval-error-dedupe branch April 4, 2026 17:24

github-actions Bot mentioned this pull request Apr 4, 2026

📡 Upstream Digest — 2026-04-04 18:30 UTC curtismercier/openclaw-mods#467

Open

clawsweeper Bot mentioned this pull request Jun 14, 2026

Suppress expired exec approval followup warnings #66685

Merged

clawsweeper Bot mentioned this pull request Jul 1, 2026

fix: consume stale google chat approval clicks #98433

Closed

This was referenced Jul 1, 2026

[Bug]: Google Chat stale approval cards keep retrying after approval not found #98432

Open

fix(bug): Google Chat stale approval cards keep retrying after approv… #98457

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

core: dedupe approval not-found handling#60932

core: dedupe approval not-found handling#60932
gumadeiras merged 5 commits into
mainfrom
codex/approval-error-dedupe

gumadeiras commented Apr 4, 2026

Uh oh!

greptile-apps Bot commented Apr 4, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

gumadeiras commented Apr 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

gumadeiras commented Apr 4, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Apr 4, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

gumadeiras commented Apr 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants