Skip to content

Commit 97f9b78

Browse files
committed
fix(azure): recover repeated worker executions
1 parent a6dce01 commit 97f9b78

6 files changed

Lines changed: 140 additions & 8 deletions

bin/fm-azure-worker-provider.py

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1595,6 +1595,32 @@ def upload_json_blob(
15951595
and hashlib.sha256(current_payload).hexdigest() == digest
15961596
):
15971597
return digest
1598+
# Sequential executes deliberately reuse the assignment's
1599+
# fixed request/result blob names. The worker record can
1600+
# still carry the ETag from the prior execute, so adopt the
1601+
# observed same-assignment blob and retry one CAS rather
1602+
# than permanently wedging retained-disk recovery. Another
1603+
# assignment never passes the complete expected tag subset,
1604+
# and a concurrent writer loses the fresh If-Match.
1605+
current_metadata = current.get("metadata") or {}
1606+
expected_metadata = tags_to_metadata(tags)
1607+
if overwrite and all(
1608+
current_metadata.get(key) == item
1609+
for key, item in expected_metadata.items()
1610+
):
1611+
retry_args = list(upload_args)
1612+
match_index = retry_args.index("--if-match")
1613+
retry_args[match_index + 1] = current_etag
1614+
_, retry_rc, retry_stderr = az(
1615+
controller, retry_args, check=False
1616+
)
1617+
if retry_rc == 0:
1618+
return digest
1619+
raise ProviderError(
1620+
"conditionally retried worker staging upload failed: {}".format(
1621+
retry_stderr
1622+
)
1623+
)
15981624
finally:
15991625
with contextlib.suppress(FileNotFoundError):
16001626
Path(current_path).unlink()
@@ -2546,6 +2572,24 @@ def mutate_deallocate(controller, action):
25462572
return final
25472573

25482574

2575+
def service_cancel_allows_missing_task_command(action):
2576+
proof = action.get("service_cancel_proof")
2577+
if not isinstance(proof, dict):
2578+
return False
2579+
unsigned = dict(proof)
2580+
supplied = unsigned.pop("proof_digest", None)
2581+
bindings = action.get("bindings") or {}
2582+
return (
2583+
proof.get("schema") == "fm.worker-service-cancel/v1"
2584+
and proof.get("verdict") == "cancelled-before-execution"
2585+
and supplied == hashlib.sha256(canonical_bytes(unsigned)).hexdigest()
2586+
and proof.get("task") == bindings.get("task")
2587+
and proof.get("task_generation") == bindings.get("task_generation")
2588+
and proof.get("assignment_generation") == bindings.get("assignment_generation")
2589+
and proof.get("cloud_instance_id") == action.get("cloud_instance_id")
2590+
)
2591+
2592+
25492593
def mutate_delete_compute(controller, action):
25502594
snapshot = inventory(controller, include_metrics=False)
25512595
worker = worker_by_slot(snapshot, action["slot"])
@@ -2566,7 +2610,11 @@ def mutate_delete_compute(controller, action):
25662610
or not cleanup_marker(container, EXECUTE_ABANDON_MARKER, retired_execute_key)
25672611
):
25682612
raise ProviderError("retired-execute custody marker is not exact")
2569-
if task_command_missing and retired_execute_key is None:
2613+
if (
2614+
task_command_missing
2615+
and retired_execute_key is None
2616+
and not service_cancel_allows_missing_task_command(action)
2617+
):
25702618
raise ProviderError(
25712619
"missing task-command has no exact retired-execute custody proof"
25722620
)
@@ -3236,6 +3284,11 @@ def mutate_execute(controller, action):
32363284
if disposition in (EXECUTE_DISPOSITION_TERMINAL, EXECUTE_DISPOSITION_RECOVERED):
32373285
if disposition == EXECUTE_DISPOSITION_RECOVERED:
32383286
persist_execute_result(controller, action, names, tags, recovered)
3287+
worker = worker_by_slot(
3288+
inventory(controller, include_metrics=False), action["slot"]
3289+
)
3290+
if worker is None:
3291+
raise ProviderError("execution result persistence lost its exact worker")
32393292
return worker, recovered
32403293
if disposition != EXECUTE_DISPOSITION_SUBMIT:
32413294
raise ProviderError("worker execution disposition is unsupported")
@@ -3336,7 +3389,10 @@ def mutate_execute(controller, action):
33363389
if execution is None:
33373390
raise ProviderError("private worker execution returned no exact result")
33383391
persist_execute_result(controller, action, names, tags, execution)
3339-
return worker_by_slot(inventory(controller, include_metrics=False), action["slot"]), execution
3392+
worker = worker_by_slot(inventory(controller, include_metrics=False), action["slot"])
3393+
if worker is None:
3394+
raise ProviderError("execution result persistence lost its exact worker")
3395+
return worker, execution
33403396

33413397

