From cbcfaeac77499da71d97adc74e69736bc09f23b8 Mon Sep 17 00:00:00 2001 From: dietrich ayala Date: Sun, 27 Sep 2026 07:43:58 +0200 Subject: [PATCH] poll-and-land, submit, records: a needs-person ending waits for a person's call The poller records a run step exit 11 as needs-person, with the failing stage from the forge's result and the line naming submit.sh, and neither resends it nor counts it. submit.sh resends a needs-person record at its recorded base without raising resends or applying the three-resend refusal, and refuses one whose spec changed, naming --restart. records.py accepts the outcome. --- jobs/scripts/poll-and-land.sh | 15 ++++++- jobs/scripts/records.py | 8 ++-- jobs/scripts/submit.sh | 19 ++++++++- jobs/tests/poll_and_land.rs | 32 +++++++++++++++ jobs/tests/submit.rs | 75 +++++++++++++++++++++++++++++++++++ 5 files changed, 143 insertions(+), 6 deletions(-) diff --git a/jobs/scripts/poll-and-land.sh b/jobs/scripts/poll-and-land.sh index 38d71838..76cbf7cc 100755 --- a/jobs/scripts/poll-and-land.sh +++ b/jobs/scripts/poll-and-land.sh @@ -23,6 +23,12 @@ # records nothing, so the next pass fetches again. # 1 a stage failed: `failed`, with the failing stage read from the forge's result when # it can be; not resent. +# 11 the job's failed rework rounds are used up and it waits for a person +# (specs/job-review-rework.md): `needs-person`, with the failing stage read from the +# forge's result as for exit 1, and the line `resend with jobs/scripts/submit.sh +# once a fix or a spec change is in`. Not resent and not counted as failed: +# submit.sh resends it once a person has pushed a fix to the forge's +# job/, and submit.sh --restart once the spec or its manifest changed. # 7 publish failed: resent with submit.sh (counted), or `parked` when it has # already been resent three times. # 3 nothing ran: the inventory check (jobs/src/inventory.rs) refused the job for @@ -81,7 +87,8 @@ # and passed once it is running or has ended with a `run` step exit (each sent once, however # many passes see it); `land` started before bench/land.sh and passed or failed after it (not # with --check); and, whenever an outcome is recorded, an outcome event: landed as passed, -# refused and conflict as needs-person, failed, parked and stopped as themselves. A resend +# refused and conflict as needs-person, failed, parked, stopped and needs-person as +# themselves. A resend # sends nothing here; submit.sh reports the new run. # # Settings, each defaulting to the real collaborator so tests can replace it: @@ -158,7 +165,7 @@ record() { case "$2" in landed) outcome=passed ;; refused|conflict) outcome=needs-person ;; - failed|parked|stopped) outcome="$2" ;; + failed|parked|stopped|needs-person) outcome="$2" ;; esac [ -z "$outcome" ] || [ -z "$CURRENT_RUN" ] || event_send outcome "$1" "$CURRENT_RUN" "$outcome" "$3" } @@ -421,6 +428,10 @@ poll() { fi ;; 3) resend "$name" uncounted "run $id: nothing ran" ;; + 11) + why="$(failed_stage "$name")" + record "$name" needs-person "run $id: its failed rounds are used up${why:+: $why}; resend with jobs/scripts/submit.sh $name once a fix or a spec change is in" + ;; 8) record "$name" stopped "run $id: stopped; resend with jobs/scripts/submit.sh $name" ;; 9) why="$(preflight_reasons "$name" "$id")" diff --git a/jobs/scripts/records.py b/jobs/scripts/records.py index 78a613b6..7c8af86e 100755 --- a/jobs/scripts/records.py +++ b/jobs/scripts/records.py @@ -11,10 +11,12 @@ $HOME/.local/state/nev-jobs), so no host or address can reach a commit. A record holds: name the job's name resends how many times submit.sh enqueued it after the first (a number; resends - after a `run` step exit 3 are not counted) + after a `run` step exit 3, and resends of a needs-person record, are not + counted) run_id the last Dagu run id, which `bench/dagu-lane.sh watch` needs outcome empty while the job is in Dagu's hands, then landed, refused, conflict, - failed, stopped or parked + failed, stopped, parked or needs-person (the job's failed rework rounds are + used up and it waits for a person's call, specs/job-review-rework.md) message the last message spec_sha the spec's blob at base that the last run was sent with (submit.sh); once outcome is landed, the blob that landed. submit.sh refuses a landed job only @@ -32,7 +34,7 @@ import sys import tempfile FIELDS = {"name", "resends", "run_id", "outcome", "message", "spec_sha", "base_sha"} -OUTCOMES = {"", "landed", "refused", "conflict", "failed", "stopped", "parked"} +OUTCOMES = {"", "landed", "refused", "conflict", "failed", "stopped", "parked", "needs-person"} NAME = re.compile(r"^[A-Za-z0-9._-]+$") diff --git a/jobs/scripts/submit.sh b/jobs/scripts/submit.sh index 3baf9750..f4f84b76 100755 --- a/jobs/scripts/submit.sh +++ b/jobs/scripts/submit.sh @@ -20,6 +20,11 @@ # waiting). That last check reads the run from Dagu, so with the lane or Dagu not # answering it exits 3 instead. A `landed` record whose `spec_sha` differs (a later # version of the spec) counts as no record: the job starts afresh, resends at 0; +# - a `needs-person` record (the job's failed rework rounds are used up, +# specs/job-review-rework.md) has a `spec_sha` equal to the spec's blob at base; one +# whose spec changed is refused, naming --restart. Its resend, which follows a +# person's fix pushed to the forge's job/, neither raises `resends` nor is +# refused for reaching three, and its line says the job resumes after a person's call; # - on a resend whose spec blob at the base tip equals the record's `spec_sha`, the base # it resumes at (below) is a commit here, on the forge's base branch, holds the spec # with that blob, and its manifest passes `job-check manifest`. @@ -198,9 +203,20 @@ case "$PREV_OUTCOME" in PREV_RUN="" PREV_OUTCOME="" PREV_RESENDS=0 ;; esac +# A needs-person record (the job's failed rework rounds are used up) is resent once a +# person has made the call: a fix pushed to the forge's job/ resumes it here, without +# counting, since that is not a retry of a flaky publish. A changed spec or manifest needs +# --restart, because the lane's `prepare` resumes a job only at the spec it started with. +PERSON=no +if [ "$PREV_OUTCOME" = needs-person ] && [ "$RESTART" = no ]; then + if [ -z "$PREV_SPEC" ] || [ "$PREV_SPEC" != "$SPEC_BLOB" ]; then + refuse "$NAME waits for a person and $SPEC changed since it was sent (blob ${PREV_SPEC:0:12} then, ${SPEC_BLOB:0:12} now); a resend resumes only at the spec it started with, so send it with jobs/scripts/submit.sh --restart $NAME" + fi + PERSON=yes +fi RESENDS=0 if [ -n "$PREV_RUN" ] && [ "$RESTART" = no ]; then - if [ "$UNCOUNTED" = yes ]; then + if [ "$UNCOUNTED" = yes ] || [ "$PERSON" = yes ]; then RESENDS="$PREV_RESENDS" else [ "$PREV_RESENDS" -lt 3 ] || refuse "$NAME has been resent $PREV_RESENDS times" @@ -244,6 +260,7 @@ if [ "$RESTART" = no ] && [ -n "$PREV_RUN" ] && [ -n "$PREV_SPEC" ] && [ "$PREV_ [ "$PREV_BASE" = "$BASE_SHA" ] || check_manifest "$PREV_BASE" SEND_BASE="$PREV_BASE" RESUME=", resumed at the recorded base" + [ "$PERSON" = no ] || RESUME=", resumed after a person's call, at the recorded base" [ "$SEND_BASE" = "$BASE_SHA" ] || RESUME="$RESUME (the base tip is ${BASE_SHA:0:12})" fi diff --git a/jobs/tests/poll_and_land.rs b/jobs/tests/poll_and_land.rs index b6b2a4c1..035fd9ac 100644 --- a/jobs/tests/poll_and_land.rs +++ b/jobs/tests/poll_and_land.rs @@ -342,6 +342,38 @@ fn a_fourth_resend_is_parked() { assert!(s.run_calls().is_empty()); } +#[test] +fn exit_11_needs_a_person_with_the_stage_and_is_not_resent() { + let s = Setup::new("exit11"); + s.publish(|r| { + r["status"] = json!("needs-person"); + r["failed_stage"] = json!("review"); + r["reason"] = json!("2 failed rounds of 2 used up"); + }); + s.open_job(2, "failed 11 tools:succeeded prepare:succeeded run:failed"); + let out = s.poll(); + let rec = s.record(); + assert_eq!(rec["outcome"], "needs-person"); + assert_eq!(rec["resends"], 2); + let message = rec["message"].as_str().unwrap(); + assert!(message.contains("stage review: 2 failed rounds of 2 used up"), "{rec}"); + assert!( + message.contains(&format!("resend with jobs/scripts/submit.sh {NAME} once a fix or a spec change is in")), + "{rec}" + ); + assert!(text(&out.stdout).starts_with(&format!("{NAME}: needs-person: ")), "{}", text(&out.stdout)); + assert!(s.run_calls().is_empty(), "{:?}", s.lines("calls")); + assert!(s.land_calls().is_empty()); + assert_eq!(s.events(), vec![ev("step queue passed RUN"), ev("outcome - needs-person RUN")]); + + // A later pass does not ask about the record again. + let calls = s.lines("calls").len(); + let before = s.record(); + s.poll(); + assert_eq!(s.lines("calls").len(), calls, "{:?}", s.lines("calls")); + assert_eq!(s.record(), before); +} + #[test] fn exit_8_is_stopped_and_not_resent() { let s = Setup::new("exit8"); diff --git a/jobs/tests/submit.rs b/jobs/tests/submit.rs index 671174d5..96b70966 100644 --- a/jobs/tests/submit.rs +++ b/jobs/tests/submit.rs @@ -607,6 +607,81 @@ fn a_resend_with_a_changed_spec_goes_at_the_tip() { assert!(!stdout(&out).contains("resumed"), "{}", stdout(&out)); } +/// A `needs-person` record for the job, sent at `base` with the spec blob `spec_sha`. +fn needs_person_record(s: &Setup, resends: u32, spec_sha: &str, base: &str) { + s.write_record( + NAME, + &format!( + r#"{{"name":"{NAME}","resends":{resends},"run_id":"old","outcome":"needs-person","message":"run old: its failed rounds are used up","spec_sha":"{spec_sha}","base_sha":"{base}"}}"# + ), + ); +} + +#[test] +fn a_needs_person_record_is_resent_at_its_base_without_counting() { + for resends in [0, 3] { + let s = Setup::new(&format!("needs-person-{resends}")); + let first = s.tip(); + needs_person_record(&s, resends, &blob(&s.repo()), &first); + move_main(&s, "later"); + assert_ne!(s.tip(), first); + s.stub_file("run-id", "run-2\n"); + let out = s.submit(); + assert_eq!(out.status.code(), Some(0), "{}", stderr(&out)); + assert_eq!( + s.run_calls(), + vec![format!("--lane 0 run --no-wait bench/dagu/job.yaml NAME={NAME} SPEC_SHA={} BASE_SHA={first}", blob(&s.repo()))] + ); + let rec = s.record().unwrap(); + assert_eq!(rec["resends"], resends); + assert_eq!(rec["run_id"], "run-2"); + assert_eq!(rec["outcome"], ""); + assert_eq!(rec["base_sha"], first); + assert!(stdout(&out).contains("resumed after a person's call"), "{}", stdout(&out)); + } +} + +#[test] +fn a_needs_person_record_whose_spec_changed_is_refused_naming_restart() { + let s = Setup::new("needs-person-changed"); + needs_person_record(&s, 0, &blob(&s.repo()), &s.tip()); + std::fs::write( + s.repo().join(format!("specs/{NAME}.md")), + format!("---\nenabled: true\ndepends-on: [{DEP}]\n---\n\n# {NAME}\n\nA clearer rule.\n"), + ) + .unwrap(); + s.commit("amended spec"); + s.push(); + s.assert_refused("--restart"); + assert!(s.calls().is_empty(), "{:?}", s.calls()); +} + +#[test] +fn records_accepts_the_needs_person_outcome() { + let s = Setup::new("records-needs-person"); + let records = crate_dir().join("scripts/records.py"); + let state = s.state(); + let set = Command::new("python3") + .arg(&records) + .arg(&state) + .args(["set", NAME, "run_id=r1", "outcome=needs-person", "message=waits"]) + .output() + .unwrap(); + assert!(set.status.success(), "{}", stderr(&set)); + assert_eq!(s.record().unwrap()["outcome"], "needs-person"); + // Not open: a needs-person record is not the poller's to ask about again. + let open = Command::new("python3").arg(&records).arg(&state).arg("open").output().unwrap(); + assert!(open.status.success(), "{}", stderr(&open)); + assert_eq!(stdout(&open), ""); + let bad = Command::new("python3") + .arg(&records) + .arg(&state) + .args(["set", NAME, "outcome=needs-a-person"]) + .output() + .unwrap(); + assert!(!bad.status.success()); +} + #[test] fn a_restart_clears_the_forge_and_lane_keeps_the_record_and_sends_afresh() { let s = Setup::new("restart"); -- 2.54.0