diff --git a/rip302_agent_economy.py b/rip302_agent_economy.py index f236feeca..401d72a71 100644 --- a/rip302_agent_economy.py +++ b/rip302_agent_economy.py @@ -533,14 +533,26 @@ def agent_deliver_job(job_id): return jsonify({"error": "Job not found"}), 404 j = dict(zip(cols, row)) - if j["status"] != STATUS_CLAIMED: - return jsonify({"error": f"Job must be in 'claimed' status (current: {j['status']})"}), 409 + # A disputed job is a re-delivery, not a first delivery: /dispute + # answers the worker with "Worker can re-deliver or admin can + # refund", and no route ever moved 'disputed' back to 'claimed', + # so rejecting it here left the worker with no way to act on the + # rejection reason while the escrow stayed locked. + if j["status"] not in (STATUS_CLAIMED, STATUS_DISPUTED): + return jsonify({ + "error": f"Job must be in 'claimed' or 'disputed' status (current: {j['status']})" + }), 409 + redelivery = j["status"] == STATUS_DISPUTED if j["worker_wallet"] != worker: return jsonify({"error": "Only the assigned worker can deliver"}), 403 now = int(time.time()) - if now > j["expires_at"]: + # TTL only gates a first delivery. A disputed job is deliberately + # outside the expiry sweep (_expire_refundable_job ignores it), so + # applying the gate here would fail re-delivery with a misleading + # STATE_RACE once the original TTL elapsed. + if not redelivery and now > j["expires_at"]: if _expire_refundable_job(c, j, now): conn.commit() return jsonify({"error": "Job has expired"}), 410 @@ -550,12 +562,14 @@ def agent_deliver_job(job_id): "code": "STATE_RACE", }), 409 + expected_status = STATUS_DISPUTED if redelivery else STATUS_CLAIMED c.execute(""" UPDATE agent_jobs SET status = 'delivered', deliverable_url = ?, - deliverable_hash = ?, result_summary = ?, delivered_at = ? + deliverable_hash = ?, result_summary = ?, delivered_at = ?, + rejection_reason = '' WHERE job_id = ? AND status = ? - """, (deliverable_url, deliverable_hash, result_summary, now, job_id, STATUS_CLAIMED)) + """, (deliverable_url, deliverable_hash, result_summary, now, job_id, expected_status)) if c.rowcount == 0: conn.rollback() return jsonify({ @@ -563,7 +577,7 @@ def agent_deliver_job(job_id): "code": "STATE_RACE", }), 409 - _log_job_action(c, job_id, "delivered", worker, + _log_job_action(c, job_id, "redelivered" if redelivery else "delivered", worker, f"url={deliverable_url}") conn.commit() @@ -750,7 +764,9 @@ def agent_dispute_job(job_id): "ok": True, "job_id": job_id, "status": STATUS_DISPUTED, - "message": "Job disputed. Escrow held pending resolution. Worker can re-deliver or admin can refund." + "message": ("Job disputed. Escrow held pending resolution. The assigned worker " + "can re-deliver via POST /agent/jobs//deliver, or the poster can " + "refund the escrow via POST /agent/jobs//cancel.") }) except Exception as e: diff --git a/tests/test_agent_dispute_redelivery.py b/tests/test_agent_dispute_redelivery.py new file mode 100644 index 000000000..edb80d68d --- /dev/null +++ b/tests/test_agent_dispute_redelivery.py @@ -0,0 +1,254 @@ +# SPDX-License-Identifier: MIT +"""RIP-302: a disputed job must remain workable by the assigned worker. + +`POST /agent/jobs//dispute` answers the client with "the assigned worker can +re-deliver", but `/deliver` used to accept only `claimed` jobs and no route ever +moved a job back out of `disputed`. That left the worker unable to act on the +rejection reason while the escrow stayed locked (a disputed job is also outside +the TTL expiry sweep), so the only reachable exit was the poster cancelling and +taking the full escrow back — after the deliverable was already readable on the +public job endpoint. +""" +import sqlite3 +from pathlib import Path + +from flask import Flask + +import rip302_agent_economy + + +def _make_app(tmp_path: Path, poster_balance: int = 5_000_000): + db_path = tmp_path / "agent_jobs.db" + app = Flask(__name__) + rip302_agent_economy.register_agent_economy(app, str(db_path)) + with sqlite3.connect(db_path) as conn: + conn.execute( + "CREATE TABLE balances (miner_id TEXT PRIMARY KEY, amount_i64 INTEGER NOT NULL)" + ) + conn.execute( + "INSERT INTO balances (miner_id, amount_i64) VALUES (?, ?)", + ("poster", poster_balance), + ) + return app, db_path + + +def _balance(db_path: Path, wallet: str) -> int: + with sqlite3.connect(db_path) as conn: + row = conn.execute( + "SELECT amount_i64 FROM balances WHERE miner_id = ?", (wallet,) + ).fetchone() + return row[0] if row else 0 + + +def _job_row(db_path: Path, job_id: str) -> dict: + with sqlite3.connect(db_path) as conn: + conn.row_factory = sqlite3.Row + return dict( + conn.execute( + "SELECT * FROM agent_jobs WHERE job_id = ?", (job_id,) + ).fetchone() + ) + + +def _post_job(client, reward_rtc: int = 1) -> str: + resp = client.post( + "/agent/jobs", + json={ + "poster_wallet": "poster", + "title": "Write a scraper", + "description": "Scrape the public listing page and return a CSV of rows.", + "category": "code", + "reward_rtc": reward_rtc, + }, + ) + assert resp.status_code == 201, resp.get_json() + return resp.get_json()["job_id"] + + +def _disputed_job(client) -> str: + job_id = _post_job(client) + assert client.post( + f"/agent/jobs/{job_id}/claim", json={"worker_wallet": "worker"} + ).status_code == 200 + assert client.post( + f"/agent/jobs/{job_id}/deliver", + json={ + "worker_wallet": "worker", + "result_summary": "first attempt", + "deliverable_url": "https://example.com/v1.csv", + }, + ).status_code == 200 + assert client.post( + f"/agent/jobs/{job_id}/dispute", + json={"poster_wallet": "poster", "reason": "missing the price column"}, + ).status_code == 200 + return job_id + + +def test_assigned_worker_can_redeliver_a_disputed_job_and_get_paid(tmp_path): + app, db_path = _make_app(tmp_path) + client = app.test_client() + job_id = _disputed_job(client) + + resp = client.post( + f"/agent/jobs/{job_id}/deliver", + json={ + "worker_wallet": "worker", + "result_summary": "added the price column", + "deliverable_url": "https://example.com/v2.csv", + }, + ) + assert resp.status_code == 200, resp.get_json() + assert resp.get_json()["status"] == "delivered" + + job = _job_row(db_path, job_id) + assert job["status"] == "delivered" + assert job["deliverable_url"] == "https://example.com/v2.csv" + assert job["result_summary"] == "added the price column" + + # The escrow can now actually reach the worker instead of only going back. + accept = client.post( + f"/agent/jobs/{job_id}/accept", json={"poster_wallet": "poster"} + ) + assert accept.status_code == 200, accept.get_json() + assert _balance(db_path, "worker") == 1_000_000 + assert _balance(db_path, "agent_escrow") == 0 + + +def test_redelivery_clears_the_stale_rejection_reason(tmp_path): + app, db_path = _make_app(tmp_path) + client = app.test_client() + job_id = _disputed_job(client) + assert _job_row(db_path, job_id)["rejection_reason"] == "missing the price column" + + assert client.post( + f"/agent/jobs/{job_id}/deliver", + json={"worker_wallet": "worker", "result_summary": "fixed"}, + ).status_code == 200 + + assert _job_row(db_path, job_id)["rejection_reason"] == "" + + +def test_redelivery_is_recorded_distinctly_in_the_activity_log(tmp_path): + app, db_path = _make_app(tmp_path) + client = app.test_client() + job_id = _disputed_job(client) + assert client.post( + f"/agent/jobs/{job_id}/deliver", + json={"worker_wallet": "worker", "result_summary": "fixed"}, + ).status_code == 200 + + actions = [ + entry["action"] + for entry in client.get(f"/agent/jobs/{job_id}").get_json()["job"]["activity_log"] + ] + assert actions.count("delivered") == 1 + assert "redelivered" in actions + + +def test_only_the_assigned_worker_can_redeliver_a_disputed_job(tmp_path): + app, db_path = _make_app(tmp_path) + client = app.test_client() + job_id = _disputed_job(client) + + resp = client.post( + f"/agent/jobs/{job_id}/deliver", + json={"worker_wallet": "someone_else", "result_summary": "let me in"}, + ) + assert resp.status_code == 403 + assert _job_row(db_path, job_id)["status"] == "disputed" + + +def test_redelivery_still_works_after_the_original_ttl_elapsed(tmp_path): + """A disputed job is outside the expiry sweep, so its TTL must not gate + re-delivery — otherwise the fix would silently expire for any dispute that + outlives the original deadline.""" + app, db_path = _make_app(tmp_path) + client = app.test_client() + job_id = _disputed_job(client) + with sqlite3.connect(db_path) as conn: + conn.execute("UPDATE agent_jobs SET expires_at = 1 WHERE job_id = ?", (job_id,)) + + resp = client.post( + f"/agent/jobs/{job_id}/deliver", + json={"worker_wallet": "worker", "result_summary": "late but done"}, + ) + assert resp.status_code == 200, resp.get_json() + assert _job_row(db_path, job_id)["status"] == "delivered" + + +def test_open_and_completed_jobs_are_still_not_deliverable(tmp_path): + app, db_path = _make_app(tmp_path) + client = app.test_client() + + open_job = _post_job(client) + resp = client.post( + f"/agent/jobs/{open_job}/deliver", + json={"worker_wallet": "worker", "result_summary": "unclaimed"}, + ) + assert resp.status_code == 409 + assert _job_row(db_path, open_job)["status"] == "open" + + done_job = _post_job(client) + assert client.post( + f"/agent/jobs/{done_job}/claim", json={"worker_wallet": "worker"} + ).status_code == 200 + assert client.post( + f"/agent/jobs/{done_job}/deliver", + json={"worker_wallet": "worker", "result_summary": "done"}, + ).status_code == 200 + assert client.post( + f"/agent/jobs/{done_job}/accept", json={"poster_wallet": "poster"} + ).status_code == 200 + + resp = client.post( + f"/agent/jobs/{done_job}/deliver", + json={"worker_wallet": "worker", "result_summary": "again"}, + ) + assert resp.status_code == 409 + assert _job_row(db_path, done_job)["status"] == "completed" + + +def test_claimed_job_past_ttl_still_expires_and_refunds_on_deliver(tmp_path): + """Regression guard: skipping the TTL gate must apply only to re-delivery.""" + app, db_path = _make_app(tmp_path) + client = app.test_client() + job_id = _post_job(client) + assert client.post( + f"/agent/jobs/{job_id}/claim", json={"worker_wallet": "worker"} + ).status_code == 200 + with sqlite3.connect(db_path) as conn: + conn.execute("UPDATE agent_jobs SET expires_at = 1 WHERE job_id = ?", (job_id,)) + + resp = client.post( + f"/agent/jobs/{job_id}/deliver", + json={"worker_wallet": "worker", "result_summary": "too late"}, + ) + assert resp.status_code == 410 + assert _job_row(db_path, job_id)["status"] == "expired" + assert _balance(db_path, "agent_escrow") == 0 + assert _balance(db_path, "poster") == 5_000_000 + + +def test_dispute_response_only_names_routes_that_exist(tmp_path): + app, _ = _make_app(tmp_path) + client = app.test_client() + job_id = _post_job(client) + assert client.post( + f"/agent/jobs/{job_id}/claim", json={"worker_wallet": "worker"} + ).status_code == 200 + assert client.post( + f"/agent/jobs/{job_id}/deliver", + json={"worker_wallet": "worker", "result_summary": "done"}, + ).status_code == 200 + + message = client.post( + f"/agent/jobs/{job_id}/dispute", + json={"poster_wallet": "poster", "reason": "not what I asked for"}, + ).get_json()["message"] + + # The old text promised an admin refund; no admin route is registered. + rules = {str(rule.rule) for rule in app.url_map.iter_rules()} + assert "admin" not in message.lower() + assert "/agent/jobs//deliver" in rules + assert "/agent/jobs//cancel" in rules diff --git a/wallet/coinbase_wallet.py b/wallet/coinbase_wallet.py index fb1bb1cba..065df64ab 100644 --- a/wallet/coinbase_wallet.py +++ b/wallet/coinbase_wallet.py @@ -224,13 +224,20 @@ def coinbase_create(args): def coinbase_show(args): - """Show Coinbase Base wallet info.""" + """Show Coinbase Base wallet info. + + Exit codes: + - 0: Success (balance fetched) + - 1: Usage error (no wallet found) + - 2: Network error (cannot reach node) + - 3: Invalid response (malformed balance data) + """ wallet = _load_coinbase_wallet() if not wallet: print(f"\n {YELLOW}No Coinbase wallet found.{NC}") print(f" Create one: clawrtc wallet coinbase create") - print(f" Or link: clawrtc wallet coinbase link 0xYourAddress\n") - return + print(f" Or link: clawrtc wallet coinbase link 0xYourAddress\n", file=sys.stderr) + sys.exit(1) print(f"\n {GREEN}{BOLD}Coinbase Base Wallet{NC}") print(f" {GREEN}Address:{NC} {BOLD}{wallet['address']}{NC}") @@ -241,13 +248,20 @@ def coinbase_show(args): balance, error = _get_wallet_balance_from_node(wallet["address"]) if error: - print(f" {YELLOW}Unable to fetch balance:{NC} {error}") + print(f" {RED}Error: Unable to fetch balance{NC}", file=sys.stderr) + print(f" {error}", file=sys.stderr) print(f" {DIM}Troubleshooting:{NC}") print(f" - Verify internet access and DNS resolution") - print(f" - Check RustChain node availability at {NODE_URL}") + print(f" - Check RustChain node availability at {NODE_URL}", file=sys.stderr) + # Distinguish error types by message + if "Network unreachable" in error or "Request failed" in error: + sys.exit(2) # Network error + else: + sys.exit(3) # Invalid response/balance format else: print(f" {DIM}Balance:{NC} {GREEN}{balance:.8f} RTC{NC}") print() + sys.exit(0) # Success def coinbase_link(args): diff --git a/wallet/tests/test_wallet_balance_error_handling.py b/wallet/tests/test_wallet_balance_error_handling.py new file mode 100644 index 000000000..40c23139a --- /dev/null +++ b/wallet/tests/test_wallet_balance_error_handling.py @@ -0,0 +1,163 @@ +""" +Test suite for wallet balance error handling (Rustchain#7889). + +Tests verify that all error paths in coinbase_show produce: +1. Clear error messages on stderr +2. Distinct non-zero exit codes (2=network, 3=invalid response) +3. No silent failures or misleading success +""" + +import pytest +import sys +import json +from unittest.mock import patch, MagicMock +from io import StringIO + +# Import the module under test +sys.path.insert(0, ".") +from coinbase_wallet import ( + coinbase_show, + _fetch_with_retry, + _get_wallet_balance_from_node, +) + + +class TestFetchWithRetry: + """Test _fetch_with_retry error handling.""" + + def test_network_error_returns_error_message(self): + """Network error should return None and error message.""" + with patch("coinbase_wallet.requests.get") as mock_get: + mock_get.side_effect = Exception("Connection refused") + payload, error = _fetch_with_retry("http://invalid.local/api") + assert payload is None + assert error is not None + assert "Network unreachable" in error or "failed" in error.lower() + + def test_http_error_returns_status_code(self): + """HTTP error (e.g., 500) should return error with status.""" + with patch("coinbase_wallet.requests.get") as mock_get: + mock_resp = MagicMock() + mock_resp.status_code = 500 + mock_resp.raise_for_status.side_effect = Exception("HTTP 500") + mock_get.return_value = mock_resp + + payload, error = _fetch_with_retry("http://localhost/api") + assert payload is None + assert error is not None + + +class TestGetWalletBalance: + """Test _get_wallet_balance_from_node error handling.""" + + def test_network_error(self): + """Network error should propagate.""" + with patch("coinbase_wallet._fetch_with_retry") as mock_fetch: + mock_fetch.return_value = (None, "Network unreachable: DNS resolution failed") + balance, error = _get_wallet_balance_from_node("0xtest") + assert balance is None + assert "Network unreachable" in error + + def test_invalid_balance_format(self): + """Invalid balance data should produce error.""" + with patch("coinbase_wallet._fetch_with_retry") as mock_fetch: + mock_fetch.return_value = ({"wrong_field": "value"}, None) + balance, error = _get_wallet_balance_from_node("0xtest") + assert balance is None + assert "Invalid balance format" in error + + def test_balance_not_float_convertible(self): + """Non-numeric balance should produce error.""" + with patch("coinbase_wallet._fetch_with_retry") as mock_fetch: + mock_fetch.return_value = ({"balance": "not_a_number"}, None) + balance, error = _get_wallet_balance_from_node("0xtest") + assert balance is None + assert "Invalid balance format" in error + + def test_successful_balance_fetch(self): + """Successful balance fetch should return float.""" + with patch("coinbase_wallet._fetch_with_retry") as mock_fetch: + mock_fetch.return_value = ({"balance": 42.5}, None) + balance, error = _get_wallet_balance_from_node("0xtest") + assert balance == 42.5 + assert error is None + + def test_alternative_field_names(self): + """Alternative field names (amount_rtc, amount) should work.""" + with patch("coinbase_wallet._fetch_with_retry") as mock_fetch: + mock_fetch.return_value = ({"amount_rtc": 99.99}, None) + balance, error = _get_wallet_balance_from_node("0xtest") + assert balance == 99.99 + assert error is None + + +class TestCoinbaseShowExitCodes: + """Test that coinbase_show exits with correct codes.""" + + def test_no_wallet_found_exits_1(self): + """No wallet file should exit(1).""" + mock_args = MagicMock() + with patch("coinbase_wallet._load_coinbase_wallet") as mock_load: + mock_load.return_value = None + with pytest.raises(SystemExit) as exc_info: + coinbase_show(mock_args) + assert exc_info.value.code == 1 + + def test_network_error_exits_2(self): + """Network error should exit(2).""" + mock_args = MagicMock() + mock_wallet = {"address": "0xtest"} + with patch("coinbase_wallet._load_coinbase_wallet") as mock_load: + mock_load.return_value = mock_wallet + with patch("coinbase_wallet._get_wallet_balance_from_node") as mock_balance: + mock_balance.return_value = (None, "Network unreachable: DNS resolution failed for rustchain.org") + with pytest.raises(SystemExit) as exc_info: + coinbase_show(mock_args) + assert exc_info.value.code == 2 + + def test_invalid_response_exits_3(self): + """Invalid balance response should exit(3).""" + mock_args = MagicMock() + mock_wallet = {"address": "0xtest"} + with patch("coinbase_wallet._load_coinbase_wallet") as mock_load: + mock_load.return_value = mock_wallet + with patch("coinbase_wallet._get_wallet_balance_from_node") as mock_balance: + mock_balance.return_value = (None, "Invalid balance format") + with pytest.raises(SystemExit) as exc_info: + coinbase_show(mock_args) + assert exc_info.value.code == 3 + + def test_success_exits_0(self): + """Successful balance fetch should exit(0).""" + mock_args = MagicMock() + mock_wallet = {"address": "0xtest"} + with patch("coinbase_wallet._load_coinbase_wallet") as mock_load: + mock_load.return_value = mock_wallet + with patch("coinbase_wallet._get_wallet_balance_from_node") as mock_balance: + mock_balance.return_value = (123.456, None) + with patch("builtins.print") as mock_print: + with pytest.raises(SystemExit) as exc_info: + coinbase_show(mock_args) + assert exc_info.value.code == 0 + + +class TestErrorMessagesOnStderr: + """Test that errors go to stderr, not stdout.""" + + def test_network_error_to_stderr(self): + """Network errors should print to stderr.""" + mock_args = MagicMock() + mock_wallet = {"address": "0xtest"} + with patch("coinbase_wallet._load_coinbase_wallet") as mock_load: + mock_load.return_value = mock_wallet + with patch("coinbase_wallet._get_wallet_balance_from_node") as mock_balance: + mock_balance.return_value = (None, "Network unreachable: Connection refused") + with patch("sys.stderr", new_callable=StringIO) as mock_stderr: + with pytest.raises(SystemExit): + coinbase_show(mock_args) + stderr_output = mock_stderr.getvalue() + assert "Error" in stderr_output or "unreachable" in stderr_output + + +if __name__ == "__main__": + pytest.main([__file__, "-v"])