fix(issue-57): review nits - a damaged audit sequence or an unresolvable log root never aborts the check
_check_disk_fallback converted transition_seq outside its try, so a status.json whose sequence is not an integer would raise out of the subscriber's main loop; such a status is now ignored with a warning, like a stale one. delegate_started_recorded returned 1 before looking at the registry's events.log when the audit-log root could not be resolved; it now skips only the audit check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
f299034d8d
commit
c982616188
3 files changed
+16
-3
No files matched your search
@@ -2381,9 +2381,12 @@ delegate_started_recorded() {
|
|||||||
local reg_dir="${MAM_JOBS_DIR:-${WORKSPACE_ROOT:-$PWD}/.mam/jobs}"
|
local reg_dir="${MAM_JOBS_DIR:-${WORKSPACE_ROOT:-$PWD}/.mam/jobs}"
|
||||||
# Issue #57: the audit log is resolved by the one rule (next to the registry
|
# Issue #57: the audit log is resolved by the one rule (next to the registry
|
||||||
# unless DELEGATE_JOB_LOGS_DIR says otherwise); no bash copy of the default.
|
# unless DELEGATE_JOB_LOGS_DIR says otherwise); no bash copy of the default.
|
||||||
[ -n "$logs_dir" ] || logs_dir="$(delegate_logs_dir "$reg_dir")" || return 1
|
# If that resolution fails, only the audit check is skipped: the registry's
|
||||||
|
# events.log is still evidence.
|
||||||
|
[ -n "$logs_dir" ] || logs_dir="$(delegate_logs_dir "$reg_dir")" || logs_dir=""
|
||||||
local ev_log="$reg_dir/$job_id.events.log"
|
local ev_log="$reg_dir/$job_id.events.log"
|
||||||
local audit="$logs_dir/$job_id/events.ndjson"
|
local audit=""
|
||||||
|
[ -n "$logs_dir" ] && audit="$logs_dir/$job_id/events.ndjson"
|
||||||
[ -f "$ev_log" ] || [ -f "$audit" ] || return 1
|
[ -f "$ev_log" ] || [ -f "$audit" ] || return 1
|
||||||
EV_LOG="$ev_log" EV_AUDIT="$audit" EV_TOK="$tok" python3 -c '
|
EV_LOG="$ev_log" EV_AUDIT="$audit" EV_TOK="$tok" python3 -c '
|
||||||
import hashlib, json, os, sys
|
import hashlib, json, os, sys
|
||||||
|
|||||||
@@ -110,7 +110,11 @@ def _check_disk_fallback(
|
|||||||
# ignores it. An unversioned record (never transitioned, or
|
# ignores it. An unversioned record (never transitioned, or
|
||||||
# MAM_ENGINE_CAS=0) and a missing record keep the old rule.
|
# MAM_ENGINE_CAS=0) and a missing record keep the old rule.
|
||||||
aud_seq = logged.get("transition_seq")
|
aud_seq = logged.get("transition_seq")
|
||||||
if rec_seq is not None and (aud_seq is None or int(aud_seq) < int(rec_seq)):
|
try:
|
||||||
|
stale = rec_seq is not None and (aud_seq is None or int(aud_seq) < int(rec_seq))
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
stale = True # a damaged status.json proves nothing either
|
||||||
|
if stale:
|
||||||
logger.warning("disk fallback ignored audit status %r for %s: transition_seq %r is older "
|
logger.warning("disk fallback ignored audit status %r for %s: transition_seq %r is older "
|
||||||
"than the record's %r", logged.get("status"), jid, aud_seq, rec_seq)
|
"than the record's %r", logged.get("status"), jid, aud_seq, rec_seq)
|
||||||
else:
|
else:
|
||||||
|
|||||||
@@ -369,6 +369,12 @@ def test_old_audit_terminal_does_not_complete_the_redispatched_attempt(moved, mo
|
|||||||
pending, terminal = {jid}, {}
|
pending, terminal = {jid}, {}
|
||||||
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid: rec["dispatch_token"]}, {jid: 2})
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid: rec["dispatch_token"]}, {jid: 2})
|
||||||
assert pending == {jid} and terminal == {}
|
assert pending == {jid} and terminal == {}
|
||||||
|
# a damaged sequence is ignored too, and does not take the subscriber down
|
||||||
|
mqtt_common.job_log_path(jid, mqtt_common.STATUS_FILENAME, str(logs)).write_text(
|
||||||
|
json.dumps({"job_id": jid, "status": "completed", "transition_seq": "n/a"}))
|
||||||
|
pending, terminal = {jid}, {}
|
||||||
|
job_subscriber._check_disk_fallback(pending, str(reg), terminal, {jid: rec["dispatch_token"]}, {jid: 2})
|
||||||
|
assert pending == {jid} and terminal == {}
|
||||||
# a newer audit-only terminal is still taken
|
# a newer audit-only terminal is still taken
|
||||||
mqtt_common.update_logged_status(jid, "completed", logs_dir=str(logs), transition_seq=rec["transition_seq"] + 1)
|
mqtt_common.update_logged_status(jid, "completed", logs_dir=str(logs), transition_seq=rec["transition_seq"] + 1)
|
||||||
pending, terminal = {jid}, {}
|
pending, terminal = {jid}, {}
|
||||||
|
|||||||
Reference in new issue
Block a user