33423398
def mutate_steer(controller, action):

bin/fm-secondmate-cloud-monitor.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,9 @@
235235
# Child model/effort ride the child's spawn argv, so the monitor is stricter
236236
# than the runner: a bounded shell-safe token or nothing.
237237
SAFE_OPTION = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$")
238+
SAFE_MODEL_OPTION = re.compile(
239+
r"^[A-Za-z0-9][A-Za-z0-9._-]{0,63}(?:/[A-Za-z0-9][A-Za-z0-9._-]{0,63})?$"
240+
)
238241
# The cloud lane runs exactly one runtime: fm-spawn refuses every other
239242
# harness under cloud placement and forces this one itself.
240243
CLOUD_CHILD_HARNESS = "pi"
@@ -971,9 +974,14 @@ def check_child_request(message, task, generation, assignment):
971974
# fm-secondmate-session.py refuses leading-dash inbox text, and the
972975
# same reason child_model/child_effort carry a strict charset here.
973976
return "request brief begins with '-' and cannot ride the pi argv"
974-
for key in CHILD_REQUEST_OPTIONAL:
975-
if key in message and not SAFE_OPTION.fullmatch(message[key]):
976-
return "request {} is malformed".format(key)
977+
if "child_model" in message and not SAFE_MODEL_OPTION.fullmatch(
978+
message["child_model"]
979+
):
980+
return "request child_model is malformed"
981+
if "child_effort" in message and not SAFE_OPTION.fullmatch(
982+
message["child_effort"]
983+
):
984+
return "request child_effort is malformed"
977985
return ""
978986

979987

bin/fm-worker-lifecycle.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2057,6 +2057,14 @@ def make_action(env, action_type, worker=None, item=None, **fields):
20572057
})
20582058
if worker.get("retired_execute_key") is not None:
20592059
action["retired_execute_key"] = worker["retired_execute_key"]
2060+
service_cancel_proof = worker.get("release_proof")
2061+
if (
2062+
action_type == "delete-compute"
2063+
and isinstance(service_cancel_proof, dict)
2064+
and service_cancel_proof.get("schema") == "fm.worker-service-cancel/v1"
2065+
and service_cancel_proof.get("verdict") == "cancelled-before-execution"
2066+
):
2067+
action["service_cancel_proof"] = copy.deepcopy(service_cancel_proof)
20602068
if item is not None:
20612069
action["request"] = item
20622070
action.update(fields)
@@ -2498,6 +2506,10 @@ def apply_action_result(env, state, action, result):
24982506
elif execution.get("step_outcome_sha256") not in (None, ""):
24992507
raise LifecycleError(
25002508
"absent no-mistakes service return asserted a step outcome digest")
2509+
cloud = result.get("worker")
2510+
if not isinstance(cloud, dict) or cloud.get("slot") != worker["slot"]:
2511+
raise LifecycleError("provider execute result returned the wrong worker slot")
2512+
adopt_cloud_resources(worker, cloud)
25012513
state["executions"][action["request_digest"]] = execution
25022514
worker["last_execution_digest"] = supplied
25032515
worker["last_execution_at"] = iso_utc()

tests/fm-azure-pilot.test.sh

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,6 +1479,51 @@ try:
14791479
raise AssertionError("an overwrite upload took the create-once convergence path")
14801480
except provider.ProviderError as exc:
14811481
assert "staging upload failed" in str(exc), exc
1482+
1483+
# A later execute reuses the assignment's fixed request blob name. Its action
1484+
# can carry the ETag observed before the prior execute updated that blob, so a
1485+
# same-assignment ConditionNotMet must adopt the current ETag and retry one CAS.
1486+
# A foreign assignment still fails the complete expected-tag comparison.
1487+
conditional_tags = {"task-binding": "task-one", "assignment-generation": "asg-00000001"}
1488+
conditional_calls = []
1489+
old_payload = b'{"old":true}\n'
1490+
1491+
1492+
def conditional_az(controller, args, check=True, timeout=provider.AZ_TIMEOUT_SECONDS):
1493+
conditional_calls.append(list(args))
1494+
if "upload" in args:
1495+
match = args[args.index("--if-match") + 1]
1496+
if match == '"stale"':
1497+
return None, 1, "ErrorCode:ConditionNotMet"
1498+
assert match == '"current"', args
1499+
return {}, 0, ""
1500+
if "show" in args:
1501+
return {
1502+
"etag": '"current"',
1503+
"metadata": provider.tags_to_metadata(conditional_tags),
1504+
"properties": {"etag": '"current"'},
1505+
}, 0, ""
1506+
if "download" in args:
1507+
Path(args[args.index("--file") + 1]).write_bytes(old_payload)
1508+
return {}, 0, ""
1509+
raise AssertionError(args)
1510+
1511+
1512+
provider.az = conditional_az
1513+
provider.upload_json_blob(
1514+
controller, "acct", "worker-state-01", "request.json", {"new": True},
1515+
conditional_tags, overwrite=True, if_match='"stale"',
1516+
)
1517+
assert sum(1 for args in conditional_calls if "upload" in args) == 2, conditional_calls
1518+
foreign_tags = dict(conditional_tags, **{"assignment-generation": "asg-00000002"})
1519+
try:
1520+
provider.upload_json_blob(
1521+
controller, "acct", "worker-state-01", "request.json", {"newer": True},
1522+
foreign_tags, overwrite=True, if_match='"stale"',
1523+
)
1524+
raise AssertionError("a foreign assignment replaced the sequential execute blob")
1525+
except provider.ProviderError as exc:
1526+
assert "staging upload failed" in str(exc), exc
14821527
# The checks above drive upload_json_blob directly and so cannot see the REAL
14831528
# caller dropping volatile_fields, which is what made S1 revertible with a
14841529
# green suite. Capture what create_lifecycle_children actually passes.

