diff --git a/scripts/internal/remote-sync.mjs b/scripts/internal/remote-sync.mjs index 4bb4d960..7ed35e27 100755 --- a/scripts/internal/remote-sync.mjs +++ b/scripts/internal/remote-sync.mjs @@ -2,7 +2,7 @@ import { createHash } from "node:crypto"; import { constants } from "node:fs"; import { spawn } from "node:child_process"; -import { appendFile, lstat, mkdir, open, readFile, rename, rmdir, stat, writeFile } from "node:fs/promises"; +import { appendFile, lstat, mkdir, open, readFile, rename, rm, rmdir, stat, writeFile } from "node:fs/promises"; import { basename, dirname, join, resolve, sep } from "node:path"; import process from "node:process"; import { realpathSync } from "node:fs"; @@ -142,6 +142,111 @@ async function recordCycleSuccess(team, at, writeFileCall = writeFile) { } catch { /* best-effort: never fail a working cycle over its own bookkeeping */ } } +// A refusal the caller can act on, recorded where something can read it (#773). +// +// A remote may answer a write with a status meaning "the caller must do +// something" rather than "try again later". The engine used to treat that as a +// transport failure: not retryable, so it left the loop, and the process +// exited. `status` then said "engine stopped -- run: remote.sh sync start", +// which invites the one action that cannot work; starting it again produces +// the same refusal and the same exit, for as long as the server's answer +// stands. +// +// THE REASON WAS NEVER MISSING. `event()` writes `fatal` to stdout and +// `sync start` captures it into `run/remote-sync..log`, with a +// timestamp, at a known path. What was missing is a place to READ it from: +// `status` opens a pidfile and the cycle stamp, and nothing an agent consults +// mentions the log. So this is one more fact beside the cycle stamp, not a new +// mechanism -- the same move #760 made. +// +// WHAT IS STORED, AND WHAT IS DELIBERATELY NOT. +// +// Stored: the status, the code, the time, and the host from the endpoint the +// config already holds. Verbatim, as the server said them. +// +// Not stored: any sentence about what the refusal MEANS or what to do about +// it. This engine talks to *a* remote -- self-hosted, someone else's, or a +// service -- and it cannot know why a particular one refused. A sentence it +// invents is wrong for some server. Interpretation belongs to whoever operates +// that server, and the host is there so a reader knows who that is. +function refusalPath(team) { + const connectionRoot = process.env.AGMSG_SYNC_CONNECTION_DIR ?? process.env.SKILL_DIR; + if (!connectionRoot) throw new Error("sync connection root is unavailable"); + return join(connectionRoot, "run", `remote-sync.${team}.refusal.json`); +} + +/** + * A refusal is a 4xx the retry policy does not cover. + * + * BY CLASS, NOT BY NUMBER. There is one such status in use today, and it is + * deliberately not named anywhere in this file — not in the code and not in + * this comment, which a check enforces. A server may refuse for reasons this + * protocol never enumerates, and every one of them is "the server decided, and + * said so" rather than "ask again later"; naming one would invite the next + * reader to special-case it, and the one after that to add a sentence about + * what it means. + * + * Everything in the 4xx range that the retry policy does not cover lands here, + * for that same reason. 5xx stays a transport failure and the retryable + * statuses stay retryable, because `isRetryable` is asked first. + * + * 5xx stays a transport failure, and the retryable 4xx (408, 429) stay + * retryable, because `isRetryable` is asked first. + */ +/** The host of an endpoint, or null when it cannot be read as a URL. */ +function hostOf(endpoint) { + try { + return new URL(endpoint).host; + } catch { + return null; + } +} + +export function isRefusal(error) { + const status = error?.status; + if (typeof status !== "number") return false; + if (isRetryable(error)) return false; + return status >= 400 && status < 500; +} + +/** + * Write the refusal down. Best-effort, like the cycle stamp and for the same + * reason: bookkeeping must never be the thing that takes syncing down. + */ +async function recordRefusal(team, fact, writeFileCall = writeFile) { + try { + await writeFileCall(refusalPath(team), `${JSON.stringify({ + type: "sync_refusal", ...fact, + })}\n`); + } catch { /* best-effort */ } +} + +/** + * Forget it, because a cycle has since succeeded. + * + * A refusal that outlives its truth is worse than no record: `status` would + * keep reporting a server decision that has been reversed, and the operator + * would keep acting on it. + * + * BEST-EFFORT, AND NOT WHAT MAKES THAT TRUE. This removal can fail — an + * unwritable run directory, a permission change, a crash between the two + * writes — and it is deliberately not retried or escalated, for the same + * reason `recordCycleSuccess` is not: bookkeeping must never take down a cycle + * that worked. + * + * What makes the guarantee hold is on the READING side: `remote.sh` compares + * the record to the last successful cycle and reports nothing older. Deleting + * can fail; comparing cannot. An earlier version of this comment said the two + * facts were written in the same place and so could never disagree — they are, + * and they still could, because one of the two writes is allowed to fail + * (raised in review). + */ +async function clearRefusal(team, rmCall = rm) { + try { + await rmCall(refusalPath(team), { force: true }); + } catch { /* best-effort */ } +} + // What actually went wrong, when the thing that threw is a wrapper. // // `fetch` rejects with a bare `TypeError: fetch failed` whose own `code` is @@ -2722,6 +2827,9 @@ export async function runLoop(config, options, dependencies = {}) { const eventCall = dependencies.eventCall ?? event; const isRetryableCall = dependencies.isRetryableCall ?? isRetryable; const recordCycleCall = dependencies.recordCycleCall ?? recordCycleSuccess; + const clearRefusalCall = dependencies.clearRefusalCall ?? clearRefusal; + const recordRefusalCall = dependencies.recordRefusalCall ?? recordRefusal; + const isRefusalCall = dependencies.isRefusalCall ?? isRefusal; const nowCall = dependencies.nowCall ?? (() => new Date().toISOString()); // An explicit --limit is a ceiling for BOTH push and pull (request size / @@ -2751,6 +2859,11 @@ export async function runLoop(config, options, dependencies = {}) { // success. Same shape as the `cycle.error` logging below. try { await recordCycleCall(config.local_team, nowCall()); + // A success outdates any refusal on record. Cleared HERE, beside the + // stamp, so a reader that never looks at the timestamps still sees the + // right thing most of the time — but the guarantee is the reader's + // comparison, not this line, because this line may fail. + await clearRefusalCall(config.local_team); } catch { /* bookkeeping is best-effort; the cycle already succeeded */ } catchUp = result?.pushSaturated === true; // catch-up removes the wait ONLY between successful, progress-making @@ -2763,6 +2876,40 @@ export async function runLoop(config, options, dependencies = {}) { try { await eventCall("cycle.error", { message: error.message, ...causeOf(error) }); } catch { /* logging is best-effort */ } + + // A REFUSAL DOES NOT LEAVE THE LOOP (#773). + // + // It is not retryable — asking again does not change a decision — and it + // is not a transport failure either: the server has answered, and said + // what it decided. Exiting on it turns a recoverable condition into a + // dead process whose `status` line recommends starting it again, which + // reproduces the refusal and the exit. + // + // And the answer CAN change, out of band: someone pays, a quota resets, + // an operator fixes a setting. Staying up means the next cycle recovers + // with nobody typing anything. So this backs off to the longest interval + // and keeps asking quietly, and the failure count is not advanced — + // a refusal is not evidence that the transport is degrading. + if (isRefusalCall(error)) { + try { + await recordRefusalCall(config.local_team, { + status: error?.status ?? null, + code: error?.code ?? null, + at: nowCall(), + // From the config the engine already holds. Not an interpretation: + // it says WHERE the operator of that server would be reached. + endpoint_host: hostOf(config.endpoint), + }); + } catch { /* best-effort */ } + try { + await eventCall("cycle.refused", { + status: error?.status ?? null, code: error?.code ?? null, + }); + } catch { /* logging is best-effort */ } + await sleepCall(MAX_BACKOFF_MS); + continue; + } + if (!isRetryableCall(error)) throw error; consecutiveFailures += 1; const backoffMs = Math.min(MAX_BACKOFF_MS, BASE_BACKOFF_MS * 2 ** (consecutiveFailures - 1)); diff --git a/scripts/remote.sh b/scripts/remote.sh index c5a0f2aa..8e92d891 100644 --- a/scripts/remote.sh +++ b/scripts/remote.sh @@ -1358,6 +1358,57 @@ _remote_sync_engine_pidfile() { printf '%s' "$CONNECTION_ROOT/run/remote-sync.$1 # Written by the engine after a cycle completes (#756). Derived here the same way # the pidfile is, because it has the same lifetime and the same owner. _remote_sync_engine_cycle_stamp() { printf '%s' "$CONNECTION_ROOT/run/remote-sync.$1.cycles.json"; } +# Written by the engine when the server REFUSED, and removed by the engine on +# the next successful cycle (#773). Same directory, same derivation, same +# lifetime as the two above. +# +# The reason for its existence is that the reason was already on disk and +# unread: `event()` writes a `fatal` line into the run log with a timestamp, +# and nothing `status` opens has ever mentioned it. This is a place to read +# from, not a new place to write. +_remote_sync_engine_refusal() { printf '%s' "$CONNECTION_ROOT/run/remote-sync.$1.refusal.json"; } + +# The refusal record, or nothing when a cycle has SINCE succeeded. +# +# THE READER DOES NOT TRUST THE DELETE. The engine removes this file after a +# successful cycle, and that removal is best-effort — an unwritable run +# directory, a permission change, a crash between the two writes. If the +# removal is the only thing standing between a reversed decision and the +# operator, then a failed removal reports a refusal that is no longer true, +# for ever, on every `status` (raised in review). +# +# So the file is not the answer; the file COMPARED TO THE LAST SUCCESSFUL +# CYCLE is. Deleting can fail. Comparing cannot. That makes this the stronger +# of the two guarantees — "it does not lie even when the delete failed" rather +# than "the delete is made certain" — and it is why the clear stays +# best-effort in the engine rather than growing a retry. +# +# The comparison is lexicographic on the two timestamps, which is exact +# because BOTH are written by the same engine in the same format (an ISO-8601 +# UTC instant). It is not a general date comparison and must not be reused as +# one. +# +# With no cycle stamp there is nothing to compare against, and the refusal is +# reported: an engine that has refused and never succeeded is exactly the case +# the record exists for. +_remote_sync_engine_refusal_current() { + local team="$1" file stamp last_cycle refused_at + file="$(_remote_sync_engine_refusal "$team")" + [ -f "$file" ] || return 0 + # Quiet on purpose: an unreadable record is ABSENT, and saying so on stderr + # would put a parse error into the same stream a caller reads the JSON from + # (measured — it turned the "unreadable reads as absent" case red). + refused_at="$(_remote_read_config_field "$file" '$.at' 2>/dev/null)" + [ -n "$refused_at" ] && [ "$refused_at" != "null" ] || return 0 + stamp="$(_remote_sync_engine_cycle_stamp "$team")" + if [ -f "$stamp" ]; then + last_cycle="$(_remote_read_config_field "$stamp" '$.last_success_at' 2>/dev/null)" + if [ -n "$last_cycle" ] && [ "$last_cycle" != "null" ] && [[ "$last_cycle" > "$refused_at" ]]; then + return 0 + fi + fi + cat "$file" 2>/dev/null || true +} # _remote_holds_current_key -> 0 when this machine holds the identity # for the team's CURRENT epoch, 1 otherwise. @@ -2133,6 +2184,34 @@ _remote_status_one() { echo " cycles: last successful sync $last_cycle" fi fi + # WHY IT IS NOT SYNCING, when the server has said so (#773). + # + # Printed for a running engine AND a stopped one: a refusal recorded by an + # engine that has since been stopped is still the last thing the server said, + # and the operator asking "why" is owed it either way. + # + # REPEATED, NOT INTERPRETED. The status and the code are the server's words. + # This client talks to *a* remote — self-hosted, someone else's, or a service + # — and cannot know what a particular one means by a particular code. A + # sentence invented here is wrong for some server. What it may add is where + # the operator of that server would be reached, which is the host the + # binding already names. + local refusal_raw refusal_status refusal_code refusal_at refusal_host + # Through the currency check, not straight off the file — see the helper. + refusal_raw="$(_remote_sync_engine_refusal_current "$team")" + if [ -n "$refusal_raw" ]; then + local refusal_file="$(_remote_sync_engine_refusal "$team")" + refusal_status="$(_remote_read_config_field "$refusal_file" '$.status')" + refusal_code="$(_remote_read_config_field "$refusal_file" '$.code')" + refusal_at="$(_remote_read_config_field "$refusal_file" '$.at')" + refusal_host="$(_remote_read_config_field "$refusal_file" '$.endpoint_host')" + if [ -n "$refusal_status" ] && [ "$refusal_status" != "null" ]; then + echo " refused: the server answered $refusal_status${refusal_code:+ $refusal_code} at $refusal_at" + if [ -n "$refusal_host" ] && [ "$refusal_host" != "null" ]; then + echo " that server is $refusal_host — what the answer means is theirs to say" + fi + fi + fi if [ "$binding_cipher" = "age-v1" ]; then if [ -n "$key_id" ] && [ "$key_id" != "null" ]; then echo " encryption: age-v1, key present" @@ -2202,8 +2281,22 @@ _remote_status_json_one() { _remote_config_shape_ok "$raw" || return 2 IFS=$'\t' read -r engine_state engine_pid < <(_remote_sync_engine_status "$team") - printf '%s' "$raw" | python3 -c ' -import json, sys + # THE SURFACE AN AGENT READS (#773). + # + # Someone asks their agent "why isn't this syncing?" and the agent has to be + # able to answer. `/agmsg remote status` runs this with `--json`, and it is + # the only thing an agent consults about the engine — so the refusal has to + # be here, not only in a line a human reads. + # + # Passed as a whole file rather than as parsed fields: whatever the engine + # recorded is what the reader gets, and adding a field there does not need a + # change here. + # Same currency check as the human line, from the same helper: the two must + # not be able to disagree about whether a refusal still stands. + local refusal_raw + refusal_raw="$(_remote_sync_engine_refusal_current "$team")" + printf '%s' "$raw" | REFUSAL_JSON="$refusal_raw" python3 -c ' +import json, os, sys team, engine_state, engine_pid_text = sys.argv[1:4] try: cfg = json.loads(sys.stdin.read()) @@ -2219,6 +2312,12 @@ engine_pid = int(engine_pid_text) if engine_pid_text else None if state == "disconnected": engine_state = "stopped" engine_pid = None +try: + refusal = json.loads(os.environ.get("REFUSAL_JSON") or "null") +except Exception: + refusal = None +if not isinstance(refusal, dict): + refusal = None print(json.dumps({ "local_team": team, "endpoint": binding.get("endpoint"), @@ -2228,6 +2327,10 @@ print(json.dumps({ "state": state, "engine_state": engine_state, "engine_pid": engine_pid, + # null when the server has refused nothing, or when what was recorded + # cannot be read. An unreadable record is reported as absent rather than + # guessed at; the human line above is derived from the same file. + "refusal": refusal, }, sort_keys=True)) ' "$team" "$engine_state" "$engine_pid" } diff --git a/tests/remote_sync_engine.test.mjs b/tests/remote_sync_engine.test.mjs index 3d3c1b5f..d57c6b2b 100644 --- a/tests/remote_sync_engine.test.mjs +++ b/tests/remote_sync_engine.test.mjs @@ -18,6 +18,7 @@ import { exportAgeSnapshot, isRetryable, initialAgeSnapshot, + isRefusal, runLoop, loadConfig, nextLocalAgeSnapshot, @@ -3388,3 +3389,105 @@ test("an error code is read from the protocol shape, and from the edge's for now assert.equal(errorCode({}), "unknown-error"); assert.equal(errorCode(undefined), "unknown-error"); }); + +// ── a refusal the caller can act on (#773) ────────────────────────────────── +// +// The engine used to leave the loop on one: not retryable, so it threw, main() +// rejected, and the process exited. `status` then said "engine stopped — run: +// remote.sh sync start", which invites the one action that cannot work. + +test("isRefusal: a 4xx the retry policy does not cover, by class and not by number", () => { + // The status in use today is deliberately not named in the engine, so it is + // not named here either — what is asserted is the CLASS. A server may refuse + // for reasons this protocol never enumerates. + for (const status of [400, 401, 402, 403, 409, 422, 451]) { + assert.equal(isRefusal({ status }), true, `${status} should be a refusal`); + } + // Retryable 4xx stay retryable: isRetryable is asked first. + for (const status of [408, 429]) { + assert.equal(isRefusal({ status }), false, `${status} is retryable, not refused`); + } + // 5xx is a transport failure, not a decision. + for (const status of [500, 502, 503, 504]) { + assert.equal(isRefusal({ status }), false, `${status} is transport, not refused`); + } + // An error carrying no status at all — a socket failure — is neither. + assert.equal(isRefusal(new Error("fetch failed")), false); + assert.equal(isRefusal({ retryable: true, status: 402 }), false); +}); + +test("runLoop: a refusal is recorded and does NOT leave the loop", async () => { + const recorded = []; + const sleeps = []; + let i = 0; + // An endpoint the host can actually be read from: the recorded host is the + // one fact here that says WHERE the operator of that server would be + // reached, and a fixture without one would let the assertion pass on null. + const refusedConfig = { ...config, endpoint: "https://sync.example.test" }; + await assert.rejects(() => runLoop(refusedConfig, {}, { + cycleCall: async () => { + i += 1; + if (i <= 2) { const refused = new Error("HTTP 402 payment_required"); refused.status = 402; refused.code = "payment_required"; throw refused; } + // Only a DIFFERENT, non-retryable error ends the loop — which is how + // this test can end at all. If a refusal exited, i would never reach 3. + const stop = new Error("stop"); stop.retryable = false; throw stop; + }, + isRetryableCall: (error) => error.retryable === true, + isRefusalCall: (error) => typeof error.status === "number" && error.status >= 400 && error.status < 500, + recordRefusalCall: async (team, fact) => { recorded.push({ team, ...fact }); }, + clearRefusalCall: async () => {}, + nowCall: () => "2026-08-14T00:00:00Z", + sleepCall: async (ms) => { sleeps.push(ms); }, + eventCall: async () => {}, + }), /stop/); + + // It came back for a second cycle after the first refusal, and a third. + assert.equal(i, 3); + // Both refusals were written down, verbatim, with the host from the config. + assert.equal(recorded.length, 2); + assert.equal(recorded[0].status, 402); + assert.equal(recorded[0].code, "payment_required"); + assert.equal(recorded[0].at, "2026-08-14T00:00:00Z"); + assert.equal(recorded[0].endpoint_host, "sync.example.test"); + // Backed off to the longest interval rather than hammering, and the failure + // count was not advanced — a refusal is not evidence the transport is + // degrading, so it must not shorten anything else's backoff. + assert.deepEqual(sleeps, [60000, 60000]); +}); + +test("runLoop: a successful cycle clears a refusal that is no longer true", async () => { + // A record that outlives its truth is worse than no record: `status` would + // keep reporting a decision the server has since reversed. + const cleared = []; + let i = 0; + await assert.rejects(() => runLoop(config, {}, { + cycleCall: async () => { + i += 1; + if (i === 1) { const refused = new Error("refused"); refused.status = 402; throw refused; } + if (i === 2) return { pushSaturated: false }; + const stop = new Error("stop"); stop.retryable = false; throw stop; + }, + isRetryableCall: (error) => error.retryable === true, + isRefusalCall: (error) => typeof error.status === "number" && error.status >= 400 && error.status < 500, + recordRefusalCall: async () => {}, + clearRefusalCall: async (team) => { cleared.push(team); }, + recordCycleCall: async () => {}, + nowCall: () => "2026-08-14T00:00:00Z", + sleepCall: async () => {}, + eventCall: async () => {}, + }), /stop/); + assert.deepEqual(cleared, [config.local_team]); +}); + +test("runLoop: a non-retryable error that is NOT a refusal still ends the loop", async () => { + // The negative control. Staying up for everything would turn a malformed + // config into an engine that spins forever saying nothing useful — exiting + // is right for that, and the refusal case is the exception, not the rule. + await assert.rejects(() => runLoop(config, {}, { + cycleCall: async () => { const bad = new Error("config is unreadable"); throw bad; }, + isRetryableCall: () => false, + isRefusalCall: () => false, + sleepCall: async () => {}, + eventCall: async () => {}, + }), /config is unreadable/); +}); diff --git a/tests/test_remote_refusal.bats b/tests/test_remote_refusal.bats new file mode 100644 index 00000000..0e458f04 --- /dev/null +++ b/tests/test_remote_refusal.bats @@ -0,0 +1,170 @@ +#!/usr/bin/env bats + +load test_helper + +# A refusal the operator could act on, recorded and readable (#773). +# +# The engine used to treat one as a transport failure: not retryable, so it +# left the loop, and the process exited. `status` then said "engine stopped — +# run: remote.sh sync start", which invites the one action that cannot work. +# +# The reason was never missing — `event()` writes it to the run log — it was +# unread. So what is tested here is the reading: a place `status` opens, and +# the JSON an agent consults, carrying what the server said and nothing this +# client invented. + +setup() { + setup_test_env + bash "$SCRIPTS/join.sh" testteam alice claude-code /tmp/project-a + + local cfg="$TEST_SKILL_DIR/teams/testteam/config.json" escaped updated + escaped="$(sed "s/'/''/g" "$cfg")" + updated="$(sqlite_mem " + SELECT json_set('$escaped', '\$.remote_binding', json_object( + 'endpoint', 'https://sync.example.test', + 'server_instance_id', '018f0000-0000-7000-8000-000000000001', + 'remote_team_id', '018f0000-0000-7000-8000-000000000002', + 'protocol_version', 1, + 'capabilities', json_object('write_allowed_ciphers', json_array('none')), + 'connected_at', '2026-07-30T00:00:00Z', + 'disconnected_at', null + ));")" + printf '%s\n' "$updated" > "$cfg" + mkdir -p "$TEST_SKILL_DIR/run" +} + +teardown() { teardown_test_env; } + +# What the engine writes when the server refuses. Written here rather than by +# running the engine: this file is about what READS it. +write_refusal() { + printf '%s\n' "{\"type\":\"sync_refusal\",\"status\":${1:-402},\"code\":\"${2:-payment_required}\",\"at\":\"2026-08-14T00:00:00Z\",\"endpoint_host\":\"sync.example.test\"}" \ + > "$TEST_SKILL_DIR/run/remote-sync.testteam.refusal.json" +} + +@test "status says what the server answered, and does not say what it meant" { + write_refusal 402 payment_required + run bash "$SCRIPTS/remote.sh" status testteam + [ "$status" -eq 0 ] + printf '%s' "$output" | grep -q 'refused: the server answered 402 payment_required' + # WHERE the operator of that server is, which the binding already knows. + printf '%s' "$output" | grep -q 'sync.example.test' + # And nothing this client invented about what it MEANS. A sentence here + # would be wrong for some server, and every one of these is a sentence only + # the operator of that server may write. + refute grep -qi 'subscri' <<<"$output" + refute grep -qi 'upgrade' <<<"$output" + refute grep -qi 'billing' <<<"$output" + refute grep -qi 'plan' <<<"$output" +} + +@test "status repeats a status this protocol never enumerated" { + # BY CLASS, NOT BY NUMBER. A self-hosted server may refuse for its own + # reasons with a code nothing here has heard of, and the answer must survive + # the trip unchanged. + write_refusal 451 tenant_suspended_by_operator + run bash "$SCRIPTS/remote.sh" status testteam + printf '%s' "$output" | grep -q '451 tenant_suspended_by_operator' +} + +@test "the agent's surface carries it, verbatim" { + # `/agmsg remote status` runs this. It is the only thing an agent consults + # about the engine, so a refusal that reached only the human line would + # leave "why isn't this syncing?" unanswerable. + write_refusal 402 payment_required + run bash "$SCRIPTS/remote.sh" status testteam --json + [ "$status" -eq 0 ] + local got + got="$(printf '%s' "$output" | python3 -c 'import json,sys; print(json.load(sys.stdin)["refusal"]["status"])')" + [ "$got" = "402" ] + got="$(printf '%s' "$output" | python3 -c 'import json,sys; print(json.load(sys.stdin)["refusal"]["code"])')" + [ "$got" = "payment_required" ] + got="$(printf '%s' "$output" | python3 -c 'import json,sys; print(json.load(sys.stdin)["refusal"]["endpoint_host"])')" + [ "$got" = "sync.example.test" ] +} + +@test "no refusal is null, not a missing key" { + # A consumer that has to tell "absent" from "unreadable" needs the key to be + # there either way. + run bash "$SCRIPTS/remote.sh" status testteam --json + [ "$status" -eq 0 ] + local got + got="$(printf '%s' "$output" | python3 -c 'import json,sys; d=json.load(sys.stdin); print("KEY" if "refusal" in d else "MISSING", d["refusal"])')" + [ "$got" = "KEY None" ] +} + +@test "an unreadable record reads as absent, not as a guess" { + printf '%s\n' 'not json at all' > "$TEST_SKILL_DIR/run/remote-sync.testteam.refusal.json" + run bash "$SCRIPTS/remote.sh" status testteam --json + [ "$status" -eq 0 ] + local got + got="$(printf '%s' "$output" | python3 -c 'import json,sys; print(json.load(sys.stdin)["refusal"])')" + [ "$got" = "None" ] +} + +@test "the engine carries no knowledge of any particular remote" { + # The requirement that is easiest to lose later, so it is a check rather + # than a note. `402` is the case that exists today and the engine classifies + # by CLASS — a 4xx the retry policy does not cover — so the number has no + # business being in there. + refute grep -n '402' "$SCRIPTS/internal/remote-sync.mjs" + refute grep -ni 'payment_required' "$SCRIPTS/internal/remote-sync.mjs" + # A positive control on the search itself: it has to be able to find + # something in that file, or these three prove nothing. + grep -q 'isRefusal' "$SCRIPTS/internal/remote-sync.mjs" +} + +# ── a refusal that is no longer true ──────────────────────────────────────── +# +# The engine removes the record after a successful cycle, best-effort. If that +# removal fails — an unwritable run directory, a permission change, a crash +# between the two writes — the file stays. Nothing else stood between it and +# the operator, so `status` reported a reversed decision for ever (raised in +# review, and it contradicted this PR's own stated standard). +# +# The reader now compares the record to the last successful cycle. Deleting can +# fail; comparing cannot. + +write_cycle_stamp() { + printf '%s\n' "{\"type\":\"sync_cycle_stamp\",\"first_success_at\":\"$1\",\"last_success_at\":\"$1\"}" \ + > "$TEST_SKILL_DIR/run/remote-sync.testteam.cycles.json" +} + +@test "a refusal older than the last successful cycle is not reported, even if the file remains" { + write_refusal 402 payment_required # recorded at 2026-08-14T00:00:00Z + write_cycle_stamp "2026-08-14T01:00:00Z" # and a cycle succeeded after it + # The file is still there — this is the failed-delete case, made deliberate. + [ -f "$TEST_SKILL_DIR/run/remote-sync.testteam.refusal.json" ] + + run bash "$SCRIPTS/remote.sh" status testteam + [ "$status" -eq 0 ] + refute grep -q 'refused:' <<<"$output" + + run bash "$SCRIPTS/remote.sh" status testteam --json + local got + got="$(printf '%s' "$output" | python3 -c 'import json,sys; print(json.load(sys.stdin)["refusal"])')" + [ "$got" = "None" ] +} + +@test "a refusal newer than the last successful cycle is still reported" { + # The other side of the comparison. Without this the check could be satisfied + # by never reporting anything, which is the failure mode of every filter. + write_cycle_stamp "2026-08-13T00:00:00Z" + write_refusal 402 payment_required # recorded a day later + run bash "$SCRIPTS/remote.sh" status testteam + printf '%s' "$output" | grep -q 'refused: the server answered 402' + + run bash "$SCRIPTS/remote.sh" status testteam --json + local got + got="$(printf '%s' "$output" | python3 -c 'import json,sys; print(json.load(sys.stdin)["refusal"]["status"])')" + [ "$got" = "402" ] +} + +@test "with no cycle stamp at all, the refusal is reported" { + # An engine that has refused and never succeeded is exactly the case the + # record exists for. Nothing to compare against must not mean "assume stale". + write_refusal 402 payment_required + [ ! -f "$TEST_SKILL_DIR/run/remote-sync.testteam.cycles.json" ] + run bash "$SCRIPTS/remote.sh" status testteam + printf '%s' "$output" | grep -q 'refused: the server answered 402' +}