From b5be8d93b43b2d7179deb215a18b9e004f21b65a Mon Sep 17 00:00:00 2001 From: mohamedwael201193 Date: Tue, 11 Aug 2026 06:18:05 +0300 Subject: [PATCH 1/2] test(fixtures): add execution recovery contract pack v1 Adds golden status envelopes and recovery rules for KeeperHub/cli#53. Does not implement --require-verified (see #95). --- README_EXECUTION_RECOVERY.md | 12 +++++ docs/execution-recovery-v1/contract.md | 52 +++++++++++++++++++ .../execution_recovery_v1/cold_start.json | 9 ++++ .../completed_with_tx.json | 18 +++++++ .../completed_without_tx.json | 9 ++++ testdata/execution_recovery_v1/failed.json | 10 ++++ testdata/execution_recovery_v1/malformed.json | 7 +++ testdata/execution_recovery_v1/not_found.json | 7 +++ testdata/execution_recovery_v1/queued.json | 9 ++++ .../execution_recovery_v1/rate_limited.json | 8 +++ testdata/execution_recovery_v1/reverted.json | 17 ++++++ .../execution_recovery_v1/unconfirmed.json | 17 ++++++ 12 files changed, 175 insertions(+) create mode 100644 README_EXECUTION_RECOVERY.md create mode 100644 docs/execution-recovery-v1/contract.md create mode 100644 testdata/execution_recovery_v1/cold_start.json create mode 100644 testdata/execution_recovery_v1/completed_with_tx.json create mode 100644 testdata/execution_recovery_v1/completed_without_tx.json create mode 100644 testdata/execution_recovery_v1/failed.json create mode 100644 testdata/execution_recovery_v1/malformed.json create mode 100644 testdata/execution_recovery_v1/not_found.json create mode 100644 testdata/execution_recovery_v1/queued.json create mode 100644 testdata/execution_recovery_v1/rate_limited.json create mode 100644 testdata/execution_recovery_v1/reverted.json create mode 100644 testdata/execution_recovery_v1/unconfirmed.json diff --git a/README_EXECUTION_RECOVERY.md b/README_EXECUTION_RECOVERY.md new file mode 100644 index 0000000..4c6c9fb --- /dev/null +++ b/README_EXECUTION_RECOVERY.md @@ -0,0 +1,12 @@ +# Execution Recovery Contract Pack v1 + +Fixture suite for [KeeperHub/cli#53](https://github.com/KeeperHub/cli/issues/53). + +Does **not** implement CLI `--require-verified` (see #95). Complements Option B of #53 with golden status envelopes + recovery rules. + +## Layout +- `testdata/execution_recovery_v1/*.json` — synthetic DEMO FIXTURE envelopes +- `docs/execution-recovery-v1/contract.md` — normative rules R1–R6 + +## Consumer note +A TypeScript reference consumer lives in the EMBER repo. Prefer Go tests here that load these fixtures and assert the decision table in `contract.md`. diff --git a/docs/execution-recovery-v1/contract.md b/docs/execution-recovery-v1/contract.md new file mode 100644 index 0000000..64463e2 --- /dev/null +++ b/docs/execution-recovery-v1/contract.md @@ -0,0 +1,52 @@ +# Execution recovery contract (normative) + +Version: **1.0.0** +Audience: KeeperHub CLI / MCP / HTTP adapter authors + +## Definitions + +- **Write**: any API call that may create or re-drive an onchain side effect (execute workflow, transfer, contract call). +- **Status read**: polling `get_execution` / `GET /api/execute/{id}/status` (or equivalent). +- **Chain evidence**: at least one transaction hash with a verified successful receipt for the expected predicate (token, amount, recipient, chain). +- **Idempotency key**: client-supplied key that must be byte-identical across retries of the same logical write. + +## Rules + +### R1 — Unconfirmed → poll, do not resubmit + +If status is `queued`, `running`, `pending`, or `unconfirmed`, the client MUST continue status reads against the **same** execution ID. The client MUST NOT issue a new write for the same logical intent while that execution ID remains durable. + +### R2 — No chain evidence → fail closed + +If the caller requires payment/landing proof and status is `completed` (or `success`) but no transaction hash / verified receipt is present, the client MUST treat the outcome as **not successful** for payment purposes. + +### R3 — Write retry → stable idempotency key + +If a write is retried after transport failure (timeout, 429, 5xx) before an execution ID is known, the client MUST reuse the same idempotency key. After an execution ID is known, prefer R1. + +### R4 — Terminal failure + +Statuses `failed`, `reverted`, `not_found`, and unparseable/malformed bodies are terminal for that attempt. Clients MUST NOT invent a success path from partial fields. + +### R5 — Rate limit + +HTTP `429` responses require backoff. They are not success. Preserve the idempotency key for the next write attempt of the same intent. + +### R6 — Cold start + +A first status read that returns `not_found` immediately after submit may be transient. Clients SHOULD poll briefly before concluding not_found as terminal, unless the product documents otherwise. + +## Fixture mapping + +| Fixture file | Exercises | +| --- | --- | +| `queued.json` | R1 | +| `completed_with_tx.json` | happy path with chain evidence | +| `completed_without_tx.json` | R2 | +| `failed.json` | R4 | +| `unconfirmed.json` | R1 | +| `reverted.json` | R4 | +| `not_found.json` | R4 / R6 | +| `rate_limited.json` | R5 | +| `cold_start.json` | R6 | +| `malformed.json` | R4 | diff --git a/testdata/execution_recovery_v1/cold_start.json b/testdata/execution_recovery_v1/cold_start.json new file mode 100644 index 0000000..eb2a0d0 --- /dev/null +++ b/testdata/execution_recovery_v1/cold_start.json @@ -0,0 +1,9 @@ +{ + "httpStatus": 200, + "execution": { + "id": "exec_fixture_cold_001", + "status": "queued", + "workflowId": "wf_fixture_001", + "note": "First status read immediately after submit may still be queued; treat as cold start under R6." + } +} diff --git a/testdata/execution_recovery_v1/completed_with_tx.json b/testdata/execution_recovery_v1/completed_with_tx.json new file mode 100644 index 0000000..b48c687 --- /dev/null +++ b/testdata/execution_recovery_v1/completed_with_tx.json @@ -0,0 +1,18 @@ +{ + "httpStatus": 200, + "execution": { + "id": "exec_fixture_ok_001", + "status": "completed", + "workflowId": "wf_fixture_001", + "transactionHash": "0x1111111111111111111111111111111111111111111111111111111111111111", + "receipts": [ + { + "hash": "0x1111111111111111111111111111111111111111111111111111111111111111", + "chainId": 8453, + "verified": true, + "receiptStatus": "success" + } + ], + "completedAt": "2026-08-11T00:01:00.000Z" + } +} diff --git a/testdata/execution_recovery_v1/completed_without_tx.json b/testdata/execution_recovery_v1/completed_without_tx.json new file mode 100644 index 0000000..82240a1 --- /dev/null +++ b/testdata/execution_recovery_v1/completed_without_tx.json @@ -0,0 +1,9 @@ +{ + "httpStatus": 200, + "execution": { + "id": "exec_fixture_notx_001", + "status": "completed", + "workflowId": "wf_fixture_001", + "completedAt": "2026-08-11T00:01:00.000Z" + } +} diff --git a/testdata/execution_recovery_v1/failed.json b/testdata/execution_recovery_v1/failed.json new file mode 100644 index 0000000..9a067e3 --- /dev/null +++ b/testdata/execution_recovery_v1/failed.json @@ -0,0 +1,10 @@ +{ + "httpStatus": 200, + "execution": { + "id": "exec_fixture_failed_001", + "status": "failed", + "workflowId": "wf_fixture_001", + "error": "action_failed", + "completedAt": "2026-08-11T00:01:00.000Z" + } +} diff --git a/testdata/execution_recovery_v1/malformed.json b/testdata/execution_recovery_v1/malformed.json new file mode 100644 index 0000000..efe1fb0 --- /dev/null +++ b/testdata/execution_recovery_v1/malformed.json @@ -0,0 +1,7 @@ +{ + "httpStatus": 200, + "data": { + "weird": true, + "statusCode": "DONE" + } +} diff --git a/testdata/execution_recovery_v1/not_found.json b/testdata/execution_recovery_v1/not_found.json new file mode 100644 index 0000000..b49b886 --- /dev/null +++ b/testdata/execution_recovery_v1/not_found.json @@ -0,0 +1,7 @@ +{ + "httpStatus": 404, + "error": { + "code": "not_found", + "message": "execution not found" + } +} diff --git a/testdata/execution_recovery_v1/queued.json b/testdata/execution_recovery_v1/queued.json new file mode 100644 index 0000000..f43596b --- /dev/null +++ b/testdata/execution_recovery_v1/queued.json @@ -0,0 +1,9 @@ +{ + "httpStatus": 200, + "execution": { + "id": "exec_fixture_queued_001", + "status": "queued", + "workflowId": "wf_fixture_001", + "createdAt": "2026-08-11T00:00:00.000Z" + } +} diff --git a/testdata/execution_recovery_v1/rate_limited.json b/testdata/execution_recovery_v1/rate_limited.json new file mode 100644 index 0000000..be0e28d --- /dev/null +++ b/testdata/execution_recovery_v1/rate_limited.json @@ -0,0 +1,8 @@ +{ + "httpStatus": 429, + "error": { + "code": "rate_limited", + "message": "too many requests", + "retryAfterSeconds": 2 + } +} diff --git a/testdata/execution_recovery_v1/reverted.json b/testdata/execution_recovery_v1/reverted.json new file mode 100644 index 0000000..32acd75 --- /dev/null +++ b/testdata/execution_recovery_v1/reverted.json @@ -0,0 +1,17 @@ +{ + "httpStatus": 200, + "execution": { + "id": "exec_fixture_reverted_001", + "status": "completed", + "workflowId": "wf_fixture_001", + "transactionHash": "0x3333333333333333333333333333333333333333333333333333333333333333", + "receipts": [ + { + "hash": "0x3333333333333333333333333333333333333333333333333333333333333333", + "chainId": 8453, + "verified": true, + "receiptStatus": "reverted" + } + ] + } +} diff --git a/testdata/execution_recovery_v1/unconfirmed.json b/testdata/execution_recovery_v1/unconfirmed.json new file mode 100644 index 0000000..e0e129f --- /dev/null +++ b/testdata/execution_recovery_v1/unconfirmed.json @@ -0,0 +1,17 @@ +{ + "httpStatus": 200, + "execution": { + "id": "exec_fixture_unconfirmed_001", + "status": "unconfirmed", + "workflowId": "wf_fixture_001", + "transactionHash": "0x2222222222222222222222222222222222222222222222222222222222222222", + "receipts": [ + { + "hash": "0x2222222222222222222222222222222222222222222222222222222222222222", + "chainId": 8453, + "verified": false, + "receiptStatus": "pending" + } + ] + } +} From 3505b09b514999433b4d23795d1232ef25254985 Mon Sep 17 00:00:00 2001 From: mohamedwael201193 Date: Wed, 12 Aug 2026 01:09:07 +0300 Subject: [PATCH 2/2] test: #53 execution recovery pack with loader, idempotency, cold-start Address review on PR #97: flat wire-shape fixtures consumed by table tests, fail closed on reverted receipts, stable Idempotency-Key on write retries, bounded not_found polling during --wait, docs sync for execution-recovery. --- .github/workflows/sync-cli-docs.yml | 2 +- README_EXECUTION_RECOVERY.md | 12 -- cmd/execute/contract_call.go | 9 + cmd/execute/status.go | 54 ++++-- cmd/execute/transfer.go | 60 ++++++- cmd/execute/transfer_recovery_test.go | 147 ++++++++++++++++ docs/execution-recovery-v1/contract.md | 75 +++++--- docs/execution-recovery.md | 29 ++++ docs/generate.go | 4 +- docs/kh_execute_contract-call.md | 17 +- docs/kh_execute_transfer.md | 17 +- internal/execrecovery/classify.go | 160 ++++++++++++++++++ internal/execrecovery/fixture.go | 112 ++++++++++++ internal/execrecovery/fixture_test.go | 142 ++++++++++++++++ internal/execrecovery/idempotency.go | 29 ++++ internal/execrecovery/idempotency_test.go | 38 +++++ internal/execrecovery/vocabulary.go | 27 +++ .../execution_recovery_v1/cold_start.json | 9 - .../cold_start.sequence.json | 40 +++++ .../completed_with_tx.json | 9 +- .../completed_without_tx.json | 11 +- testdata/execution_recovery_v1/failed.json | 11 +- testdata/execution_recovery_v1/malformed.json | 9 +- testdata/execution_recovery_v1/not_found.json | 12 +- testdata/execution_recovery_v1/queued.json | 10 +- .../execution_recovery_v1/rate_limited.json | 10 +- testdata/execution_recovery_v1/reverted.json | 14 +- .../execution_recovery_v1/unconfirmed.json | 20 +-- 28 files changed, 955 insertions(+), 134 deletions(-) delete mode 100644 README_EXECUTION_RECOVERY.md create mode 100644 cmd/execute/transfer_recovery_test.go create mode 100644 docs/execution-recovery.md create mode 100644 internal/execrecovery/classify.go create mode 100644 internal/execrecovery/fixture.go create mode 100644 internal/execrecovery/fixture_test.go create mode 100644 internal/execrecovery/idempotency.go create mode 100644 internal/execrecovery/idempotency_test.go create mode 100644 internal/execrecovery/vocabulary.go delete mode 100644 testdata/execution_recovery_v1/cold_start.json create mode 100644 testdata/execution_recovery_v1/cold_start.sequence.json diff --git a/.github/workflows/sync-cli-docs.yml b/.github/workflows/sync-cli-docs.yml index 3a58017..fe4442a 100644 --- a/.github/workflows/sync-cli-docs.yml +++ b/.github/workflows/sync-cli-docs.yml @@ -70,7 +70,7 @@ jobs: echo "Synced $COPIED command pages." # Sync hand-written guides (add frontmatter for Nextra) - for file in quickstart.md concepts.md; do + for file in quickstart.md concepts.md execution-recovery.md; do TITLE=$(head -1 "$CLI_DOCS/$file" | sed 's/^# //') # Build the Nextra-compatible version with frontmatter diff --git a/README_EXECUTION_RECOVERY.md b/README_EXECUTION_RECOVERY.md deleted file mode 100644 index 4c6c9fb..0000000 --- a/README_EXECUTION_RECOVERY.md +++ /dev/null @@ -1,12 +0,0 @@ -# Execution Recovery Contract Pack v1 - -Fixture suite for [KeeperHub/cli#53](https://github.com/KeeperHub/cli/issues/53). - -Does **not** implement CLI `--require-verified` (see #95). Complements Option B of #53 with golden status envelopes + recovery rules. - -## Layout -- `testdata/execution_recovery_v1/*.json` — synthetic DEMO FIXTURE envelopes -- `docs/execution-recovery-v1/contract.md` — normative rules R1–R6 - -## Consumer note -A TypeScript reference consumer lives in the EMBER repo. Prefer Go tests here that load these fixtures and assert the decision table in `contract.md`. diff --git a/cmd/execute/contract_call.go b/cmd/execute/contract_call.go index 6ea1f54..eb263d1 100644 --- a/cmd/execute/contract_call.go +++ b/cmd/execute/contract_call.go @@ -9,6 +9,7 @@ import ( "time" "github.com/jedib0t/go-pretty/v6/table" + "github.com/keeperhub/cli/internal/execrecovery" khhttp "github.com/keeperhub/cli/internal/http" "github.com/keeperhub/cli/internal/output" "github.com/keeperhub/cli/pkg/cmdutil" @@ -61,6 +62,7 @@ func NewContractCallCmd(f *cmdutil.Factory) *cobra.Command { abiFile, _ := cmd.Flags().GetString("abi-file") wait, _ := cmd.Flags().GetBool("wait") timeout, _ := cmd.Flags().GetDuration("timeout") + idemKeyFlag, _ := cmd.Flags().GetString("idempotency-key") reqBody := contractCallRequest{ ContractAddress: contract, @@ -88,11 +90,17 @@ func NewContractCallCmd(f *cmdutil.Factory) *cobra.Command { return fmt.Errorf("marshalling request: %w", err) } + idemKey, err := execrecovery.ResolveIdempotencyKey(idemKeyFlag) + if err != nil { + return err + } + req, err := client.NewRequest(http.MethodPost, khhttp.BuildBaseURL(host)+"/api/execute/contract-call", bytes.NewReader(bodyBytes)) if err != nil { return err } req.Header.Set("Content-Type", "application/json") + req.Header.Set(execrecovery.IdempotencyHeader, idemKey) resp, err := client.Do(req) if err != nil { @@ -143,6 +151,7 @@ func NewContractCallCmd(f *cmdutil.Factory) *cobra.Command { cmd.Flags().String("abi-file", "", "Path to local ABI JSON file") cmd.Flags().Bool("wait", false, "Wait for completion") cmd.Flags().Duration("timeout", 5*time.Minute, "Timeout when using --wait") + cmd.Flags().String("idempotency-key", "", "Stable Idempotency-Key for write intents (auto-generated if empty)") _ = cmd.MarkFlagRequired("chain") _ = cmd.MarkFlagRequired("contract") diff --git a/cmd/execute/status.go b/cmd/execute/status.go index b2d5a99..7be83ff 100644 --- a/cmd/execute/status.go +++ b/cmd/execute/status.go @@ -1,7 +1,9 @@ package execute import ( + "errors" "fmt" + "net/http" "time" "github.com/jedib0t/go-pretty/v6/table" @@ -14,15 +16,30 @@ import ( // ExecStatusResponse represents the execution status API response. // Shared by transfer, contract-call and status commands. type ExecStatusResponse struct { - ExecutionID string `json:"executionId"` - Status string `json:"status"` - Type string `json:"type"` - TransactionHash *string `json:"transactionHash"` - TransactionLink *string `json:"transactionLink"` - Result any `json:"result"` - Error *string `json:"error"` - CreatedAt string `json:"createdAt"` - CompletedAt *string `json:"completedAt"` + ExecutionID string `json:"executionId"` + Status string `json:"status"` + Type string `json:"type"` + TransactionHash *string `json:"transactionHash"` + TransactionLink *string `json:"transactionLink"` + Result any `json:"result"` + Error *string `json:"error"` + CreatedAt string `json:"createdAt"` + CompletedAt *string `json:"completedAt"` + Receipts []ExecReceipt `json:"receipts,omitempty"` +} + +// ExecReceipt is a chain-re-fetched proof entry attached to an execution. +// A transactionHash alone proves a transaction was submitted; a receipt with +// verified=true and receiptStatus="success" proves it landed onchain. +// receiptStatus="reverted" is Failure even when status=completed. +type ExecReceipt struct { + Hash string `json:"hash"` + ChainID int64 `json:"chainId"` + Verified bool `json:"verified"` + ReceiptStatus string `json:"receiptStatus"` + BlockNumber *int64 `json:"blockNumber,omitempty"` + GasUsed *string `json:"gasUsed,omitempty"` + VerifiedAt *string `json:"verifiedAt,omitempty"` } func NewStatusCmd(f *cmdutil.Factory) *cobra.Command { @@ -96,17 +113,17 @@ func renderExecStatus(p *output.Printer, f *cmdutil.Factory, sr *ExecStatusRespo if sr.Error != nil && *sr.Error != "" { tw.AppendRow(table.Row{"Error", *sr.Error}) } + for i, r := range sr.Receipts { + label := fmt.Sprintf("Receipt[%d]", i) + tw.AppendRow(table.Row{label, fmt.Sprintf("%s verified=%v status=%s", r.Hash, r.Verified, r.ReceiptStatus)}) + } tw.Render() }); err != nil { return err } - if sr.Status == "failed" { - msg := fmt.Sprintf("execution %s failed", sr.ExecutionID) - if sr.Error != nil && *sr.Error != "" { - msg = *sr.Error - } - return fmt.Errorf("%s", msg) + if err := execOutcomeError(sr); err != nil { + return err } return nil @@ -122,6 +139,13 @@ func watchExecStatus(f *cmdutil.Factory, client *khhttp.Client, host, executionI case <-ticker.C: sr, err := fetchExecStatus(client, host, executionID) if err != nil { + var apiErr *khhttp.APIError + if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound { + if isTTY && !p.IsJSON() { + fmt.Fprintf(f.IOStreams.Out, "\r%s not_found", executionID) + } + continue + } return err } diff --git a/cmd/execute/transfer.go b/cmd/execute/transfer.go index 04f7589..751a926 100644 --- a/cmd/execute/transfer.go +++ b/cmd/execute/transfer.go @@ -3,11 +3,14 @@ package execute import ( "bytes" "encoding/json" + "errors" "fmt" "net/http" + "strings" "time" "github.com/jedib0t/go-pretty/v6/table" + "github.com/keeperhub/cli/internal/execrecovery" khhttp "github.com/keeperhub/cli/internal/http" "github.com/keeperhub/cli/internal/output" "github.com/keeperhub/cli/pkg/cmdutil" @@ -60,6 +63,7 @@ func NewTransferCmd(f *cmdutil.Factory) *cobra.Command { tokenAddress, _ := cmd.Flags().GetString("token-address") wait, _ := cmd.Flags().GetBool("wait") timeout, _ := cmd.Flags().GetDuration("timeout") + idemKeyFlag, _ := cmd.Flags().GetString("idempotency-key") body := transferRequest{ Network: chain, @@ -78,11 +82,18 @@ func NewTransferCmd(f *cmdutil.Factory) *cobra.Command { return fmt.Errorf("marshalling request: %w", err) } + idemKey, err := execrecovery.ResolveIdempotencyKey(idemKeyFlag) + if err != nil { + return err + } + req, err := client.NewRequest(http.MethodPost, khhttp.BuildBaseURL(host)+"/api/execute/transfer", bytes.NewReader(bodyBytes)) if err != nil { return err } req.Header.Set("Content-Type", "application/json") + // Set once before Do so go-retryablehttp retries reuse the same key (R3). + req.Header.Set(execrecovery.IdempotencyHeader, idemKey) resp, err := client.Do(req) if err != nil { @@ -124,6 +135,7 @@ func NewTransferCmd(f *cmdutil.Factory) *cobra.Command { cmd.Flags().String("token-address", "", "ERC-20 token contract address") cmd.Flags().Bool("wait", false, "Wait for completion") cmd.Flags().Duration("timeout", 5*time.Minute, "Timeout when using --wait") + cmd.Flags().String("idempotency-key", "", "Stable Idempotency-Key for this write intent (auto-generated if empty)") _ = cmd.MarkFlagRequired("chain") _ = cmd.MarkFlagRequired("to") @@ -153,16 +165,28 @@ func pollExecStatus(f *cmdutil.Factory, client *khhttp.Client, host, executionID case <-ticker.C: statusResp, err := fetchExecStatus(client, host, executionID) if err != nil { + var apiErr *khhttp.APIError + // R6: tolerate cold-start 404 until the wait deadline. + if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound { + if time.Now().After(deadline) { + return fmt.Errorf("timeout after %s: execution %s not found", timeout, executionID) + } + continue + } return err } + if statusResp.Status == "not_found" { + if time.Now().After(deadline) { + return fmt.Errorf("timeout after %s: execution %s not found", timeout, executionID) + } + continue + } + if execTerminalStatuses[statusResp.Status] { - if statusResp.Status == "failed" { - msg := fmt.Sprintf("execution %s failed", executionID) - if statusResp.Error != nil { - msg = *statusResp.Error - } - return fmt.Errorf("%s", msg) + if err := execOutcomeError(statusResp); err != nil { + _ = printExecStatusResult(p, statusResp) + return err } return printExecStatusResult(p, statusResp) } @@ -192,6 +216,10 @@ func fetchExecStatus(client *khhttp.Client, host, executionID string) (*ExecStat } defer resp.Body.Close() + if resp.StatusCode == http.StatusNotFound { + return nil, khhttp.NewAPIError(resp) + } + if resp.StatusCode != http.StatusOK { return nil, khhttp.NewAPIError(resp) } @@ -213,6 +241,26 @@ func printExecStatusResult(p *output.Printer, sr *ExecStatusResponse) error { if sr.TransactionLink != nil && *sr.TransactionLink != "" { tw.AppendRow(table.Row{"TX Link", *sr.TransactionLink}) } + for i, r := range sr.Receipts { + tw.AppendRow(table.Row{fmt.Sprintf("Receipt[%d]", i), fmt.Sprintf("%s verified=%v status=%s", r.Hash, r.Verified, r.ReceiptStatus)}) + } tw.Render() }) } + +// execOutcomeError returns a non-nil error for failed / reverted terminal states. +func execOutcomeError(sr *ExecStatusResponse) error { + if sr.Status == "failed" { + msg := fmt.Sprintf("execution %s failed", sr.ExecutionID) + if sr.Error != nil && *sr.Error != "" { + msg = *sr.Error + } + return fmt.Errorf("%s", msg) + } + for _, r := range sr.Receipts { + if strings.EqualFold(r.ReceiptStatus, "reverted") { + return fmt.Errorf("execution %s completed but transaction reverted (%s)", sr.ExecutionID, r.Hash) + } + } + return nil +} diff --git a/cmd/execute/transfer_recovery_test.go b/cmd/execute/transfer_recovery_test.go new file mode 100644 index 0000000..a5abbdf --- /dev/null +++ b/cmd/execute/transfer_recovery_test.go @@ -0,0 +1,147 @@ +package execute_test + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/keeperhub/cli/cmd/execute" + "github.com/keeperhub/cli/internal/execrecovery" + "github.com/keeperhub/cli/pkg/iostreams" +) + +func TestTransferCmd_SendsIdempotencyKey(t *testing.T) { + var gotKey string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotKey = r.Header.Get(execrecovery.IdempotencyHeader) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-idem","status":"completed"}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newTransferFactory(ios, srv) + cmd := execute.NewTransferCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--to", "0xabc", "--amount", "0.1", "--idempotency-key", "stable-intent-1"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotKey != "stable-intent-1" { + t.Fatalf("Idempotency-Key=%q, want stable-intent-1", gotKey) + } +} + +func TestTransferCmd_IdempotencyKeyStableAcrossHTTPRetries(t *testing.T) { + var keys []string + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n := calls.Add(1) + keys = append(keys, r.Header.Get(execrecovery.IdempotencyHeader)) + if n == 1 { + w.WriteHeader(http.StatusBadGateway) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-retry","status":"completed"}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newTransferFactory(ios, srv) + cmd := execute.NewTransferCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--to", "0xabc", "--amount", "0.1", "--idempotency-key", "retry-stable"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if calls.Load() < 2 { + t.Fatalf("expected HTTP retry, got %d calls", calls.Load()) + } + for i, k := range keys { + if k != "retry-stable" { + t.Fatalf("call %d Idempotency-Key=%q, want retry-stable", i, k) + } + } +} + +func TestTransferCmd_WaitToleratesInitialNotFound(t *testing.T) { + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.Method == http.MethodPost { + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-cold","status":"pending"}`)) + return + } + n := calls.Add(1) + if n == 1 { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"Execution not found","code":"not_found"}`)) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"executionId":"exec-cold","status":"completed","transactionHash":"0xabc"}`)) + })) + defer srv.Close() + + ios, buf, _, _ := iostreams.Test() + f := newTransferFactory(ios, srv) + cmd := execute.NewTransferCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--to", "0xabc", "--amount", "0.1", "--wait", "--timeout", "15s"}) + + start := time.Now() + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if time.Since(start) > 14*time.Second { + t.Fatal("cold-start wait took too long") + } + out := buf.String() + if !strings.Contains(out, "exec-cold") { + t.Fatalf("expected execution in output, got %q", out) + } + if calls.Load() < 2 { + t.Fatalf("expected cold-start poll, got %d status calls", calls.Load()) + } +} + +func TestTransferCmd_WaitFailsOnRevertedReceipt(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.Method == http.MethodPost { + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-rev","status":"pending"}`)) + return + } + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]any{ + "executionId": "exec-rev", + "status": "completed", + "transactionHash": "0xrev", + "receipts": []map[string]any{ + {"hash": "0xrev", "chainId": 8453, "verified": true, "receiptStatus": "reverted"}, + }, + }) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newTransferFactory(ios, srv) + cmd := execute.NewTransferCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--to", "0xabc", "--amount", "0.1", "--wait", "--timeout", "10s"}) + + err := cmd.Execute() + if err == nil { + t.Fatal("expected reverted receipt to fail") + } + if !strings.Contains(err.Error(), "reverted") { + t.Fatalf("expected reverted error, got %v", err) + } +} diff --git a/docs/execution-recovery-v1/contract.md b/docs/execution-recovery-v1/contract.md index 64463e2..6d512c6 100644 --- a/docs/execution-recovery-v1/contract.md +++ b/docs/execution-recovery-v1/contract.md @@ -1,52 +1,71 @@ # Execution recovery contract (normative) -Version: **1.0.0** -Audience: KeeperHub CLI / MCP / HTTP adapter authors +Version: **1.1.0** +Audience: KeeperHub CLI / MCP / HTTP adapter authors +Published path: this file is synced to docs.keeperhub.com via `docs/execution-recovery.md`. ## Definitions -- **Write**: any API call that may create or re-drive an onchain side effect (execute workflow, transfer, contract call). -- **Status read**: polling `get_execution` / `GET /api/execute/{id}/status` (or equivalent). -- **Chain evidence**: at least one transaction hash with a verified successful receipt for the expected predicate (token, amount, recipient, chain). -- **Idempotency key**: client-supplied key that must be byte-identical across retries of the same logical write. +- **Write**: any API call that may create or re-drive an onchain side effect (execute transfer, contract call, workflow webhook). +- **Status read**: polling `GET /api/execute/{id}/status` (direct execution) or workflow run status (separate vocabulary). +- **Chain evidence**: at least one receipt with `verified=true` and `receiptStatus=success` for the expected predicate. +- **Idempotency key**: client-supplied `Idempotency-Key` header that must be byte-identical across retries of the same logical write. + +## Status vocabularies (do not mix) + +| Surface | Pending | Terminal | +| --- | --- | --- | +| Direct execution (`/api/execute/.../status`) | `pending`, `running`, `queued`, `unconfirmed`, transport `not_found` | `completed`, `failed` | +| Workflow run (`/api/workflows/executions/.../status`) | `pending`, `running` | `success`, `error`, `cancelled` | + +`completed` is **not** proof of onchain success. Inspect `receipts[].receiptStatus`. ## Rules ### R1 — Unconfirmed → poll, do not resubmit -If status is `queued`, `running`, `pending`, or `unconfirmed`, the client MUST continue status reads against the **same** execution ID. The client MUST NOT issue a new write for the same logical intent while that execution ID remains durable. +If status is `pending`, `running`, `queued`, or `unconfirmed`, continue status reads against the **same** execution ID. Do not issue a new write for the same logical intent while that execution ID remains durable. + +**CLI conformance:** `kh ex transfer --wait` / `kh ex cc --wait` poll the same ID. -### R2 — No chain evidence → fail closed +### R2 — Chain evidence / reverted receipts -If the caller requires payment/landing proof and status is `completed` (or `success`) but no transaction hash / verified receipt is present, the client MUST treat the outcome as **not successful** for payment purposes. +1. If any receipt has `receiptStatus=reverted`, the outcome is **Failure** even when `status=completed` and `verified=true`. +2. When the caller requires payment/landing proof (`RequireChainEvidence` / `--require-verified`), `completed` without a verified successful receipt is **Failure**. + +**CLI conformance:** wait paths fail closed on reverted receipts. Strict “no receipt ⇒ fail” is opt-in (see also PR discussion for `--require-verified`). ### R3 — Write retry → stable idempotency key -If a write is retried after transport failure (timeout, 429, 5xx) before an execution ID is known, the client MUST reuse the same idempotency key. After an execution ID is known, prefer R1. +If a write is retried after transport failure (timeout, 5xx) before an execution ID is known, the client MUST reuse the same `Idempotency-Key`. After an execution ID is known, prefer R1. + +**CLI conformance:** `kh ex transfer` and `kh ex cc` (writes) set `Idempotency-Key` once per invocation before `Do`, so HTTP-layer retries reuse it. Use `--idempotency-key` to pin a key across process restarts. -### R4 — Terminal failure +### R4 — Terminal failure / malformed -Statuses `failed`, `reverted`, `not_found`, and unparseable/malformed bodies are terminal for that attempt. Clients MUST NOT invent a success path from partial fields. +Statuses `failed` (direct) and unparseable/malformed bodies are terminal for that attempt. Missing `status` after a 200 decode is **malformed**, not success. Do not invent a success path from partial fields. ### R5 — Rate limit -HTTP `429` responses require backoff. They are not success. Preserve the idempotency key for the next write attempt of the same intent. +HTTP `429` responses require backoff. They are not success. Preserve the idempotency key for the next write attempt of the same intent. The HTTP client does not auto-retry 429. + +### R6 — Cold start `not_found` -### R6 — Cold start +A first status read that returns HTTP 404 / `not_found` immediately after submit may be transient. During `--wait`, poll briefly before treating not_found as terminal timeout. -A first status read that returns `not_found` immediately after submit may be transient. Clients SHOULD poll briefly before concluding not_found as terminal, unless the product documents otherwise. +## Fixture mapping + conformance -## Fixture mapping +| Fixture | Rule | Expect | Consumed by | +| --- | --- | --- | --- | +| `queued.json` | R1 | pending | `TestFixtures_ClassifyTable` | +| `unconfirmed.json` | R1 | pending | `TestFixtures_ClassifyTable` | +| `completed_with_tx.json` | R2 | success (strict) | `TestFixtures_ClassifyTable` | +| `completed_without_tx.json` | R2 | failure (strict) | `TestFixtures_ClassifyTable` | +| `reverted.json` | R2 | failure | `TestFixtures_ClassifyTable` + `TestRevertedIsNeverSuccess` | +| `failed.json` | R4 | failure | `TestFixtures_ClassifyTable` | +| `malformed.json` | R4 | malformed | `TestFixtures_ClassifyTable` | +| `not_found.json` | R6 | pending | `TestFixtures_ClassifyTable` | +| `rate_limited.json` | R5 | rate_limited | `TestFixtures_ClassifyTable` | +| `cold_start.sequence.json` | R6 | 404→pending→success | `TestColdStartSequence_R6` | -| Fixture file | Exercises | -| --- | --- | -| `queued.json` | R1 | -| `completed_with_tx.json` | happy path with chain evidence | -| `completed_without_tx.json` | R2 | -| `failed.json` | R4 | -| `unconfirmed.json` | R1 | -| `reverted.json` | R4 | -| `not_found.json` | R4 / R6 | -| `rate_limited.json` | R5 | -| `cold_start.json` | R6 | -| `malformed.json` | R4 | +Fixtures use the **flat** direct-execution wire shape (`executionId`, not nested `execution.id`) so `json.Unmarshal` into `DirectStatus` / `ExecStatusResponse` cannot silently zero-decode. diff --git a/docs/execution-recovery.md b/docs/execution-recovery.md new file mode 100644 index 0000000..9b3ae27 --- /dev/null +++ b/docs/execution-recovery.md @@ -0,0 +1,29 @@ +# Execution recovery + +Agents and adapters that submit KeeperHub writes must recover safely when the +network flakes, a status read races ahead of persistence, or an onchain +receipt reverts. + +This guide is the published summary of the normative contract in +[`execution-recovery-v1/contract.md`](./execution-recovery-v1/contract.md). + +## Safe first-write sequence + +1. Simulate when available (`"simulate": true`) and continue only if the call would not revert. +2. Broadcast once with a stable `Idempotency-Key` that names the **work**, not the attempt. +3. Save `executionId`. +4. Poll `GET /api/execute/{executionId}/status`. +5. Treat `receipts[]` as authoritative: `verified` + `receiptStatus=success` prove landing. `receiptStatus=reverted` is failure even if `status=completed`. + +See also the Direct Execution API docs. + +## CLI behaviour + +- `kh ex transfer` / `kh ex cc` attach `Idempotency-Key` automatically; override with `--idempotency-key`. +- `--wait` polls the same execution ID, tolerates a bounded initial `404`/`not_found`, and fails closed on reverted receipts. +- Workflow run status uses a different vocabulary (`success` / `error` / `cancelled`) — do not mix it with direct-execution statuses. + +## Fixtures + +Golden responses live under `testdata/execution_recovery_v1/` and are loaded by +`go test ./internal/execrecovery/...`. diff --git a/docs/generate.go b/docs/generate.go index 236c253..0ef8c69 100644 --- a/docs/generate.go +++ b/docs/generate.go @@ -41,8 +41,8 @@ func main() { // pruneGeneratedPages removes the generated command reference from dir. // // Only `kh*.md` is touched: the hand-written guides (quickstart.md, -// concepts.md) and the generator's own sources live alongside it and must -// survive. +// concepts.md, execution-recovery.md) and the generator's own sources live +// alongside it and must survive. func pruneGeneratedPages(dir string) error { matches, err := filepath.Glob(filepath.Join(dir, "kh*.md")) if err != nil { diff --git a/docs/kh_execute_contract-call.md b/docs/kh_execute_contract-call.md index cb117fa..40fe13d 100644 --- a/docs/kh_execute_contract-call.md +++ b/docs/kh_execute_contract-call.md @@ -19,14 +19,15 @@ kh execute contract-call [flags] ### Options ``` - --abi-file string Path to local ABI JSON file - --args string Method arguments as JSON array: '["arg1","arg2"]' - --chain string Chain ID (required) - --contract string Contract address (required) - -h, --help help for contract-call - --method string Method name (required) - --timeout duration Timeout when using --wait (default 5m0s) - --wait Wait for completion + --abi-file string Path to local ABI JSON file + --args string Method arguments as JSON array: '["arg1","arg2"]' + --chain string Chain ID (required) + --contract string Contract address (required) + -h, --help help for contract-call + --idempotency-key string Stable Idempotency-Key for write intents (auto-generated if empty) + --method string Method name (required) + --timeout duration Timeout when using --wait (default 5m0s) + --wait Wait for completion ``` ### Options inherited from parent commands diff --git a/docs/kh_execute_transfer.md b/docs/kh_execute_transfer.md index 4fda686..783f7b0 100644 --- a/docs/kh_execute_transfer.md +++ b/docs/kh_execute_transfer.md @@ -19,14 +19,15 @@ kh execute transfer [flags] ### Options ``` - --amount string Amount to transfer (required) - --chain string Chain ID (required) - -h, --help help for transfer - --timeout duration Timeout when using --wait (default 5m0s) - --to string Recipient address (required) - --token string Token symbol (default "ETH") - --token-address string ERC-20 token contract address - --wait Wait for completion + --amount string Amount to transfer (required) + --chain string Chain ID (required) + -h, --help help for transfer + --idempotency-key string Stable Idempotency-Key for this write intent (auto-generated if empty) + --timeout duration Timeout when using --wait (default 5m0s) + --to string Recipient address (required) + --token string Token symbol (default "ETH") + --token-address string ERC-20 token contract address + --wait Wait for completion ``` ### Options inherited from parent commands diff --git a/internal/execrecovery/classify.go b/internal/execrecovery/classify.go new file mode 100644 index 0000000..d61912c --- /dev/null +++ b/internal/execrecovery/classify.go @@ -0,0 +1,160 @@ +// Package execrecovery implements the execution-recovery contract (R1–R6) +// used by fixture conformance tests and by direct-execution wait paths. +package execrecovery + +import ( + "encoding/json" + "fmt" + "net/http" + "strings" +) + +// Outcome is the classified result of one status observation. +type Outcome string + +const ( + OutcomePending Outcome = "pending" + OutcomeSuccess Outcome = "success" + OutcomeFailure Outcome = "failure" + OutcomeMalformed Outcome = "malformed" + OutcomeRateLimited Outcome = "rate_limited" +) + +// Options controls classification strictness. +type Options struct { + // RequireChainEvidence enables R2 strict mode: completed without a + // verified successful receipt is Failure, not Success. + RequireChainEvidence bool +} + +// Receipt is a chain-re-fetched proof entry (direct-execution status API). +type Receipt struct { + Hash string `json:"hash"` + ChainID int64 `json:"chainId"` + Verified bool `json:"verified"` + ReceiptStatus string `json:"receiptStatus"` +} + +// DirectStatus is the flat wire shape of GET /api/execute/{id}/status. +type DirectStatus struct { + ExecutionID string `json:"executionId"` + Status string `json:"status"` + Type string `json:"type"` + TransactionHash *string `json:"transactionHash"` + TransactionLink *string `json:"transactionLink"` + Result any `json:"result"` + Error *string `json:"error"` + CreatedAt string `json:"createdAt"` + CompletedAt *string `json:"completedAt"` + Receipts []Receipt `json:"receipts,omitempty"` +} + +// Sample is one HTTP observation of an execution status endpoint. +type Sample struct { + HTTPStatus int + Body []byte +} + +// Classify maps one status observation to an Outcome. +// +// Vocabulary note: direct-execution statuses are pending|running|completed|failed +// (and transport-level not_found). Workflow run statuses (success|error|cancelled) +// belong to a different API and must not be fed here — see Vocabulary(). +func Classify(sample Sample, opts Options) (Outcome, string) { + if sample.HTTPStatus == http.StatusTooManyRequests { + return OutcomeRateLimited, "HTTP 429" + } + + // Cold-start / missing: callers may poll again (R6). Terminal failure is a + // poll-budget decision, not Classify's. + if sample.HTTPStatus == http.StatusNotFound { + return OutcomePending, "not_found" + } + + if sample.HTTPStatus != 0 && sample.HTTPStatus != http.StatusOK && sample.HTTPStatus != http.StatusAccepted { + // Non-404 errors are failures for a status read. + if sample.HTTPStatus >= 400 { + return OutcomeFailure, fmt.Sprintf("HTTP %d", sample.HTTPStatus) + } + } + + if len(sample.Body) == 0 { + return OutcomeMalformed, "empty body" + } + + trimmed := strings.TrimSpace(string(sample.Body)) + if !json.Valid([]byte(trimmed)) { + return OutcomeMalformed, "unparseable body" + } + + var st DirectStatus + if err := json.Unmarshal([]byte(trimmed), &st); err != nil { + return OutcomeMalformed, "json decode failed" + } + + status := strings.ToLower(strings.TrimSpace(st.Status)) + if status == "" { + // Unrecognised schema: valid JSON but no status field. + return OutcomeMalformed, "missing status field" + } + + switch status { + case "pending", "running", "queued", "unconfirmed": + return OutcomePending, status + case "not_found": + return OutcomePending, status + case "failed", "error", "cancelled": + return OutcomeFailure, status + case "completed", "success": + return classifyCompleted(st, opts) + default: + return OutcomeMalformed, "unrecognised status: " + status + } +} + +func classifyCompleted(st DirectStatus, opts Options) (Outcome, string) { + if hasRevertedReceipt(st.Receipts) { + return OutcomeFailure, "receiptStatus=reverted" + } + + if hasVerifiedSuccess(st.Receipts) { + return OutcomeSuccess, "verified successful receipt" + } + + if opts.RequireChainEvidence { + if st.TransactionHash == nil || strings.TrimSpace(*st.TransactionHash) == "" { + return OutcomeFailure, "completed without transaction hash" + } + if len(st.Receipts) == 0 { + return OutcomeFailure, "completed without verified successful receipt" + } + return OutcomeFailure, "no verified successful receipt" + } + + // Compatible default: completed without receipts is still Success for + // callers that have not opted into R2 strict mode (see --require-verified). + return OutcomeSuccess, "completed" +} + +func hasRevertedReceipt(receipts []Receipt) bool { + for _, r := range receipts { + if strings.EqualFold(r.ReceiptStatus, "reverted") { + return true + } + } + return false +} + +func hasVerifiedSuccess(receipts []Receipt) bool { + for _, r := range receipts { + if r.Verified && strings.EqualFold(r.ReceiptStatus, "success") { + return true + } + } + return false +} + +// HasRevertedReceipt reports whether any receipt is an onchain revert. +func HasRevertedReceipt(receipts []Receipt) bool { + return hasRevertedReceipt(receipts) +} diff --git a/internal/execrecovery/fixture.go b/internal/execrecovery/fixture.go new file mode 100644 index 0000000..6277459 --- /dev/null +++ b/internal/execrecovery/fixture.go @@ -0,0 +1,112 @@ +package execrecovery + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +// Fixture is one conformance case loaded from testdata/execution_recovery_v1. +type Fixture struct { + Name string `json:"-"` + Rule string `json:"rule"` + HTTPStatus int `json:"httpStatus"` + RequireChainEvidence bool `json:"requireChainEvidence"` + Expect Outcome `json:"expect"` + Response json.RawMessage `json:"response"` + ResponseRaw string `json:"responseRaw,omitempty"` + Note string `json:"note,omitempty"` +} + +// SequenceStep is one observation in a multi-response cold-start sequence. +type SequenceStep struct { + HTTPStatus int `json:"httpStatus"` + RequireChainEvidence bool `json:"requireChainEvidence"` + Expect Outcome `json:"expect"` + Response json.RawMessage `json:"response"` + ResponseRaw string `json:"responseRaw,omitempty"` +} + +// SequenceFixture exercises multi-poll recovery (R6). +type SequenceFixture struct { + Name string `json:"name"` + Rule string `json:"rule"` + Steps []SequenceStep `json:"steps"` +} + +// LoadFixtureDir loads every *.json fixture (not *.sequence.json) from dir. +func LoadFixtureDir(dir string) ([]Fixture, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, err + } + var out []Fixture + for _, e := range entries { + if e.IsDir() { + continue + } + name := e.Name() + if filepath.Ext(name) != ".json" { + continue + } + if len(name) >= len(".sequence.json") && name[len(name)-len(".sequence.json"):] == ".sequence.json" { + continue + } + path := filepath.Join(dir, name) + raw, err := os.ReadFile(path) + if err != nil { + return nil, err + } + var f Fixture + if err := json.Unmarshal(raw, &f); err != nil { + return nil, fmt.Errorf("%s: %w", name, err) + } + f.Name = name + out = append(out, f) + } + return out, nil +} + +// LoadSequence loads a multi-step sequence fixture. +func LoadSequence(path string) (SequenceFixture, error) { + raw, err := os.ReadFile(path) + if err != nil { + return SequenceFixture{}, err + } + var seq SequenceFixture + if err := json.Unmarshal(raw, &seq); err != nil { + return SequenceFixture{}, err + } + return seq, nil +} + +// Sample converts a fixture into a Classify input. +func (f Fixture) Sample() Sample { + body := []byte(f.ResponseRaw) + if len(body) == 0 { + body = f.Response + } + return Sample{HTTPStatus: f.HTTPStatus, Body: body} +} + +// Sample converts a sequence step into a Classify input. +func (s SequenceStep) Sample() Sample { + body := []byte(s.ResponseRaw) + if len(body) == 0 { + body = s.Response + } + return Sample{HTTPStatus: s.HTTPStatus, Body: body} +} + +// DecodeResponse unmarshals the flat DirectStatus wire body. +func (f Fixture) DecodeResponse() (DirectStatus, error) { + if f.ResponseRaw != "" { + return DirectStatus{}, fmt.Errorf("raw body is not DirectStatus JSON") + } + var st DirectStatus + if err := json.Unmarshal(f.Response, &st); err != nil { + return DirectStatus{}, err + } + return st, nil +} diff --git a/internal/execrecovery/fixture_test.go b/internal/execrecovery/fixture_test.go new file mode 100644 index 0000000..293cc96 --- /dev/null +++ b/internal/execrecovery/fixture_test.go @@ -0,0 +1,142 @@ +package execrecovery_test + +import ( + "path/filepath" + "runtime" + "testing" + + "github.com/keeperhub/cli/internal/execrecovery" +) + +func testdataDir(t *testing.T) string { + t.Helper() + _, file, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("runtime.Caller failed") + } + // internal/execrecovery -> repo root + root := filepath.Clean(filepath.Join(filepath.Dir(file), "..", "..")) + return filepath.Join(root, "testdata", "execution_recovery_v1") +} + +func TestFixtures_DecodeIntoDirectStatus(t *testing.T) { + fixtures, err := execrecovery.LoadFixtureDir(testdataDir(t)) + if err != nil { + t.Fatalf("LoadFixtureDir: %v", err) + } + if len(fixtures) == 0 { + t.Fatal("no fixtures loaded") + } + + for _, f := range fixtures { + f := f + t.Run(f.Name, func(t *testing.T) { + if f.ResponseRaw != "" { + // Malformed raw bodies are not DirectStatus JSON. + st, err := f.DecodeResponse() + if err == nil { + t.Fatalf("expected decode error for raw fixture, got %#v", st) + } + return + } + st, err := f.DecodeResponse() + if err != nil { + // not_found / rate_limited error bodies are not DirectStatus; + // Classify still handles them via HTTP status. + if f.HTTPStatus == 404 || f.HTTPStatus == 429 { + return + } + t.Fatalf("DecodeResponse: %v", err) + } + if f.HTTPStatus == 200 && f.Expect != execrecovery.OutcomeMalformed { + if st.ExecutionID == "" && f.Expect != execrecovery.OutcomeFailure { + // failed fixture has executionId; ensure we never silently zero-decode. + } + if st.Status == "" && f.Expect != execrecovery.OutcomeMalformed { + t.Fatalf("decoded empty Status for fixture %s — wire shape mismatch", f.Name) + } + } + }) + } +} + +func TestFixtures_ClassifyTable(t *testing.T) { + fixtures, err := execrecovery.LoadFixtureDir(testdataDir(t)) + if err != nil { + t.Fatalf("LoadFixtureDir: %v", err) + } + + for _, f := range fixtures { + f := f + t.Run(f.Rule+"/"+f.Name, func(t *testing.T) { + got, reason := execrecovery.Classify(f.Sample(), execrecovery.Options{ + RequireChainEvidence: f.RequireChainEvidence, + }) + if got != f.Expect { + t.Fatalf("Classify=%s (%s), want %s", got, reason, f.Expect) + } + }) + } +} + +func TestColdStartSequence_R6(t *testing.T) { + path := filepath.Join(testdataDir(t), "cold_start.sequence.json") + seq, err := execrecovery.LoadSequence(path) + if err != nil { + t.Fatalf("LoadSequence: %v", err) + } + if seq.Rule != "R6" { + t.Fatalf("rule=%s, want R6", seq.Rule) + } + if len(seq.Steps) < 2 { + t.Fatal("cold_start sequence must have at least 2 steps") + } + for i, step := range seq.Steps { + got, reason := execrecovery.Classify(step.Sample(), execrecovery.Options{ + RequireChainEvidence: step.RequireChainEvidence, + }) + if got != step.Expect { + t.Fatalf("step %d: Classify=%s (%s), want %s", i, got, reason, step.Expect) + } + } +} + +func TestRevertedIsNeverSuccess(t *testing.T) { + body := []byte(`{ + "executionId":"x", + "status":"completed", + "transactionHash":"0xabc", + "receipts":[{"hash":"0xabc","chainId":8453,"verified":true,"receiptStatus":"reverted"}] + }`) + got, reason := execrecovery.Classify(execrecovery.Sample{HTTPStatus: 200, Body: body}, execrecovery.Options{ + RequireChainEvidence: true, + }) + if got != execrecovery.OutcomeFailure { + t.Fatalf("got %s (%s), want failure", got, reason) + } +} + +func TestEmptyStatusIsMalformed(t *testing.T) { + got, _ := execrecovery.Classify(execrecovery.Sample{ + HTTPStatus: 200, + Body: []byte(`{"executionId":"x"}`), + }, execrecovery.Options{}) + if got != execrecovery.OutcomeMalformed { + t.Fatalf("got %s, want malformed", got) + } +} + +func TestVocabularySurfacesAreDistinct(t *testing.T) { + d := execrecovery.DirectExecutionVocabulary() + w := execrecovery.WorkflowRunVocabulary() + if d.Surface == w.Surface { + t.Fatal("vocabularies must name distinct surfaces") + } + for _, term := range w.Terminal { + for _, dTerm := range d.Terminal { + if term == dTerm { + t.Fatalf("shared terminal term %q across surfaces — keep vocabularies separate", term) + } + } + } +} diff --git a/internal/execrecovery/idempotency.go b/internal/execrecovery/idempotency.go new file mode 100644 index 0000000..1246fd0 --- /dev/null +++ b/internal/execrecovery/idempotency.go @@ -0,0 +1,29 @@ +package execrecovery + +import ( + "crypto/rand" + "encoding/hex" + "fmt" +) + +const IdempotencyHeader = "Idempotency-Key" + +// NewIdempotencyKey returns a random UUID-like key for a single write intent. +// Callers that retry the same intent across process restarts must persist or +// derive a stable key instead (see docs.keeperhub.com/api/direct-execution). +func NewIdempotencyKey() (string, error) { + var b [16]byte + if _, err := rand.Read(b[:]); err != nil { + return "", fmt.Errorf("generating idempotency key: %w", err) + } + // UUID v4 layout bits are not required by the API; hex is sufficient. + return hex.EncodeToString(b[:]), nil +} + +// ResolveIdempotencyKey returns explicit if non-empty, otherwise a new key. +func ResolveIdempotencyKey(explicit string) (string, error) { + if explicit != "" { + return explicit, nil + } + return NewIdempotencyKey() +} diff --git a/internal/execrecovery/idempotency_test.go b/internal/execrecovery/idempotency_test.go new file mode 100644 index 0000000..57a62dc --- /dev/null +++ b/internal/execrecovery/idempotency_test.go @@ -0,0 +1,38 @@ +package execrecovery_test + +import ( + "testing" + + "github.com/keeperhub/cli/internal/execrecovery" +) + +func TestResolveIdempotencyKey_ExplicitStable(t *testing.T) { + a, err := execrecovery.ResolveIdempotencyKey("stable-key-1") + if err != nil { + t.Fatal(err) + } + b, err := execrecovery.ResolveIdempotencyKey("stable-key-1") + if err != nil { + t.Fatal(err) + } + if a != b || a != "stable-key-1" { + t.Fatalf("explicit key must be preserved: %q vs %q", a, b) + } +} + +func TestResolveIdempotencyKey_GeneratedUnique(t *testing.T) { + a, err := execrecovery.ResolveIdempotencyKey("") + if err != nil { + t.Fatal(err) + } + b, err := execrecovery.ResolveIdempotencyKey("") + if err != nil { + t.Fatal(err) + } + if a == "" || b == "" { + t.Fatal("generated keys must be non-empty") + } + if a == b { + t.Fatal("different intents must get different generated keys") + } +} diff --git a/internal/execrecovery/vocabulary.go b/internal/execrecovery/vocabulary.go new file mode 100644 index 0000000..5fc01b1 --- /dev/null +++ b/internal/execrecovery/vocabulary.go @@ -0,0 +1,27 @@ +package execrecovery + +// Vocabulary documents which status strings belong to which API surface. +// Direct-execution and workflow-run statuses must not be mixed. +type Vocabulary struct { + Surface string + Pending []string + Terminal []string +} + +// DirectExecutionVocabulary is GET /api/execute/{id}/status. +func DirectExecutionVocabulary() Vocabulary { + return Vocabulary{ + Surface: "direct-execution", + Pending: []string{"pending", "running", "queued", "unconfirmed", "not_found"}, + Terminal: []string{"completed", "failed"}, + } +} + +// WorkflowRunVocabulary is GET /api/workflows/executions/{id}/status. +func WorkflowRunVocabulary() Vocabulary { + return Vocabulary{ + Surface: "workflow-run", + Pending: []string{"pending", "running"}, + Terminal: []string{"success", "error", "cancelled"}, + } +} diff --git a/testdata/execution_recovery_v1/cold_start.json b/testdata/execution_recovery_v1/cold_start.json deleted file mode 100644 index eb2a0d0..0000000 --- a/testdata/execution_recovery_v1/cold_start.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "httpStatus": 200, - "execution": { - "id": "exec_fixture_cold_001", - "status": "queued", - "workflowId": "wf_fixture_001", - "note": "First status read immediately after submit may still be queued; treat as cold start under R6." - } -} diff --git a/testdata/execution_recovery_v1/cold_start.sequence.json b/testdata/execution_recovery_v1/cold_start.sequence.json new file mode 100644 index 0000000..942e674 --- /dev/null +++ b/testdata/execution_recovery_v1/cold_start.sequence.json @@ -0,0 +1,40 @@ +{ + "name": "cold_start", + "rule": "R6", + "steps": [ + { + "httpStatus": 404, + "expect": "pending", + "response": { + "error": "Execution not found", + "code": "not_found" + } + }, + { + "httpStatus": 200, + "expect": "pending", + "response": { + "executionId": "exec_fixture_cold_001", + "status": "pending" + } + }, + { + "httpStatus": 200, + "requireChainEvidence": true, + "expect": "success", + "response": { + "executionId": "exec_fixture_cold_001", + "status": "completed", + "transactionHash": "0x4444444444444444444444444444444444444444444444444444444444444444", + "receipts": [ + { + "hash": "0x4444444444444444444444444444444444444444444444444444444444444444", + "chainId": 8453, + "verified": true, + "receiptStatus": "success" + } + ] + } + } + ] +} diff --git a/testdata/execution_recovery_v1/completed_with_tx.json b/testdata/execution_recovery_v1/completed_with_tx.json index b48c687..a653a97 100644 --- a/testdata/execution_recovery_v1/completed_with_tx.json +++ b/testdata/execution_recovery_v1/completed_with_tx.json @@ -1,10 +1,13 @@ { + "rule": "R2", "httpStatus": 200, - "execution": { - "id": "exec_fixture_ok_001", + "requireChainEvidence": true, + "expect": "success", + "response": { + "executionId": "exec_fixture_ok_001", "status": "completed", - "workflowId": "wf_fixture_001", "transactionHash": "0x1111111111111111111111111111111111111111111111111111111111111111", + "transactionLink": "https://basescan.org/tx/0x1111111111111111111111111111111111111111111111111111111111111111", "receipts": [ { "hash": "0x1111111111111111111111111111111111111111111111111111111111111111", diff --git a/testdata/execution_recovery_v1/completed_without_tx.json b/testdata/execution_recovery_v1/completed_without_tx.json index 82240a1..21e28db 100644 --- a/testdata/execution_recovery_v1/completed_without_tx.json +++ b/testdata/execution_recovery_v1/completed_without_tx.json @@ -1,9 +1,12 @@ { + "rule": "R2", "httpStatus": 200, - "execution": { - "id": "exec_fixture_notx_001", + "requireChainEvidence": true, + "expect": "failure", + "response": { + "executionId": "exec_fixture_no_tx_001", "status": "completed", - "workflowId": "wf_fixture_001", "completedAt": "2026-08-11T00:01:00.000Z" - } + }, + "note": "completed without chain evidence must fail closed when proof is required." } diff --git a/testdata/execution_recovery_v1/failed.json b/testdata/execution_recovery_v1/failed.json index 9a067e3..e0657ec 100644 --- a/testdata/execution_recovery_v1/failed.json +++ b/testdata/execution_recovery_v1/failed.json @@ -1,10 +1,11 @@ { + "rule": "R4", "httpStatus": 200, - "execution": { - "id": "exec_fixture_failed_001", + "requireChainEvidence": false, + "expect": "failure", + "response": { + "executionId": "exec_fixture_failed_001", "status": "failed", - "workflowId": "wf_fixture_001", - "error": "action_failed", - "completedAt": "2026-08-11T00:01:00.000Z" + "error": "simulation reverted: insufficient funds" } } diff --git a/testdata/execution_recovery_v1/malformed.json b/testdata/execution_recovery_v1/malformed.json index efe1fb0..a905de8 100644 --- a/testdata/execution_recovery_v1/malformed.json +++ b/testdata/execution_recovery_v1/malformed.json @@ -1,7 +1,8 @@ { + "rule": "R4", "httpStatus": 200, - "data": { - "weird": true, - "statusCode": "DONE" - } + "requireChainEvidence": false, + "expect": "malformed", + "responseRaw": "{{{this is not json", + "note": "Genuine unparseable body — must not be treated as success or empty-status poll forever." } diff --git a/testdata/execution_recovery_v1/not_found.json b/testdata/execution_recovery_v1/not_found.json index b49b886..3dfabc2 100644 --- a/testdata/execution_recovery_v1/not_found.json +++ b/testdata/execution_recovery_v1/not_found.json @@ -1,7 +1,11 @@ { + "rule": "R4", "httpStatus": 404, - "error": { - "code": "not_found", - "message": "execution not found" - } + "requireChainEvidence": false, + "expect": "pending", + "response": { + "error": "Execution not found", + "code": "not_found" + }, + "note": "A single not_found is Pending (R6). Exhausted poll budget is a caller-level Failure." } diff --git a/testdata/execution_recovery_v1/queued.json b/testdata/execution_recovery_v1/queued.json index f43596b..eda3e25 100644 --- a/testdata/execution_recovery_v1/queued.json +++ b/testdata/execution_recovery_v1/queued.json @@ -1,9 +1,11 @@ { + "rule": "R1", "httpStatus": 200, - "execution": { - "id": "exec_fixture_queued_001", - "status": "queued", - "workflowId": "wf_fixture_001", + "requireChainEvidence": false, + "expect": "pending", + "response": { + "executionId": "exec_fixture_queued_001", + "status": "pending", "createdAt": "2026-08-11T00:00:00.000Z" } } diff --git a/testdata/execution_recovery_v1/rate_limited.json b/testdata/execution_recovery_v1/rate_limited.json index be0e28d..41f9e32 100644 --- a/testdata/execution_recovery_v1/rate_limited.json +++ b/testdata/execution_recovery_v1/rate_limited.json @@ -1,8 +1,10 @@ { + "rule": "R5", "httpStatus": 429, - "error": { - "code": "rate_limited", - "message": "too many requests", - "retryAfterSeconds": 2 + "requireChainEvidence": false, + "expect": "rate_limited", + "response": { + "error": "Rate limit exceeded", + "code": "rate_limited" } } diff --git a/testdata/execution_recovery_v1/reverted.json b/testdata/execution_recovery_v1/reverted.json index 32acd75..0122430 100644 --- a/testdata/execution_recovery_v1/reverted.json +++ b/testdata/execution_recovery_v1/reverted.json @@ -1,9 +1,11 @@ { + "rule": "R2", "httpStatus": 200, - "execution": { - "id": "exec_fixture_reverted_001", + "requireChainEvidence": true, + "expect": "failure", + "response": { + "executionId": "exec_fixture_reverted_001", "status": "completed", - "workflowId": "wf_fixture_001", "transactionHash": "0x3333333333333333333333333333333333333333333333333333333333333333", "receipts": [ { @@ -12,6 +14,8 @@ "verified": true, "receiptStatus": "reverted" } - ] - } + ], + "completedAt": "2026-08-11T00:01:00.000Z" + }, + "note": "verified=true does not mean successful; receiptStatus=reverted is Failure." } diff --git a/testdata/execution_recovery_v1/unconfirmed.json b/testdata/execution_recovery_v1/unconfirmed.json index e0e129f..bc25883 100644 --- a/testdata/execution_recovery_v1/unconfirmed.json +++ b/testdata/execution_recovery_v1/unconfirmed.json @@ -1,17 +1,13 @@ { + "rule": "R1", "httpStatus": 200, - "execution": { - "id": "exec_fixture_unconfirmed_001", + "requireChainEvidence": false, + "expect": "pending", + "response": { + "executionId": "exec_fixture_unconfirmed_001", "status": "unconfirmed", - "workflowId": "wf_fixture_001", "transactionHash": "0x2222222222222222222222222222222222222222222222222222222222222222", - "receipts": [ - { - "hash": "0x2222222222222222222222222222222222222222222222222222222222222222", - "chainId": 8453, - "verified": false, - "receiptStatus": "pending" - } - ] - } + "createdAt": "2026-08-11T00:00:00.000Z" + }, + "note": "Broadcast but not yet confirmed — poll same executionId; do not resubmit." }