tests/fm-secondmate-cloud-monitor.test.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1484,7 +1484,7 @@ test_chain_tip_argv_is_accepted_by_the_real_lifecycle_cli() {
14841484

14851485
test_valid_child_request_spawns_with_the_exact_parent_pair() {
14861486
make_world child-valid
1487-
emit_child_intent '{"kind":"ship","brief":"ship the compartment child","model":"gpt-5","effort":"high"}'
1487+
emit_child_intent '{"kind":"ship","brief":"ship the compartment child","model":"openai-codex/gpt-5.6-sol","effort":"high"}'
14881488
start_monitor
14891489
wait_for "the child spawn" test -s "$SP_LOG"
14901490
wait_for "the acceptance delivered into the inbox" inbox_has 'FIRSTMATE ACCEPTED'
@@ -1504,7 +1504,7 @@ PY
15041504
argv=$(sed -n 1p "$SP_LOG")
15051505
# The project resolves out of the COMPARTMENT's home (alpha), never the
15061506
# primary's (primary-only): the split points fm-spawn's projects/ there.
1507-
[ "$argv" = "$(printf '%s\x1f%s\x1f--harness\x1fpi\x1f--model\x1fgpt-5\x1f--effort\x1fhigh' "$child" "$LANDING/projects/alpha")" ] \
1507+
[ "$argv" = "$(printf '%s\x1f%s\x1f--harness\x1fpi\x1f--model\x1fopenai-codex/gpt-5.6-sol\x1f--effort\x1fhigh' "$child" "$LANDING/projects/alpha")" ] \
15081508
|| fail "the child spawn argv is not the exact ship shape: $(printf '%s' "$argv" | tr '\037' '|')"
15091509
env_line=$(sed -n 2p "$SP_LOG")
15101510
assert_contains "$env_line" "FM_HOME=$HOME_DIR" \

tests/fm-worker-lifecycle.test.sh

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ PY
313313
}
314314

315315
service_cancel_replay_contract() {
316-
python3 - "$CONTROLLER" <<'PY' || fail "service cancellation replay contract failed"
316+
python3 - "$CONTROLLER" "$AZURE" <<'PY' || fail "service cancellation replay contract failed"
317317
import contextlib
318318
import copy
319319
import importlib.util
@@ -323,6 +323,9 @@ import sys
323323
spec = importlib.util.spec_from_file_location("lifecycle", sys.argv[1])
324324
module = importlib.util.module_from_spec(spec)
325325
spec.loader.exec_module(module)
326+
provider_spec = importlib.util.spec_from_file_location("azure_provider", sys.argv[2])
327+
provider = importlib.util.module_from_spec(provider_spec)
328+
provider_spec.loader.exec_module(provider)
326329
327330
bindings = {
328331
"home_binding": "1" * 64,
@@ -368,6 +371,14 @@ module.command_service_cancel(env, args)
368371
assert item["status"] == "releasing", item
369372
assert item["service_completion_receipt"] == worker["release_proof"], item
370373
assert worker["release_proof"]["verdict"] == "cancelled-before-execution", worker
374+
action_env = {"deployment_generation": "deployment", "owner": "owner"}
375+
worker.update({"sku": "Standard_D4as_v6", "sku_family": "standardDav6Family", "cloud_generation": 1, "reservation_usd": 1.0})
376+
delete_action = module.make_action(action_env, "delete-compute", worker=worker)
377+
assert delete_action["service_cancel_proof"] == worker["release_proof"], delete_action
378+
assert provider.service_cancel_allows_missing_task_command(delete_action)
379+
foreign_cancel = copy.deepcopy(delete_action)
380+
foreign_cancel["service_cancel_proof"]["task"] = "foreign-task"
381+
assert not provider.service_cancel_allows_missing_task_command(foreign_cancel)
371382
module.command_service_cancel(env, args)
372383
373384
item["status"] = "complete"

0 commit comments

Comments
 (0)