feat: make CLI execution provider-pinned#83
Conversation
Claude Review — REJECT (1 finding)Verified head Verified: wire construction is unconditionally provider-pinned ( Finding
Notes (non-blocking): |
GPT cross-stack review — REJECT
CI and CodeQL are green, and the no-auto/no-failover execution path is directionally correct. These cross-repository contract mismatches must be resolved before release. |
GPT ReviewerVerdict: REJECT Reviewed head Provider pinning and payment-retry safety are largely sound, and all shared validations pass. However, the advertised reproducible command drops invocation arguments, selected show output loses lifecycle metadata, payable quotes use the wrong provenance location, and the documentation incorrectly removes the optional meta envelope field. Findings
Shared exact-snapshot validation
|
Claude ReviewerVerdict: PASS Reviewed head PR #83 converts every quote/call to provider-pinned execution and lands cleanly. Explicit --provider performs zero catalog lookups and sends exactly one pinned /routes/{provider}/{category}/{action} request; omitted provider does one wallet-free /api/catalog/routes/ detail fetch, strictly validates defaultProvider and all candidates as concrete lowercase slugs (rejecting an 'auto' default before execution), then sends one pinned request. Route/provider/method/query-value and dot/auto-sentinel validation all precede any catalog, wallet, signing, or execution side effect (fetch-not-called tests). Payment machinery is preserved: one authorization per invocation, byte-identical pending retries on the pinned path, replacement-challenge refusal, bounded reconciliation, strict terminal nonpayment proof, and signed/unsigned 410 stopping with no provider substitution or second signature, all covered by new tests. withProviderSelection adds h402.cliProviderSelection via spread without clobbering server envelope/payment fields, leaving data provider-native; show --provider provably excludes the default candidate's schema/example/sample from the selected contract. Docs/help/SKILL are rewritten to the capability-store, pinned-path framing with concrete providers and provider-native async params, enforced by updated doc tests. Deterministic validation is fully green (diff-check, npm ci, typecheck, lint, 209 CLI + 14 core tests, build). Minor asymmetries examined (quote-402 top-level providerSelection, pinned command omitting body flags, 'alternatives' key naming in CLI-local errors) are deliberate, test-locked, and consistent with the settled #628 direction and this PR's review criteria, so no actionable findings remain. FindingsNone. Shared exact-snapshot validation
|
H-2 parent verification — GPT findings confirmedExact reviewed head: The GPT REJECT and Claude PASS artifacts disagree on four asymmetries. Parent verification against h402-web issue #628, this PR's declared contract, and the exact source/tests confirms the GPT findings:
The payment/retry and pinned-path safety work remains green; these are contract/output defects, led by the non-reproducible paid command. Original reviewer artifacts remain unchanged for audit. Final status remains needs rework because one completed reviewer rejected. |
|
Addressed all actionable rejected-review findings at
Validation: CLI 211 tests plus core 14 tests, typecheck, lint, build, package verification, clean-tarball smoke, Ready for re-review. |
GPT ReviewerVerdict: REJECT Reviewed head The flow metadata, quote provenance, optional meta documentation, provider validation, and retry behavior are corrected. However, the advertised pinned command still omits effective backend and budget settings inherited from environment/configuration, so it is neither deterministic nor payment-safety-equivalent. Findings
Shared exact-snapshot validation
|
Claude ReviewerVerdict: PASS Reviewed head Re-reviewed exact head 4a5dd41 against base ce71596. All four mandated findings are fixed and regression-locked. (1) pinnedCommand is now built from the validated invocation: it preserves --api-url, --json, --query, --method, --name, --wallet, --no-passphrase, --no-credit, and --max-usd with correct POSIX single-quote escaping (verified against an embedded apostrophe in a POST body), while deliberately excluding passphrases and the consumed idempotency key — exclusions the docs state explicitly and tests assert (no "never-print-this", no "idem-1"). (2) Selected show output now includes route-owned flow and flowFollowUps, asserted to survive provider filtering; the allowlist matches issue #628 decision 7's route-owned field set exactly while provably excluding contract-shaped candidate fields. (3) Provenance is uniform at h402.cliProviderSelection across payable quote ({paymentRequired, h402:{...}}), free quote, and free/paid call successes; post-resolution failures on call and quote carry the same metadata at error.detail.h402.cliProviderSelection (signed/unsigned 410, max-usd refusal, missing wallet, upstream 500 all tested), which also resolves the earlier "selection invisible on failure" finding. (4) All three docs restore the authoritative { data, meta?, h402 } envelope with meta as reserved envelope metadata; docs tests now require that notation, and withProviderSelection preserves meta and server h402 fields via spread. Money-safety invariants re-verified: one signature per invocation, byte-identical bounded settlement retries on the same pinned path and idempotency key (including the catalog-resolved provider via the realigned retry fixture), replacement-challenge refusal, hard stop on 410 with error.detail.error.candidates and no provider substitution or re-sign, and route/provider/method/query/cap validation before any catalog, network, wallet, or signing work (fetch-not-called assertions). Missing/auto/non-slug/disabled/unlisted catalog defaults are rejected fail-closed as invalid_catalog_response after exactly one wallet-free detail lookup with no retry or failover. CLI-local provider_unavailable errors and all fixtures use the server's nested {error:{routeId,requestedProvider,defaultProvider,candidates:[{provider,candidateKey,status,path}]}} shape; unknown routes pass through error.detail.error.recovery.command verbatim with no suggestion engine. Zero auto references remain outside the tombstone rejection. Shared validation is green (diff-check, npm ci, typecheck, lint, 211 CLI + 14 core tests, build). Asymmetries examined and judged non-actionable: show --provider prints top-level providerSelection on a non-execution catalog response (outside the quote/call provenance invariant, test-locked); the disabled-default error carries candidates additionally under detail.detail as bonus info while every documented recovery location holds; env/config-derived settings are not echoed into the flag-scoped recipe, matching the documented contract and reproducing correctly in-environment. FindingsNone. Shared exact-snapshot validation
|
H-2 parent verification — GPT finding confirmedExact reviewed head: The four findings from the prior review are fixed, but the new GPT major finding is reproducible on this head:
Therefore a command copied from one effective invocation can target a different backend or lose its payment ceiling in another environment. Claude's PASS artifact and its label remain preserved as the independent reviewer verdict; the mixed verdict is resolved operationally by |
Summary
--providercalls the concrete path directly; omission performs one wallet-free detail lookup, resolvesdefaultProvider, and executes one pinned request.h402 show <category/action> [--provider <name>]for full provider-native contracts while keepingsearchcompact.404/410/provider recovery details and never silently retry or select another provider.dataand add CLI provenance underh402.cliProviderSelection.Dependency
Depends on Steemhunt/h402-web#632, and transitively #631. Part of Steemhunt/h402-web#628.
Behavior
h402 search "web search"returns compact task/provider summaries.h402 show web/searchreturns every enabled provider contract.h402 show web/search --provider stableenrich-exareturns only that provider contract plus task identity/default metadata.h402 search "<route>"recovery without adding a suggestion engine.Cross-repo smoke
Exercised against PR #632:
show web/searchreturned provider-native contractsshowdid not mix another provider's schema/default fieldsVerification
npm run typechecknpm run lintnpm test— CLI 211 tests, core 14 testsnpm run buildnpm run verify:packnpm run smoke:packfrom a clean tarball installgit diff --checkLarge diff warning
This PR changes 17 files: +1,011 / -138 (net +873).
Most of the increase is the provider-selection regression suite (including the 408-line focused test file), followed by the catalog/show and pinned-execution implementation.