Skip to content

chore: prune verified-dead core code (cli, sdk, resources, scripts)#409

Open
EfeDurmaz16 wants to merge 5 commits into
mainfrom
chore/prune-core-dead-code
Open

chore: prune verified-dead core code (cli, sdk, resources, scripts)#409
EfeDurmaz16 wants to merge 5 commits into
mainfrom
chore/prune-core-dead-code

Conversation

@EfeDurmaz16

Copy link
Copy Markdown
Owner

Four atomic, behaviour-preserving removals surfaced by the minimization audit:

  • cli: drop demo.py show_post_payment_experience + 5 degraded-state handlers (called nowhere, ~135 lines); drop api.py get_client (referenced click without importing it — would NameError; imported nowhere).
  • sdk: drop the dead unused URL allocation + void url; in engine.buildUrl.
  • resources: collapse the redundant double-assignment of params['refresh'] in treasury list_financial_accounts (async+sync).
  • scripts: drop the unreachable second branch in non_success_check_count.

Verified: py_compile + tsc --noEmit (0 errors).

demo.py: drop show_post_payment_experience and the five degraded-state
handlers (handle_wallet_creation_failure / faucet_empty / port_in_use /
rpc_down / no_network) — defined but called nowhere (~135 lines).

api.py: drop get_client(), which referenced click.ClickException without
importing click (would NameError if ever called) and is imported nowhere.
buildUrl allocated `const url = new URL(path, baseURL + '/')`, never used
it (the real URL is built into `u` via string concat), then suppressed
the unused-var warning with `void url;`. Remove both. tsc --noEmit clean.
…sury

list_financial_accounts (async + sync) initialised params["refresh"] with
a bool and overwrote it on the next line with str(refresh).lower(). Set it
once. Behaviour unchanged (final value identical).
The second `if status == COMPLETED and conclusion in {SUCCESS, SKIPPED}`
was unreachable — the preceding check already `continue`s for any
conclusion in {SUCCESS, SKIPPED}. Removed it and the now-unused `status`.
@vercel

vercel Bot commented Jun 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
sardis Ready Ready Preview, Comment Jun 14, 2026 1:08pm

Request Review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 480fd0e1b9

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


1. Create or update a budget with `sardis_set_budget` before the first spend.
2. Run `sardis_check_policy` before every `sardis_spend`.
3. Continue only when the policy result is `allow`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Recognize MCP policy approvals

The skill pack advertises that these instructions work with the Sardis MCP server (packages/sardis-openclaw/README.md lines 17-19), but this flow only permits spending when sardis_check_policy returns the literal agent-tools outcome allow. The MCP implementation serializes policy results as allowed: true/false and decision.outcome: 'APPROVED' | 'BLOCKED' (packages/sardis-mcp-server/src/tools/policy.ts lines 64 and 412), so an OpenClaw agent using the supported MCP server will fail closed even for allowed payments because step 3 never matches. Include the MCP result shape, or scope the skill to agent-tools outcomes, before requiring the spend step.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 480fd0e1b9

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


1. Create or update a budget with `sardis_set_budget` before the first spend.
2. Run `sardis_check_policy` before every `sardis_spend`.
3. Continue only when the policy result is `allow`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Recognize MCP policy approvals

The skill pack advertises that these instructions work with the Sardis MCP server (packages/sardis-openclaw/README.md lines 17-19), but this flow only permits spending when sardis_check_policy returns the literal agent-tools outcome allow. The MCP implementation serializes policy results as allowed: true/false and decision.outcome: 'APPROVED' | 'BLOCKED' (packages/sardis-mcp-server/src/tools/policy.ts lines 64 and 412), so an OpenClaw agent using the supported MCP server will fail closed even for allowed payments because step 3 never matches. Include the MCP result shape, or scope the skill to agent-tools outcomes, before requiring the spend step.

Useful? React with 👍 / 👎.

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.

1 participant