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/cmd/execute/contract_call.go b/cmd/execute/contract_call.go index 454da08..3c8714c 100644 --- a/cmd/execute/contract_call.go +++ b/cmd/execute/contract_call.go @@ -1,7 +1,6 @@ package execute import ( - "bytes" "encoding/json" "fmt" "net/http" @@ -9,6 +8,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 +61,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,13 +89,13 @@ func NewContractCallCmd(f *cmdutil.Factory) *cobra.Command { return fmt.Errorf("marshalling request: %w", err) } - req, err := client.NewRequest(http.MethodPost, khhttp.BuildBaseURL(host)+"/api/execute/contract-call", bytes.NewReader(bodyBytes)) + idemKey, err := execrecovery.ResolveIdempotencyKey(idemKeyFlag) if err != nil { return err } - req.Header.Set("Content-Type", "application/json") - resp, err := client.Do(req) + deadline := time.Now().Add(timeout) + resp, err := postIdempotentJSON(client, khhttp.BuildBaseURL(host)+"/api/execute/contract-call", bodyBytes, idemKey, deadline) if err != nil { return err } @@ -152,6 +153,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/contract_call_test.go b/cmd/execute/contract_call_test.go index b946885..539af77 100644 --- a/cmd/execute/contract_call_test.go +++ b/cmd/execute/contract_call_test.go @@ -6,11 +6,13 @@ import ( "net/http/httptest" "os" "strings" + "sync/atomic" "testing" "time" "github.com/keeperhub/cli/cmd/execute" "github.com/keeperhub/cli/internal/config" + "github.com/keeperhub/cli/internal/execrecovery" khhttp "github.com/keeperhub/cli/internal/http" "github.com/keeperhub/cli/pkg/cmdutil" "github.com/keeperhub/cli/pkg/iostreams" @@ -20,7 +22,7 @@ func newContractCallFactory(ios *iostreams.IOStreams, srv *httptest.Server) *cmd client := khhttp.NewClient(khhttp.ClientOptions{ Host: srv.URL, AppVersion: "test", - IOStreams: ios, + IOStreams: ios, }) return &cmdutil.Factory{ IOStreams: ios, @@ -304,6 +306,159 @@ func TestContractCallCmd_WaitWritePolls(t *testing.T) { } } +func TestContractCallCmd_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-cc-idem","status":"completed"}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newContractCallFactory(ios, srv) + cmd := execute.NewContractCallCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--contract", "0xcontract", "--method", "transfer", "--idempotency-key", "stable-cc-1"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotKey != "stable-cc-1" { + t.Fatalf("Idempotency-Key=%q, want stable-cc-1", gotKey) + } +} + +func TestContractCallCmd_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-cc-retry","status":"completed"}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newContractCallFactory(ios, srv) + cmd := execute.NewContractCallCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--contract", "0xcontract", "--method", "transfer", "--idempotency-key", "retry-stable-cc"}) + + 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-cc" { + t.Fatalf("call %d Idempotency-Key=%q, want retry-stable-cc", i, k) + } + } +} + +func TestContractCallCmd_IdempotencyKeyStableAcross504(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.StatusGatewayTimeout) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-cc-504","status":"completed"}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newContractCallFactory(ios, srv) + cmd := execute.NewContractCallCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--contract", "0xcontract", "--method", "transfer", "--idempotency-key", "cc-504"}) + + 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 != "cc-504" { + t.Fatalf("call %d key=%q", i, k) + } + } +} + +func TestContractCallCmd_IdempotencyInProgressRetriesSameKey(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)) + w.Header().Set("Content-Type", "application/json") + if n == 1 { + w.WriteHeader(http.StatusConflict) + _, _ = w.Write([]byte(`{"error":"in flight","code":"idempotency_in_progress","retryable":true}`)) + return + } + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-cc-inprog","status":"completed"}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newContractCallFactory(ios, srv) + cmd := execute.NewContractCallCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--contract", "0xcontract", "--method", "transfer", "--idempotency-key", "cc-inprog", "--timeout", "10s"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if calls.Load() != 2 { + t.Fatalf("got %d POSTs, want 2", calls.Load()) + } + for i, k := range keys { + if k != "cc-inprog" { + t.Fatalf("call %d key=%q", i, k) + } + } +} + +func TestContractCallCmd_IdempotencyConflictFails(t *testing.T) { + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusConflict) + _, _ = w.Write([]byte(`{"error":"Idempotency-Key was reused with a different request payload.","code":"idempotency_conflict","retryable":false}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newContractCallFactory(ios, srv) + cmd := execute.NewContractCallCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--contract", "0xcontract", "--method", "transfer", "--idempotency-key", "cc-conflict"}) + + err := cmd.Execute() + if err == nil { + t.Fatal("expected conflict") + } + if !strings.Contains(err.Error(), "do not retry with a new key") { + t.Fatalf("got %v", err) + } + if calls.Load() != 1 { + t.Fatalf("got %d POSTs, want 1", calls.Load()) + } +} + func TestContractCallCmd_WaitFailsWhenWriteResponseAlreadyFailed(t *testing.T) { pollCount := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/execute/idempotent_write.go b/cmd/execute/idempotent_write.go new file mode 100644 index 0000000..5569ce6 --- /dev/null +++ b/cmd/execute/idempotent_write.go @@ -0,0 +1,76 @@ +package execute + +import ( + "bytes" + "fmt" + "io" + "net/http" + "time" + + "github.com/keeperhub/cli/internal/execrecovery" + khhttp "github.com/keeperhub/cli/internal/http" +) + +const inProgressBackoff = 200 * time.Millisecond +const inProgressBackoffMax = 2 * time.Second + +// postIdempotentJSON POSTs body with a stable Idempotency-Key. +// +// HTTP 5xx retries are handled by the retryable client (same key). +// HTTP 409 is classified by body code from lib/idempotency.ts: +// +// idempotency_in_progress -> retry the same key until deadline +// idempotency_conflict -> fail; never mint a new key +func postIdempotentJSON(client *khhttp.Client, url string, body []byte, idemKey string, deadline time.Time) (*http.Response, error) { + backoff := inProgressBackoff + for { + req, err := client.NewRequest(http.MethodPost, url, bytes.NewReader(body)) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set(execrecovery.IdempotencyHeader, idemKey) + + resp, err := client.Do(req) + if err != nil { + return nil, err + } + + if resp.StatusCode != http.StatusConflict { + return resp, nil + } + + raw, readErr := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if readErr != nil { + return nil, fmt.Errorf("reading 409 body: %w", readErr) + } + + info, ok := execrecovery.ParseIdempotencyBody(raw) + if ok && info.IsInProgress() { + if !deadline.IsZero() && time.Now().After(deadline) { + return nil, execrecovery.InProgressTimeoutError{Key: idemKey} + } + time.Sleep(backoff) + if backoff < inProgressBackoffMax { + backoff *= 2 + if backoff > inProgressBackoffMax { + backoff = inProgressBackoffMax + } + } + continue + } + if ok && info.IsConflict() { + return nil, execrecovery.ConflictError{Body: info, Key: idemKey} + } + + msg := string(raw) + if info.Error != "" { + msg = info.Error + } + if msg == "" { + msg = http.StatusText(http.StatusConflict) + } + return nil, &khhttp.APIError{StatusCode: http.StatusConflict, Body: raw, Message: msg} + } +} diff --git a/cmd/execute/status.go b/cmd/execute/status.go index c9ef914..6b50e07 100644 --- a/cmd/execute/status.go +++ b/cmd/execute/status.go @@ -8,6 +8,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" @@ -44,33 +45,12 @@ func nextPollDelay(resp *http.Response) (time.Duration, bool) { return time.Duration(min(secs, maxPollIntervalSecs)) * time.Second, false } -// 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"` - Receipts []ExecReceipt `json:"receipts"` -} +// ExecStatusResponse is the GET /api/execute/{id}/status wire type. +// Canonical definition: execrecovery.DirectStatus. +type ExecStatusResponse = execrecovery.DirectStatus -// 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. -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"` -} +// ExecReceipt is DirectExecutionReceiptEntry on the wire. +type ExecReceipt = execrecovery.Receipt func NewStatusCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ @@ -182,12 +162,8 @@ func renderExecStatus(p *output.Printer, f *cmdutil.Factory, sr *ExecStatusRespo printUnconfirmedNotice(f, sr.ExecutionID, sr.TransactionHash) } - 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 @@ -236,6 +212,8 @@ func watchExecStatus(f *cmdutil.Factory, client *khhttp.Client, host, executionI for { sr, delay, serverSaysTerminal, err := fetchExecStatus(client, host, executionID) if err != nil { + // HTTP 404 is terminal for --watch (mistyped id / other org). + // Cold-start 404 tolerance lives only in pollExecStatus (--wait). return err } diff --git a/cmd/execute/status_test.go b/cmd/execute/status_test.go index ea3487e..7c06657 100644 --- a/cmd/execute/status_test.go +++ b/cmd/execute/status_test.go @@ -19,7 +19,7 @@ func newStatusFactory(ios *iostreams.IOStreams, srv *httptest.Server) *cmdutil.F client := khhttp.NewClient(khhttp.ClientOptions{ Host: srv.URL, AppVersion: "test", - IOStreams: ios, + IOStreams: ios, }) return &cmdutil.Factory{ IOStreams: ios, @@ -205,3 +205,108 @@ func TestExecStatusCmd_Watch_PollsUntilTerminal(t *testing.T) { t.Errorf("expected tx hash in final output, got: %q", out) } } + +func TestExecStatusCmd_Watch_404Fails(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"Execution not found"}`)) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newStatusFactory(ios, srv) + cmd := execute.NewStatusCmd(f) + cmd.SetArgs([]string{"missing-id", "--watch"}) + + done := make(chan error, 1) + go func() { + done <- cmd.Execute() + }() + + select { + case err := <-done: + if err == nil { + t.Fatal("expected 404 to terminate --watch") + } + if !strings.Contains(err.Error(), "404") && !strings.Contains(err.Error(), "not found") { + t.Fatalf("got %v", err) + } + case <-time.After(8 * time.Second): + t.Fatal("--watch spun on 404 instead of failing") + } +} + +func TestExecStatusCmd_Watch_JSON404Fails(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"Execution not found"}`)) + })) + defer srv.Close() + + ios, buf, _, _ := iostreams.Test() + f := newStatusFactory(ios, srv) + cmd := execute.NewStatusCmd(f) + cmd.Flags().Bool("json", false, "Output as JSON") + cmd.SetArgs([]string{"foreign-org-id", "--watch", "--json"}) + + done := make(chan error, 1) + go func() { + done <- cmd.Execute() + }() + + select { + case err := <-done: + if err == nil { + t.Fatal("expected 404 to terminate --watch --json") + } + if strings.TrimSpace(buf.String()) != "" && !strings.Contains(err.Error(), "404") && !strings.Contains(err.Error(), "not found") { + t.Fatalf("err=%v out=%q", err, buf.String()) + } + case <-time.After(8 * time.Second): + t.Fatal("--watch --json spun on 404") + } +} + +func TestExecStatusCmd_Watch_FailedStatus(t *testing.T) { + callCount := 0 + errMsg := "reverted on-chain" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + resp := execute.ExecStatusResponse{ + ExecutionID: "exec-fail-watch", + Status: "failed", + Error: &errMsg, + } + _ = json.NewEncoder(w).Encode(resp) + })) + defer srv.Close() + + ios, _, _, _ := iostreams.Test() + f := newStatusFactory(ios, srv) + cmd := execute.NewStatusCmd(f) + cmd.SetArgs([]string{"exec-fail-watch", "--watch"}) + + done := make(chan error, 1) + go func() { + done <- cmd.Execute() + }() + + select { + case err := <-done: + if err == nil { + t.Fatal("expected failed status to return error") + } + if !strings.Contains(err.Error(), "reverted on-chain") { + t.Fatalf("got %v", err) + } + case <-time.After(8 * time.Second): + t.Fatal("timed out") + } + if callCount < 1 { + t.Fatal("expected at least one poll") + } +} diff --git a/cmd/execute/status_verified_test.go b/cmd/execute/status_verified_test.go index 8778e84..9b6921a 100644 --- a/cmd/execute/status_verified_test.go +++ b/cmd/execute/status_verified_test.go @@ -23,10 +23,12 @@ func serveStatus(t *testing.T, resp execute.ExecStatusResponse) *httptest.Server })) } +func ptrInt(v int) *int { return &v } + func verifiedReceipt(hash string) execute.ExecReceipt { return execute.ExecReceipt{ Hash: hash, - ChainID: 84532, + ChainID: ptrInt(84532), Verified: true, ReceiptStatus: "success", } @@ -89,7 +91,7 @@ func TestExecStatusCmd_RequireVerified_FailsWhenReceiptUnverified(t *testing.T) Status: "completed", Receipts: []execute.ExecReceipt{{ Hash: "0xdead", - ChainID: 84532, + ChainID: ptrInt(84532), Verified: false, ReceiptStatus: "success", }}, @@ -119,7 +121,7 @@ func TestExecStatusCmd_RequireVerified_FailsWhenReceiptNotSuccess(t *testing.T) Status: "completed", Receipts: []execute.ExecReceipt{{ Hash: "0xbeef", - ChainID: 84532, + ChainID: ptrInt(84532), Verified: true, ReceiptStatus: rs, }}, @@ -364,7 +366,7 @@ func TestExecStatusCmd_ReceiptWithoutStatus_RendersUnknown(t *testing.T) { Status: "completed", Receipts: []execute.ExecReceipt{{ Hash: "0xabc", - ChainID: 84532, + ChainID: ptrInt(84532), Verified: true, }}, }) diff --git a/cmd/execute/transfer.go b/cmd/execute/transfer.go index 5768f92..0f1d7aa 100644 --- a/cmd/execute/transfer.go +++ b/cmd/execute/transfer.go @@ -1,13 +1,14 @@ package execute import ( - "bytes" "encoding/json" + "errors" "fmt" "net/http" "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" @@ -27,16 +28,16 @@ type transferResponse struct { TransactionHash *string `json:"transactionHash,omitempty"` } -// execStatusUnconfirmed is terminal: the transaction was broadcast but its -// receipt could not be read within the API's lookup budget. The server-side -// reconciler keeps watching it, so the execution can be re-checked later. +// execStatusUnconfirmed is terminal for the CLI wait/watch loops. +// +// It is not terminal on the server: a reconciliation sweep still settles that +// row to completed or failed once the chain answers. The CLI stops on it +// anyway and reports it, rather than polling to a non-zero timeout, because a +// non-zero exit invites a re-run that broadcasts a second transaction for an +// intent that may already be on chain. Read the settled status later with +// `kh ex st `. const execStatusUnconfirmed = "unconfirmed" -// execTerminalStatuses are the statuses a poll loop stops on. -// -// unconfirmed is terminal for a client: nothing moves it until the reconciler runs on its own -// schedule, so polling past it only burns requests. --wait and --watch stop there and exit zero, -// because a non-zero exit invites a retry, and retrying a broadcast can double-spend. var execTerminalStatuses = map[string]bool{ "completed": true, "failed": true, @@ -59,14 +60,8 @@ func printUnconfirmedNotice(f *cmdutil.Factory, executionID string, txHash *stri executionID, hash, executionID) } -// terminalExecError reports a terminal status that did not succeed. -// -// Two paths reach a terminal status: the write response can already carry one, -// and pollExecStatus reads one from the status endpoint. Both must classify it -// the same way. They did not, so a write that failed fast exited zero while the -// identical failure discovered one poll later exited non-zero. -// -// apiErr is nil on the write path, whose response carries no error detail. +// terminalExecError reports a terminal status that did not succeed on the +// write-response path, which carries no receipt or error detail. func terminalExecError(executionID, status string, apiErr *string) error { if status != "failed" { return nil @@ -105,6 +100,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, @@ -123,13 +119,13 @@ func NewTransferCmd(f *cmdutil.Factory) *cobra.Command { return fmt.Errorf("marshalling request: %w", err) } - req, err := client.NewRequest(http.MethodPost, khhttp.BuildBaseURL(host)+"/api/execute/transfer", bytes.NewReader(bodyBytes)) + idemKey, err := execrecovery.ResolveIdempotencyKey(idemKeyFlag) if err != nil { return err } - req.Header.Set("Content-Type", "application/json") - resp, err := client.Do(req) + deadline := time.Now().Add(timeout) + resp, err := postIdempotentJSON(client, khhttp.BuildBaseURL(host)+"/api/execute/transfer", bodyBytes, idemKey, deadline) if err != nil { return err } @@ -178,6 +174,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") @@ -203,11 +200,27 @@ func pollExecStatus(f *cmdutil.Factory, client *khhttp.Client, host, executionID for { statusResp, delay, serverSaysTerminal, 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) + } + delay = defaultPollInterval + if remaining := time.Until(deadline); delay > remaining { + delay = remaining + } + if delay > 0 { + time.Sleep(delay) + } + continue + } return err } if execTerminalStatuses[statusResp.Status] || serverSaysTerminal { - if err := terminalExecError(executionID, statusResp.Status, statusResp.Error); err != nil { + if err := execOutcomeError(statusResp); err != nil { + _ = printExecStatusResult(p, statusResp) return err } if err := printExecStatusResult(p, statusResp); err != nil { @@ -290,6 +303,34 @@ func printExecStatusResult(p *output.Printer, sr *ExecStatusResponse) error { if completedWithoutTransaction(sr.Status, sr.TransactionHash) { tw.AppendRow(table.Row{"Transaction", noTransactionNote}) } + 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 terminal states and for +// any receipt that is not explicitly successful when the run has completed, or +// for a conclusive on-chain failure (reverted / safe_inner_failure) at any +// status. not_found / timeout receipts leave `unconfirmed` a zero-exit +// outcome: the server treats those as unread, not failed, so erroring here +// would invite the re-run that double-broadcasts. +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 execrecovery.ConclusiveFailedReceipt(r.ReceiptStatus) { + return fmt.Errorf("execution %s receipt %s status=%s", sr.ExecutionID, r.Hash, r.ReceiptStatus) + } + if sr.Status == "completed" && execrecovery.NonSuccessReceipt(r) { + return fmt.Errorf("execution %s completed with non-success receipt %s status=%s", sr.ExecutionID, r.Hash, r.ReceiptStatus) + } + } + return nil +} diff --git a/cmd/execute/transfer_recovery_test.go b/cmd/execute/transfer_recovery_test.go new file mode 100644 index 0000000..b578d3a --- /dev/null +++ b/cmd/execute/transfer_recovery_test.go @@ -0,0 +1,386 @@ +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"}`)) + 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) + } +} + +func TestTransferCmd_IdempotencyKeyStableAcross504(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.StatusGatewayTimeout) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-504","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-504"}) + + 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-504" { + t.Fatalf("call %d Idempotency-Key=%q, want retry-504", i, k) + } + } +} + +func TestTransferCmd_IdempotencyInProgressRetriesSameKey(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)) + w.Header().Set("Content-Type", "application/json") + if n < 3 { + w.WriteHeader(http.StatusConflict) + _, _ = w.Write([]byte(`{"error":"A request with this Idempotency-Key is already being processed. Retry the same key shortly; do not rotate it.","code":"idempotency_in_progress","retryable":true}`)) + return + } + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-inprog","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", "in-progress-key", "--timeout", "10s"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if calls.Load() != 3 { + t.Fatalf("got %d POSTs, want 3", calls.Load()) + } + for i, k := range keys { + if k != "in-progress-key" { + t.Fatalf("call %d minted a new key %q", i, k) + } + } +} + +func TestTransferCmd_504ThenInProgressReusesKey(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)) + w.Header().Set("Content-Type", "application/json") + switch n { + case 1: + w.WriteHeader(http.StatusGatewayTimeout) + case 2: + w.WriteHeader(http.StatusConflict) + _, _ = w.Write([]byte(`{"error":"in flight","code":"idempotency_in_progress","retryable":true}`)) + default: + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"executionId":"exec-combo","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", "combo-key", "--timeout", "15s"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if calls.Load() < 3 { + t.Fatalf("got %d calls, want at least 3 (504 retry + in_progress + 202)", calls.Load()) + } + for i, k := range keys { + if k != "combo-key" { + t.Fatalf("call %d key=%q", i, k) + } + } +} + +func TestTransferCmd_IdempotencyConflictFailsWithoutNewKey(t *testing.T) { + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusConflict) + _, _ = w.Write([]byte(`{"error":"Idempotency-Key was reused with a different request payload. Use a new key for a different request.","code":"idempotency_conflict","originalExecutionId":"exec-orig","retryable":false}`)) + })) + 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", "conflict-key"}) + + err := cmd.Execute() + if err == nil { + t.Fatal("expected conflict error") + } + if !strings.Contains(err.Error(), "idempotency") && !strings.Contains(err.Error(), "different request payload") { + t.Fatalf("got %v", err) + } + if !strings.Contains(err.Error(), "do not retry with a new key") { + t.Fatalf("conflict must tell the user not to mint a new key: %v", err) + } + if calls.Load() != 1 { + t.Fatalf("conflict must not retry the POST, got %d", calls.Load()) + } +} + +func TestTransferCmd_WaitPersistent404TimesOut(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-missing","status":"pending"}`)) + return + } + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"Execution not found"}`)) + })) + 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", "3s"}) + + err := cmd.Execute() + if err == nil { + t.Fatal("expected timeout") + } + if !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "timeout") { + t.Fatalf("got %v", err) + } +} + +// An unreadable receipt must not become a non-zero exit: that is what makes a +// caller re-run and broadcast a second transaction for an intent that may +// already be on chain. +func TestTransferCmd_WaitStopsOnUnconfirmedAndExitsZero(t *testing.T) { + var statusReads 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-unconf","status":"pending"}`)) + return + } + atomic.AddInt32(&statusReads, 1) + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]any{ + "executionId": "exec-unconf", + "status": "unconfirmed", + "transactionHash": "0xunconf", + "receipts": []map[string]any{ + {"hash": "0xunconf", "verified": false, "receiptStatus": "not_found", "verifiedAt": "2026-08-11T00:00:00Z"}, + }, + }) + })) + defer srv.Close() + + ios, out, _, _ := iostreams.Test() + f := newTransferFactory(ios, srv) + cmd := execute.NewTransferCmd(f) + cmd.SetArgs([]string{"--chain", "1", "--to", "0xabc", "--amount", "0.1", "--wait", "--timeout", "10s"}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unconfirmed must exit zero, got %v", err) + } + if got := atomic.LoadInt32(&statusReads); got != 1 { + t.Fatalf("status reads=%d, want 1 (unconfirmed must not be polled through)", got) + } + if s := out.String(); !strings.Contains(s, "unconfirmed") || !strings.Contains(s, "0xunconf") { + t.Fatalf("expected status and hash in output, got %q", s) + } +} + +func TestTransferCmd_WaitFailsOnSafeInnerFailure(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-safe","status":"pending"}`)) + return + } + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]any{ + "executionId": "exec-safe", + "status": "completed", + "transactionHash": "0xsafe", + "receipts": []map[string]any{ + {"hash": "0xsafe", "verified": false, "receiptStatus": "safe_inner_failure", "verifiedAt": "2026-08-11T00:00:00Z"}, + }, + }) + })) + 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 safe_inner_failure to fail") + } + if !strings.Contains(err.Error(), "safe_inner_failure") { + t.Fatalf("got %v", err) + } +} diff --git a/docs/execution-recovery-v1/contract.md b/docs/execution-recovery-v1/contract.md new file mode 100644 index 0000000..78c4c95 --- /dev/null +++ b/docs/execution-recovery-v1/contract.md @@ -0,0 +1,111 @@ +# Execution recovery contract (normative) + +Version: **1.4.0** +Audience: KeeperHub CLI / MCP / HTTP adapter authors +Published path: this file is synced to docs.keeperhub.com via `docs/execution-recovery.md`. + +This contract covers **direct execution**: + +- write: `POST /api/execute/transfer`, `POST /api/execute/contract-call` +- status: `GET /api/execute/{id}/status` + +It does **not** cover `POST /api/workflows//webhook`. + +## Definitions + +- **Write**: `POST /api/execute/transfer` or `POST /api/execute/contract-call`. +- **Status read**: polling `GET /api/execute/{id}/status`. +- **Successful receipt**: `receipts[]` entry with `receiptStatus=success`. +- **Idempotency key**: client-supplied `Idempotency-Key` header that must be byte-identical across retries of the same logical write. + +## Status vocabularies (do not mix) + +"Pending" and "Terminal" below are **client wait semantics**: keep polling, or stop waiting and report. Terminal is not a claim that the server will never change the row again. + +| Surface | Pending | Terminal | +| --- | --- | --- | +| Direct execution (`GET /api/execute/{id}/status`) | `pending`, `running` | `unconfirmed`, `completed`, `failed` | +| Workflow run (`GET /api/workflows/executions/{id}/status`) | `pending`, `running` | `success`, `error`, `cancelled` | + +Server enum (`app/api/execute/_lib/types.ts`): `pending | running | unconfirmed | completed | failed`. There is no `queued` value. + +HTTP 404 on the status route is `{ "error": "Execution not found" }` with no `status` field (missing id, or an execution that belongs to another organization). It is not a status string. + +`completed` is **not** by itself proof of on-chain success. Inspect `receipts[].receiptStatus`. + +Receipt statuses (`lib/web3/verify-receipt.ts`): `success | reverted | not_found | timeout | safe_inner_failure`. +`reverted` and `safe_inner_failure` are conclusive. `not_found` and `timeout` mean the receipt could not be read; `completeExecution` settles those rows as `unconfirmed`, not `failed`. + +## Rules + +### R1 — Poll the same ID, never resubmit + +If status is `pending` or `running`, 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. + +`unconfirmed` means the transaction was broadcast but no receipt could be read yet. The server keeps that row open (`completedAt` stays null) and a reconciliation sweep settles it to `completed` or `failed` once the chain answers. A waiting client must **stop** there and report it, rather than poll to an expired budget and exit non-zero: a non-zero exit invites a re-run, and re-running an intent that may already be on chain is the double-spend this contract exists to prevent. Read the settled status later against the same execution ID. + +**CLI conformance:** `kh ex transfer --wait` / `kh ex cc --wait` poll the same ID, and stop on `unconfirmed` with exit code 0, printing the status and transaction hash. + +### R2 — Receipts (client invariant) + +Never infer success from `status=completed` alone. + +1. `receiptStatus=success` is the only successful receipt. +2. `reverted` and `safe_inner_failure` are failure. +3. If a body claims `completed` plus any non-success receipt, treat it as failure. + +KEEP-966 (`completeExecution`) re-verifies every claimed hash before writing `completed`. A reverted receipt is stored `verified: false` and the row settles as `failed`. `{status:"completed", verified:true, receiptStatus:"reverted"}` is **not** an observed production envelope. Fixtures that use that shape are labeled `kind: defensive` so the client stays fail-closed if the gate regresses. + +The shipped CLI implements `--require-verified` on `kh ex status`; the write commands have no such flag. Without it, `completed` with an empty `receipts` array is still treated as success, matching a no-hash completion. With it, the CLI exits non-zero unless the execution completed carrying at least one receipt and every receipt is `verified: true` with `receiptStatus: success`; `unconfirmed` fails the gate as not proven landed. + +**CLI conformance:** wait paths fail on `status=failed` and on non-success receipts as above. + +### R3 — Write retry → stable idempotency key + +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. + +The server (`lib/idempotency.ts` `idempotencyEarlyResponse`) returns HTTP 409 for two codes: + +| code | retryable | client | +| --- | --- | --- | +| `idempotency_in_progress` | true | Retry the **same** key. Do not mint a new key. | +| `idempotency_conflict` | false | Fail. The key is bound to a different payload. Do not rotate. | + +**CLI conformance:** `kh ex transfer` and `kh ex cc` set `Idempotency-Key` once per invocation. HTTP-layer 5xx retries reuse it. 409 in_progress retries it until `--timeout`. 409 conflict is a hard error. Use `--idempotency-key` to pin a key across process restarts. + +### R4 — Terminal failure / malformed + +Statuses `failed` (direct) and unparseable/malformed bodies are terminal for that attempt. Missing `status` after a 200 decode is **malformed**, not success. + +An unknown future `status` string is **unrecognized** (not malformed, not success) so a server addition does not look like a corrupt body. + +### 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. The HTTP client does not auto-retry 429. + +### R6 — Cold start 404 (bounded) + +A first status read that returns HTTP 404 immediately after submit may be transient. During `--wait`, poll until `--timeout` before treating 404 as terminal. + +`--watch` does not apply this tolerance. `kh ex st --watch` against a missing or foreign-org id must exit on 404, not loop. + +## Fixture mapping + conformance + +| Fixture | Kind | Rule | Expect | Consumed by | +| --- | --- | --- | --- | --- | +| `pending.json` | observed | R1 | pending | `TestFixtures_ClassifyTable` | +| `running.json` | observed | R1 | pending | `TestFixtures_ClassifyTable` | +| `unconfirmed.json` | observed | R1 | unconfirmed | `TestFixtures_ClassifyTable` | +| `completed_with_tx.json` | observed | R2 | success | `TestFixtures_ClassifyTable` | +| `completed_without_tx.json` | classifier | R2 | failure (strict option) | `TestFixtures_ClassifyTable` | +| `reverted.json` | defensive | R2 | failure | `TestFixtures_ClassifyTable` + `TestRevertedIsNeverSuccess` | +| `safe_inner_failure.json` | defensive | R2 | failure | `TestFixtures_ClassifyTable` | +| `failed.json` | observed | R4 | failure | `TestFixtures_ClassifyTable` | +| `malformed.json` | observed | R4 | malformed | `TestFixtures_ClassifyTable` | +| `not_found.json` | observed | R6 | pending (HTTP 404) | `TestFixtures_ClassifyTable` | +| `rate_limited.json` | observed | R5 | rate_limited | `TestFixtures_ClassifyTable` | +| `cold_start.sequence.json` | observed | R6 | 404→pending→success | `TestColdStartSequence_R6` | + +Every fixture carries `"version": 1`. Loaders fail on a missing or unsupported version. `TestFixtures_ExpectedCountsAndRules` asserts the expected file count so a rename to `*.sequence.json` cannot drop coverage silently. + +Fixtures use the **flat** direct-execution wire shape (`executionId`, not nested `execution.id`). diff --git a/docs/execution-recovery.md b/docs/execution-recovery.md new file mode 100644 index 0000000..9114908 --- /dev/null +++ b/docs/execution-recovery.md @@ -0,0 +1,77 @@ +# Execution recovery + +Agents and adapters that submit KeeperHub **direct-execution** writes +(`POST /api/execute/transfer`, `POST /api/execute/contract-call`) must recover +safely when the network flakes, a status read races ahead of persistence, or +an on-chain receipt is not successful. + +This guide is the published summary of the normative contract in +[`execution-recovery-v1/contract.md`](./execution-recovery-v1/contract.md). + +It does **not** describe `POST /api/workflows//webhook`. That is a +different surface. + +## Safe first-write sequence + +1. Simulate when available 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. Do not infer on-chain success from `status=completed` alone. Treat + `receipts[].receiptStatus` as the receipt evidence: only `success` is a + successful receipt. `reverted` and `safe_inner_failure` are conclusive + failures. `not_found` and `timeout` mean the receipt was unreadable; the + server settles those rows as `unconfirmed`, not `failed`. + +KEEP-966 on the server re-verifies every claimed hash before writing +`completed`. A reverted receipt is stored `verified: false` and the row +settles as `failed`. A client that still sees `completed` plus a non-success +receipt must fail closed — that combination is a defensive invariant, not an +observed production envelope. + +## Direct-execution status vocabulary + +`pending | running | unconfirmed | completed | failed` + +(`app/api/execute/_lib/types.ts`. There is no `queued` status on this endpoint.) + +Poll while a row is `pending` or `running`. Stop on `unconfirmed`, `completed` +or `failed`. `unconfirmed` means the transaction was broadcast but no receipt +could be read; the server keeps reconciling that row, so treat it as "stop +waiting and report", not as a failure, and read the settled status later +against the same execution ID. + +Workflow run status uses a different vocabulary (`success` / `error` / +`cancelled`) — do not mix it with direct-execution statuses. + +## CLI behaviour + +- `kh ex transfer` / `kh ex cc` attach `Idempotency-Key` on every write. + Override with `--idempotency-key` to pin a key across process restarts. +- HTTP 5xx retries reuse that key. +- HTTP 409 `idempotency_in_progress`: retry the **same** key until `--timeout`. + Do not mint a new key. +- HTTP 409 `idempotency_conflict`: fail. Do not rotate the key (that would + broadcast a second transaction). +- `--wait` polls the same execution ID and tolerates a **bounded** initial HTTP + 404 until `--timeout` (default 5m). Persistent 404 (wrong id, other org) is + a timeout error. +- `--watch` does **not** treat 404 as pending. A mistyped or foreign-org id + exits with an error instead of looping. +- `--wait` and `--watch` stop on `unconfirmed` and report it. `--wait` exits + **zero** there, printing the status and transaction hash, because a non-zero + exit invites a re-run that would broadcast a second transaction. +- Wait paths fail when `status=failed`, when a receipt is `reverted` or + `safe_inner_failure`, and when `status=completed` carries any non-success + receipt. +- `kh ex status --require-verified` additionally demands chain proof: it exits + non-zero unless the execution completed carrying at least one receipt and + every receipt is `verified: true` with `receiptStatus: success`. `unconfirmed` + fails that gate, and so does a completion with an empty `receipts` array, + which is treated as success without the flag. + +## Fixtures + +Golden responses live under `testdata/execution_recovery_v1/` (`version: 1`) +and are loaded by `go test ./internal/execrecovery/...`. Each fixture is +labeled `observed`, `defensive`, or `classifier`. 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..5f5d045 --- /dev/null +++ b/internal/execrecovery/classify.go @@ -0,0 +1,195 @@ +// 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" + // OutcomeUnconfirmed is broadcast-but-unreadable. It is neither success nor + // failure: the transaction may already be on chain. Callers stop waiting and + // report it; they must not poll it through and must not resubmit. + OutcomeUnconfirmed Outcome = "unconfirmed" + OutcomeMalformed Outcome = "malformed" + OutcomeRateLimited Outcome = "rate_limited" + OutcomeUnrecognized Outcome = "unrecognized" +) + +// Options controls classification strictness. +type Options struct { + // RequireChainEvidence is a classifier-only option used by fixtures. + // The shipped CLI wait paths do not set it. When true, completed + // without a verified successful receipt is Failure. + RequireChainEvidence bool +} + +// Receipt is GET /api/execute/{id}/status receipts[] +// (DirectExecutionReceiptEntry in KeeperHub/keeperhub). +// +// receiptStatus values from lib/web3/verify-receipt.ts: +// success | reverted | not_found | timeout | safe_inner_failure. +// verifiedAt is required on the server type. chainId is optional. +type Receipt struct { + Hash string `json:"hash"` + ChainID *int `json:"chainId,omitempty"` + Network string `json:"network,omitempty"` + Verified bool `json:"verified"` + ReceiptStatus string `json:"receiptStatus"` + BlockNumber *int64 `json:"blockNumber,omitempty"` + GasUsed *string `json:"gasUsed,omitempty"` + VerifiedAt string `json:"verifiedAt"` +} + +// DirectStatus is the flat wire shape of GET /api/execute/{id}/status. +// Canonical type: cmd/execute aliases this as ExecStatusResponse. +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. +// +// Direct-execution statuses are pending|running|unconfirmed|completed|failed +// (app/api/execute/_lib/types.ts). Workflow run statuses (success|error|cancelled) +// belong to a different API and must not be fed here — see Vocabulary(). +// +// `unconfirmed` maps to OutcomeUnconfirmed, not OutcomePending: the server +// keeps reconciling that row, but a client must stop waiting on it rather than +// poll to a failure the chain never reported. +// +// An unknown future status is OutcomeUnrecognized (never success, never +// malformed) so a server addition does not look like a corrupt body. +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. The status endpoint answers 404 + // with {"error":"Execution not found"} and no status field. + if sample.HTTPStatus == http.StatusNotFound { + return OutcomePending, "http 404" + } + + if sample.HTTPStatus != 0 && sample.HTTPStatus != http.StatusOK && sample.HTTPStatus != http.StatusAccepted { + 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 == "" { + return OutcomeMalformed, "missing status field" + } + + switch status { + case "pending", "running": + return OutcomePending, status + case "unconfirmed": + return OutcomeUnconfirmed, status + case "failed": + return OutcomeFailure, status + case "completed": + return classifyCompleted(st, opts) + default: + return OutcomeUnrecognized, "unrecognized status: " + status + } +} + +func classifyCompleted(st DirectStatus, opts Options) (Outcome, string) { + if r := FirstBlockingReceipt(st.Receipts); r != nil { + return OutcomeFailure, "receiptStatus=" + r.ReceiptStatus + } + + 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. + // The shipped CLI does not set RequireChainEvidence. + return OutcomeSuccess, "completed" +} + +// ConclusiveFailedReceipt reports a chain-answered failure +// (lib/web3/verify-receipt.ts CONCLUSIVE_STATUSES minus success). +func ConclusiveFailedReceipt(status string) bool { + switch strings.ToLower(strings.TrimSpace(status)) { + case "reverted", "safe_inner_failure": + return true + default: + return false + } +} + +// NonSuccessReceipt is true when a receipt exists and is not explicitly successful. +func NonSuccessReceipt(r Receipt) bool { + s := strings.ToLower(strings.TrimSpace(r.ReceiptStatus)) + return s != "" && s != "success" +} + +// FirstBlockingReceipt returns the first receipt that must not be treated as success. +func FirstBlockingReceipt(receipts []Receipt) *Receipt { + for i := range receipts { + if NonSuccessReceipt(receipts[i]) { + return &receipts[i] + } + } + return nil +} + +func hasVerifiedSuccess(receipts []Receipt) bool { + for _, r := range receipts { + if r.Verified && strings.EqualFold(r.ReceiptStatus, "success") { + return true + } + } + return false +} diff --git a/internal/execrecovery/classify_receipt_test.go b/internal/execrecovery/classify_receipt_test.go new file mode 100644 index 0000000..056578a --- /dev/null +++ b/internal/execrecovery/classify_receipt_test.go @@ -0,0 +1,48 @@ +package execrecovery_test + +import ( + "testing" + + "github.com/keeperhub/cli/internal/execrecovery" +) + +func TestClassify_ReceiptStates(t *testing.T) { + completed := func(receiptStatus string, verified bool) []byte { + return []byte(`{"executionId":"x","status":"completed","transactionHash":"0xabc","receipts":[{"hash":"0xabc","verified":` + boolJSON(verified) + `,"receiptStatus":"` + receiptStatus + `","verifiedAt":"2026-08-11T00:00:00Z"}]}`) + } + unconfirmed := func(receiptStatus string) []byte { + return []byte(`{"executionId":"x","status":"unconfirmed","transactionHash":"0xabc","receipts":[{"hash":"0xabc","verified":false,"receiptStatus":"` + receiptStatus + `","verifiedAt":"2026-08-11T00:00:00Z"}]}`) + } + + t.Run("completed success", func(t *testing.T) { + got, _ := execrecovery.Classify(execrecovery.Sample{HTTPStatus: 200, Body: completed("success", true)}, execrecovery.Options{}) + if got != execrecovery.OutcomeSuccess { + t.Fatalf("got %s, want success", got) + } + }) + for _, st := range []string{"reverted", "safe_inner_failure", "not_found", "timeout"} { + st := st + t.Run("completed "+st+" is failure", func(t *testing.T) { + got, reason := execrecovery.Classify(execrecovery.Sample{HTTPStatus: 200, Body: completed(st, false)}, execrecovery.Options{}) + if got != execrecovery.OutcomeFailure { + t.Fatalf("got %s (%s), want failure", got, reason) + } + }) + } + for _, st := range []string{"not_found", "timeout"} { + st := st + t.Run("unconfirmed "+st+" is unconfirmed", func(t *testing.T) { + got, reason := execrecovery.Classify(execrecovery.Sample{HTTPStatus: 200, Body: unconfirmed(st)}, execrecovery.Options{}) + if got != execrecovery.OutcomeUnconfirmed { + t.Fatalf("got %s (%s), want unconfirmed", got, reason) + } + }) + } +} + +func boolJSON(v bool) string { + if v { + return "true" + } + return "false" +} diff --git a/internal/execrecovery/fixture.go b/internal/execrecovery/fixture.go new file mode 100644 index 0000000..7db80b9 --- /dev/null +++ b/internal/execrecovery/fixture.go @@ -0,0 +1,186 @@ +package execrecovery + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" +) + +const FixtureVersion = 1 + +// Kind labels how a fixture relates to production. +// +// observed: a response the current handler can emit. +// defensive: a client-side invariant against a body KEEP-966 makes unreachable. +// classifier: exercises a classifier option the shipped CLI does not set. +type Kind string + +const ( + KindObserved Kind = "observed" + KindDefensive Kind = "defensive" + KindClassifier Kind = "classifier" +) + +// Fixture is one conformance case loaded from testdata/execution_recovery_v1. +type Fixture struct { + Name string `json:"-"` + Version int `json:"version"` + Kind Kind `json:"kind"` + 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"` + Version int `json:"version"` + Kind Kind `json:"kind"` + Rule string `json:"rule"` + Steps []SequenceStep `json:"steps"` +} + +func validateMeta(name string, version int, kind Kind) error { + if version != FixtureVersion { + return fmt.Errorf("%s: unsupported fixture version %d (want %d)", name, version, FixtureVersion) + } + switch kind { + case KindObserved, KindDefensive, KindClassifier: + return nil + case "": + return fmt.Errorf("%s: missing kind (observed|defensive|classifier)", name) + default: + return fmt.Errorf("%s: unknown kind %q", name, kind) + } +} + +func isSequenceName(name string) bool { + return strings.HasSuffix(name, ".sequence.json") +} + +// LoadFixtureDir loads every versioned *.json fixture that is not a sequence. +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" || isSequenceName(name) { + 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) + } + if err := validateMeta(name, f.Version, f.Kind); err != nil { + return nil, err + } + if f.Rule == "" { + return nil, fmt.Errorf("%s: missing rule", name) + } + f.Name = name + out = append(out, f) + } + return out, nil +} + +// LoadSequenceDir loads every *.sequence.json fixture in dir. +func LoadSequenceDir(dir string) ([]SequenceFixture, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, err + } + var out []SequenceFixture + for _, e := range entries { + if e.IsDir() || !isSequenceName(e.Name()) { + continue + } + seq, err := LoadSequence(filepath.Join(dir, e.Name())) + if err != nil { + return nil, err + } + out = append(out, seq) + } + 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 + } + name := filepath.Base(path) + if seq.Name == "" { + seq.Name = name + } + if err := validateMeta(name, seq.Version, seq.Kind); err != nil { + return SequenceFixture{}, err + } + if seq.Rule == "" { + return SequenceFixture{}, fmt.Errorf("%s: missing rule", name) + } + if len(seq.Steps) == 0 { + return SequenceFixture{}, fmt.Errorf("%s: sequence has no steps", name) + } + 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..03e6da1 --- /dev/null +++ b/internal/execrecovery/fixture_test.go @@ -0,0 +1,242 @@ +package execrecovery_test + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/keeperhub/cli/internal/execrecovery" +) + +const ( + wantFixtures = 11 + wantSequences = 1 +) + +func testdataDir(t *testing.T) string { + t.Helper() + _, file, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("runtime.Caller failed") + } + root := filepath.Clean(filepath.Join(filepath.Dir(file), "..", "..")) + return filepath.Join(root, "testdata", "execution_recovery_v1") +} + +func TestLoadFixtureDir_EmptyFails(t *testing.T) { + dir := t.TempDir() + fixtures, err := execrecovery.LoadFixtureDir(dir) + if err != nil { + t.Fatalf("empty dir should load, got err %v", err) + } + if len(fixtures) != 0 { + t.Fatalf("got %d fixtures, want 0", len(fixtures)) + } +} + +func TestFixtures_ExpectedCountsAndRules(t *testing.T) { + dir := testdataDir(t) + fixtures, err := execrecovery.LoadFixtureDir(dir) + if err != nil { + t.Fatalf("LoadFixtureDir: %v", err) + } + if len(fixtures) != wantFixtures { + t.Fatalf("loaded %d fixtures, want %d (renaming a file to *.sequence.json must not silently drop coverage)", len(fixtures), wantFixtures) + } + seqs, err := execrecovery.LoadSequenceDir(dir) + if err != nil { + t.Fatalf("LoadSequenceDir: %v", err) + } + if len(seqs) != wantSequences { + t.Fatalf("loaded %d sequences, want %d", len(seqs), wantSequences) + } + + rules := map[string]int{} + kinds := map[execrecovery.Kind]int{} + for _, f := range fixtures { + if f.Version != execrecovery.FixtureVersion { + t.Fatalf("%s: version %d", f.Name, f.Version) + } + if f.Rule == "" { + t.Fatalf("%s: missing rule", f.Name) + } + rules[f.Rule]++ + kinds[f.Kind]++ + } + for _, need := range []string{"R1", "R2", "R4", "R5", "R6"} { + if rules[need] == 0 { + t.Fatalf("no fixture maps to rule %s", need) + } + } + if kinds[execrecovery.KindObserved] == 0 { + t.Fatal("expected at least one observed fixture") + } + if kinds[execrecovery.KindDefensive] == 0 { + t.Fatal("expected at least one defensive fixture") + } +} + +func TestLoadFixtureDir_RejectsMissingVersion(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "no_version.json") + body := []byte(`{"kind":"observed","rule":"R1","httpStatus":200,"expect":"pending","response":{"status":"pending"}}`) + if err := os.WriteFile(path, body, 0o644); err != nil { + t.Fatal(err) + } + if _, err := execrecovery.LoadFixtureDir(dir); err == nil { + t.Fatal("expected version error") + } +} + +func TestFixtures_DecodeIntoDirectStatus(t *testing.T) { + fixtures, err := execrecovery.LoadFixtureDir(testdataDir(t)) + if err != nil { + t.Fatalf("LoadFixtureDir: %v", err) + } + if len(fixtures) != wantFixtures { + t.Fatalf("loaded %d fixtures, want %d", len(fixtures), wantFixtures) + } + + for _, f := range fixtures { + f := f + t.Run(f.Name, func(t *testing.T) { + if f.ResponseRaw != "" { + 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 { + if f.HTTPStatus == 404 || f.HTTPStatus == 429 { + return + } + t.Fatalf("DecodeResponse: %v", err) + } + if f.HTTPStatus == 200 && f.Expect != execrecovery.OutcomeMalformed { + if st.Status == "" { + 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) + } + if len(fixtures) != wantFixtures { + t.Fatalf("loaded %d fixtures, want %d", len(fixtures), wantFixtures) + } + + 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) { + seqs, err := execrecovery.LoadSequenceDir(testdataDir(t)) + if err != nil { + t.Fatalf("LoadSequenceDir: %v", err) + } + if len(seqs) != wantSequences { + t.Fatalf("sequences=%d, want %d", len(seqs), wantSequences) + } + seq := seqs[0] + 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","verified":true,"receiptStatus":"reverted","verifiedAt":"2026-08-11T00:00:00Z"}] + }`) + got, reason := execrecovery.Classify(execrecovery.Sample{HTTPStatus: 200, Body: body}, execrecovery.Options{}) + 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 TestUnknownStatusIsUnrecognizedNotMalformed(t *testing.T) { + got, reason := execrecovery.Classify(execrecovery.Sample{ + HTTPStatus: 200, + Body: []byte(`{"executionId":"x","status":"settling"}`), + }, execrecovery.Options{}) + if got != execrecovery.OutcomeUnrecognized { + t.Fatalf("got %s (%s), want unrecognized", got, reason) + } +} + +func TestWorkflowStatusesAreNotDirectSuccess(t *testing.T) { + for _, status := range []string{"success", "error", "cancelled", "queued"} { + got, _ := execrecovery.Classify(execrecovery.Sample{ + HTTPStatus: 200, + Body: []byte(`{"executionId":"x","status":"` + status + `"}`), + }, execrecovery.Options{}) + if got == execrecovery.OutcomeSuccess { + t.Fatalf("status %s must not classify as success", status) + } + if got == execrecovery.OutcomeMalformed { + t.Fatalf("status %s must not classify as malformed (would break a future enum addition)", status) + } + } +} + +func TestVocabularySurfacesAreDistinct(t *testing.T) { + d := execrecovery.DirectExecutionVocabulary() + w := execrecovery.WorkflowRunVocabulary() + if d.Surface == w.Surface { + t.Fatal("vocabularies must name distinct surfaces") + } + direct := map[string]struct{}{} + for _, s := range append(append([]string{}, d.Pending...), d.Terminal...) { + direct[s] = struct{}{} + } + for _, term := range w.Terminal { + if _, ok := direct[term]; ok { + t.Fatalf("workflow terminal %q must not appear in direct-execution vocabulary", term) + } + } + for _, s := range d.Pending { + if s == "queued" || s == "not_found" { + t.Fatalf("direct pending must not include %q (not in server ExecutionStatus)", s) + } + } +} diff --git a/internal/execrecovery/idempotency.go b/internal/execrecovery/idempotency.go new file mode 100644 index 0000000..39da8ae --- /dev/null +++ b/internal/execrecovery/idempotency.go @@ -0,0 +1,114 @@ +package execrecovery + +import ( + "crypto/rand" + "encoding/hex" + "encoding/json" + "fmt" + "strings" +) + +const IdempotencyHeader = "Idempotency-Key" + +// Codes from KeeperHub/keeperhub lib/idempotency.ts idempotencyEarlyResponse. +// Both map to HTTP 409; only `code` distinguishes them. +const ( + CodeIdempotencyConflict = "idempotency_conflict" + CodeIdempotencyInProgress = "idempotency_in_progress" +) + +// IdempotencyBody is the 409 JSON from idempotencyEarlyResponse. +type IdempotencyBody struct { + Error string `json:"error"` + Code string `json:"code"` + Retryable *bool `json:"retryable"` + OriginalExecutionID *string `json:"originalExecutionId"` +} + +// ParseIdempotencyBody decodes a 409 body. ok is true when `code` is one of +// the two idempotency codes, or when retryable is present as a fallback. +func ParseIdempotencyBody(body []byte) (IdempotencyBody, bool) { + var b IdempotencyBody + if err := json.Unmarshal(body, &b); err != nil { + return IdempotencyBody{}, false + } + code := strings.ToLower(strings.TrimSpace(b.Code)) + b.Code = code + switch code { + case CodeIdempotencyConflict, CodeIdempotencyInProgress: + return b, true + } + if b.Retryable != nil { + return b, true + } + return b, false +} + +// IsInProgress is true when the same key is already processing. +// The client must retry that key and must not mint a new one. +func (b IdempotencyBody) IsInProgress() bool { + if b.Code == CodeIdempotencyInProgress { + return true + } + return b.Code == "" && b.Retryable != nil && *b.Retryable +} + +// IsConflict is true when the key is bound to a different payload. +// The client must fail closed and must not rotate the key. +func (b IdempotencyBody) IsConflict() bool { + if b.Code == CodeIdempotencyConflict { + return true + } + return b.Code == "" && b.Retryable != nil && !*b.Retryable +} + +// ConflictError is a 409 idempotency_conflict. +type ConflictError struct { + Body IdempotencyBody + Key string +} + +func (e ConflictError) Error() string { + msg := e.Body.Error + if msg == "" { + msg = "Idempotency-Key was reused with a different request payload" + } + if e.Body.OriginalExecutionID != nil && *e.Body.OriginalExecutionID != "" { + msg = fmt.Sprintf("%s (originalExecutionId=%s)", msg, *e.Body.OriginalExecutionID) + } + if e.Key != "" { + msg = fmt.Sprintf("%s; do not retry with a new key for the same intent (Idempotency-Key %s)", msg, e.Key) + } else { + msg = msg + "; do not retry with a new Idempotency-Key for the same intent" + } + return msg +} + +// InProgressTimeoutError is returned when 409 idempotency_in_progress +// outlives the wait budget. The same key must be reused on the next attempt. +type InProgressTimeoutError struct { + Key string +} + +func (e InProgressTimeoutError) Error() string { + return fmt.Sprintf("Idempotency-Key %s is still in progress; retry with --idempotency-key %s (do not rotate the key)", e.Key, e.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) + } + 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..26c2263 --- /dev/null +++ b/internal/execrecovery/idempotency_test.go @@ -0,0 +1,74 @@ +package execrecovery_test + +import ( + "strings" + "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") + } +} + +func TestParseIdempotencyBody_InProgress(t *testing.T) { + b, ok := execrecovery.ParseIdempotencyBody([]byte(`{"error":"A request with this Idempotency-Key is already being processed. Retry the same key shortly; do not rotate it.","code":"idempotency_in_progress","retryable":true}`)) + if !ok { + t.Fatal("expected parse ok") + } + if !b.IsInProgress() || b.IsConflict() { + t.Fatalf("in_progress misclassified: %+v", b) + } +} + +func TestParseIdempotencyBody_Conflict(t *testing.T) { + orig := "exec-original" + b, ok := execrecovery.ParseIdempotencyBody([]byte(`{"error":"Idempotency-Key was reused with a different request payload. Use a new key for a different request.","code":"idempotency_conflict","originalExecutionId":"exec-original","retryable":false}`)) + if !ok { + t.Fatal("expected parse ok") + } + if !b.IsConflict() || b.IsInProgress() { + t.Fatalf("conflict misclassified: %+v", b) + } + if b.OriginalExecutionID == nil || *b.OriginalExecutionID != orig { + t.Fatalf("originalExecutionId=%v", b.OriginalExecutionID) + } + err := execrecovery.ConflictError{Body: b, Key: "k1"} + if !strings.Contains(err.Error(), "exec-original") || !strings.Contains(err.Error(), "do not retry with a new key") { + t.Fatalf("conflict error=%s", err.Error()) + } +} + +func TestParseIdempotencyBody_Unknown409(t *testing.T) { + _, ok := execrecovery.ParseIdempotencyBody([]byte(`{"error":"something else"}`)) + if ok { + t.Fatal("generic 409 must not look like an idempotency code") + } +} diff --git a/internal/execrecovery/vocabulary.go b/internal/execrecovery/vocabulary.go new file mode 100644 index 0000000..502a647 --- /dev/null +++ b/internal/execrecovery/vocabulary.go @@ -0,0 +1,37 @@ +package execrecovery + +// Vocabulary documents which status strings belong to which API surface. +// Direct-execution and workflow-run statuses must not be mixed. +// +// Pending and Terminal are client wait semantics: Pending means keep polling, +// Terminal means stop waiting and report. Terminal is not a claim that the +// server will never change the row again. +type Vocabulary struct { + Surface string + Pending []string + Terminal []string +} + +// DirectExecutionVocabulary is GET /api/execute/{id}/status +// (app/api/execute/_lib/types.ts ExecutionStatus). +// +// `unconfirmed` is listed Terminal in the client sense only. The server +// documents it as non-terminal and a reconciliation sweep settles it to +// completed or failed; clients still stop there so that an unreadable receipt +// never becomes a re-run that broadcasts twice. +func DirectExecutionVocabulary() Vocabulary { + return Vocabulary{ + Surface: "direct-execution", + Pending: []string{"pending", "running"}, + Terminal: []string{"unconfirmed", "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.sequence.json b/testdata/execution_recovery_v1/cold_start.sequence.json new file mode 100644 index 0000000..9b4c224 --- /dev/null +++ b/testdata/execution_recovery_v1/cold_start.sequence.json @@ -0,0 +1,42 @@ +{ + "name": "cold_start", + "version": 1, + "kind": "observed", + "rule": "R6", + "steps": [ + { + "httpStatus": 404, + "expect": "pending", + "response": { + "error": "Execution not found" + } + }, + { + "httpStatus": 200, + "expect": "pending", + "response": { + "executionId": "exec_fixture_cold_001", + "status": "pending", + "createdAt": "2026-08-11T00:00:00.000Z" + } + }, + { + "httpStatus": 200, + "expect": "success", + "response": { + "executionId": "exec_fixture_cold_001", + "status": "completed", + "transactionHash": "0x4444444444444444444444444444444444444444444444444444444444444444", + "receipts": [ + { + "hash": "0x4444444444444444444444444444444444444444444444444444444444444444", + "chainId": 8453, + "verified": true, + "receiptStatus": "success", + "verifiedAt": "2026-08-11T00:01:00.000Z" + } + ] + } + } + ] +} 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..7b9f46e --- /dev/null +++ b/testdata/execution_recovery_v1/completed_with_tx.json @@ -0,0 +1,24 @@ +{ + "version": 1, + "kind": "observed", + "rule": "R2", + "httpStatus": 200, + "requireChainEvidence": true, + "expect": "success", + "response": { + "executionId": "exec_fixture_ok_001", + "status": "completed", + "transactionHash": "0x1111111111111111111111111111111111111111111111111111111111111111", + "transactionLink": "https://basescan.org/tx/0x1111111111111111111111111111111111111111111111111111111111111111", + "receipts": [ + { + "hash": "0x1111111111111111111111111111111111111111111111111111111111111111", + "chainId": 8453, + "verified": true, + "receiptStatus": "success", + "verifiedAt": "2026-08-11T00:01:00.000Z" + } + ], + "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..f5ac3f6 --- /dev/null +++ b/testdata/execution_recovery_v1/completed_without_tx.json @@ -0,0 +1,14 @@ +{ + "version": 1, + "kind": "classifier", + "rule": "R2", + "httpStatus": 200, + "requireChainEvidence": true, + "expect": "failure", + "response": { + "executionId": "exec_fixture_no_tx_001", + "status": "completed", + "completedAt": "2026-08-11T00:01:00.000Z" + }, + "note": "Classifier-only: RequireChainEvidence is not set by the shipped CLI. Production wait paths still treat completed with no receipts as success." +} diff --git a/testdata/execution_recovery_v1/failed.json b/testdata/execution_recovery_v1/failed.json new file mode 100644 index 0000000..7400817 --- /dev/null +++ b/testdata/execution_recovery_v1/failed.json @@ -0,0 +1,13 @@ +{ + "version": 1, + "kind": "observed", + "rule": "R4", + "httpStatus": 200, + "requireChainEvidence": false, + "expect": "failure", + "response": { + "executionId": "exec_fixture_failed_001", + "status": "failed", + "error": "simulation reverted: insufficient funds" + } +} diff --git a/testdata/execution_recovery_v1/malformed.json b/testdata/execution_recovery_v1/malformed.json new file mode 100644 index 0000000..01ebc6e --- /dev/null +++ b/testdata/execution_recovery_v1/malformed.json @@ -0,0 +1,10 @@ +{ + "version": 1, + "kind": "observed", + "rule": "R4", + "httpStatus": 200, + "requireChainEvidence": false, + "expect": "malformed", + "responseRaw": "{{{this is not json", + "note": "Genuine unparseable body — must not be treated as success." +} diff --git a/testdata/execution_recovery_v1/not_found.json b/testdata/execution_recovery_v1/not_found.json new file mode 100644 index 0000000..0a65a22 --- /dev/null +++ b/testdata/execution_recovery_v1/not_found.json @@ -0,0 +1,12 @@ +{ + "version": 1, + "kind": "observed", + "rule": "R6", + "httpStatus": 404, + "requireChainEvidence": false, + "expect": "pending", + "response": { + "error": "Execution not found" + }, + "note": "Observed GET /api/execute/{id}/status 404 body. Classify treats a single 404 as pending (cold-start). Exhausted --wait budget is a caller-level failure. --watch treats 404 as a terminal error (mistyped or foreign-org id)." +} diff --git a/testdata/execution_recovery_v1/pending.json b/testdata/execution_recovery_v1/pending.json new file mode 100644 index 0000000..29cd69a --- /dev/null +++ b/testdata/execution_recovery_v1/pending.json @@ -0,0 +1,14 @@ +{ + "version": 1, + "kind": "observed", + "rule": "R1", + "httpStatus": 200, + "requireChainEvidence": false, + "expect": "pending", + "response": { + "executionId": "exec_fixture_pending_001", + "status": "pending", + "createdAt": "2026-08-11T00:00:00.000Z" + }, + "note": "Direct-execution enum: pending | running | unconfirmed | completed | failed." +} diff --git a/testdata/execution_recovery_v1/rate_limited.json b/testdata/execution_recovery_v1/rate_limited.json new file mode 100644 index 0000000..da9c4d5 --- /dev/null +++ b/testdata/execution_recovery_v1/rate_limited.json @@ -0,0 +1,11 @@ +{ + "version": 1, + "kind": "observed", + "rule": "R5", + "httpStatus": 429, + "requireChainEvidence": false, + "expect": "rate_limited", + "response": { + "error": "Rate limit exceeded" + } +} diff --git a/testdata/execution_recovery_v1/reverted.json b/testdata/execution_recovery_v1/reverted.json new file mode 100644 index 0000000..51f926e --- /dev/null +++ b/testdata/execution_recovery_v1/reverted.json @@ -0,0 +1,24 @@ +{ + "version": 1, + "kind": "defensive", + "rule": "R2", + "httpStatus": 200, + "requireChainEvidence": false, + "expect": "failure", + "response": { + "executionId": "exec_fixture_reverted_001", + "status": "completed", + "transactionHash": "0x3333333333333333333333333333333333333333333333333333333333333333", + "receipts": [ + { + "hash": "0x3333333333333333333333333333333333333333333333333333333333333333", + "chainId": 8453, + "verified": true, + "receiptStatus": "reverted", + "verifiedAt": "2026-08-11T00:01:00.000Z" + } + ], + "completedAt": "2026-08-11T00:01:00.000Z" + }, + "note": "DEFENSIVE, not an observed production envelope. KEEP-966 re-verifies every claimed hash before writing completed; a reverted receipt is verified:false and the row settles as failed. This fixture asserts the client invariant: never infer success from status=completed alone." +} diff --git a/testdata/execution_recovery_v1/running.json b/testdata/execution_recovery_v1/running.json new file mode 100644 index 0000000..19cd389 --- /dev/null +++ b/testdata/execution_recovery_v1/running.json @@ -0,0 +1,13 @@ +{ + "version": 1, + "kind": "observed", + "rule": "R1", + "httpStatus": 200, + "requireChainEvidence": false, + "expect": "pending", + "response": { + "executionId": "exec_fixture_running_001", + "status": "running", + "createdAt": "2026-08-11T00:00:00.000Z" + } +} diff --git a/testdata/execution_recovery_v1/safe_inner_failure.json b/testdata/execution_recovery_v1/safe_inner_failure.json new file mode 100644 index 0000000..c5c5355 --- /dev/null +++ b/testdata/execution_recovery_v1/safe_inner_failure.json @@ -0,0 +1,23 @@ +{ + "version": 1, + "kind": "defensive", + "rule": "R2", + "httpStatus": 200, + "requireChainEvidence": false, + "expect": "failure", + "response": { + "executionId": "exec_fixture_safe_inner_001", + "status": "completed", + "transactionHash": "0x5555555555555555555555555555555555555555555555555555555555555555", + "receipts": [ + { + "hash": "0x5555555555555555555555555555555555555555555555555555555555555555", + "verified": false, + "receiptStatus": "safe_inner_failure", + "verifiedAt": "2026-08-11T00:01:00.000Z" + } + ], + "completedAt": "2026-08-11T00:01:00.000Z" + }, + "note": "DEFENSIVE. safe_inner_failure is a conclusive non-success receipt (verify-receipt.ts). KEEP-966 would settle the row as failed; the client must still not treat completed as success." +} diff --git a/testdata/execution_recovery_v1/unconfirmed.json b/testdata/execution_recovery_v1/unconfirmed.json new file mode 100644 index 0000000..acc7216 --- /dev/null +++ b/testdata/execution_recovery_v1/unconfirmed.json @@ -0,0 +1,23 @@ +{ + "version": 1, + "kind": "observed", + "rule": "R1", + "httpStatus": 200, + "requireChainEvidence": false, + "expect": "unconfirmed", + "response": { + "executionId": "exec_fixture_unconfirmed_001", + "status": "unconfirmed", + "transactionHash": "0x2222222222222222222222222222222222222222222222222222222222222222", + "receipts": [ + { + "hash": "0x2222222222222222222222222222222222222222222222222222222222222222", + "verified": false, + "receiptStatus": "not_found", + "verifiedAt": "2026-08-11T00:00:30.000Z" + } + ], + "createdAt": "2026-08-11T00:00:00.000Z" + }, + "note": "Broadcast but the receipt was unreadable. The server settles unconfirmed, not failed, and a reconciliation sweep finishes the row later. Wait paths stop here and exit zero; never resubmit this intent. Read the settled status later against the same executionId." +}