From d7898148920043c8a666f62fbe5fe3480a030b5d Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Sun, 20 Sep 2026 23:51:46 -0400 Subject: [PATCH] feat(OMN-18961): the session-start goal surface announces a pending tick re-fire --- .../onex/hooks/contracts/hook_inventory.yaml | 6 +- .../scripts/session_start_goal_surface.sh | 38 +++++++--- .../hooks/test_session_start_goal_surface.py | 72 +++++++++++++++++++ 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/plugins/onex/hooks/contracts/hook_inventory.yaml b/plugins/onex/hooks/contracts/hook_inventory.yaml index 7f9a0b7021..714921ab8b 100644 --- a/plugins/onex/hooks/contracts/hook_inventory.yaml +++ b/plugins/onex/hooks/contracts/hook_inventory.yaml @@ -1137,8 +1137,10 @@ expected_hooks: four dropped-work headline counts it carries (OMN-18954), and the exact re-baseline command when it is missing or stale. A stale goal also names the last morning-tick failure, read from the tick's own notice file under - $OMNI_HOME, so the staleness arrives with its cause. Print-only: pure - bash, no interpreter, no network, no writes. + $OMNI_HOME, so the staleness arrives with its cause, and announces a + pending re-fire when the tick deferred itself to an announced usage-limit + reset (OMN-18961). Print-only: pure bash, no interpreter, no network, no + writes. enforcement: false lite_mode_exit: true mask: diff --git a/plugins/onex/hooks/scripts/session_start_goal_surface.sh b/plugins/onex/hooks/scripts/session_start_goal_surface.sh index a30ac85804..1b2b113613 100755 --- a/plugins/onex/hooks/scripts/session_start_goal_surface.sh +++ b/plugins/onex/hooks/scripts/session_start_goal_surface.sh @@ -28,6 +28,8 @@ # Reads: $KNOWLEDGE_BASE_INTERNAL_PATH/beta/GOAL.md # $OMNI_HOME/.onex_state/morning-workflows/notifications/ # morning-ground-state.failure.json (only when the goal is STALE) +# $OMNI_HOME/.onex_state/morning-workflows/deferred/ +# morning-ground-state.json (only when the goal is STALE) # Writes: stdout only. No files, no state, no network, ever. # Blocks: never. Exit 0 on every user-visible outcome, including a missing # file and an unset env var. @@ -104,6 +106,12 @@ _DROPPED_KEYS=( # a stale goal is the CAUSE of the staleness rather than a second mystery. _TICK_NOTICE_REL=".onex_state/morning-workflows/notifications/${_WORKFLOW_NAME}.failure.json" +# Where the tick records a re-fire it has scheduled against an announced +# usage-limit reset (OMN-18961). A stale goal with a pending re-fire is a +# DIFFERENT state from a stale goal nobody is coming back to, and telling +# them apart is the difference between waiting and re-baselining by hand. +_TICK_DEFERRAL_REL=".onex_state/morning-workflows/deferred/${_WORKFLOW_NAME}.json" + # SessionStart delivers a JSON payload on stdin. Nothing here needs it, but an # unread stdin can hand the caller an EPIPE, so drain it unconditionally. cat >/dev/null 2>&1 || true @@ -144,18 +152,19 @@ if [[ -f "$_INTENT_SH" ]]; then fi fi -# Reads one string field out of the tick's failure notice, which is JSON written -# by python's json.dump. Field extraction is `grep` plus `sed` rather than a JSON -# parser, because this hook spins up no interpreter (see the header) -- so the -# escapes json.dump emits have to be undone here. +# Reads one string field out of a small JSON file the tick wrote with python's +# json.dump -- the failure notice, or the deferral record. Field extraction is +# `grep` plus `sed` rather than a JSON parser, because this hook spins up no +# interpreter (see the header) -- so the escapes json.dump emits have to be +# undone here. # # The three that actually occur are handled and the rest are LEFT LITERAL on # purpose: a wrong decoding is worse than a visible backslash, and the field is # a human-read cause line, not a value anything parses. `\u00b7` is the one that # showed up live -- the harness writes the separator into its own failure text. -_notice_field() { - local key="$1" - grep -m1 -E "\"${key}\"[[:space:]]*:" "${OMNI_HOME}/${_TICK_NOTICE_REL}" \ +_json_field() { + local key="$1" file="$2" + grep -m1 -E "\"${key}\"[[:space:]]*:" "$file" \ | sed -E 's/^[^:]*:[[:space:]]*//; s/^"//; s/",?[[:space:]]*$//; s/,[[:space:]]*$//' \ | sed -E 's/\\u00b7/·/g; s/\\"/"/g; s/\\\\/\\/g' } @@ -368,15 +377,24 @@ if (( _stale == 1 )); then # goal with no cause sends the reader to re-run a workflow that will fail # the same way. if [[ -n "${OMNI_HOME:-}" && -r "${OMNI_HOME}/${_TICK_NOTICE_REL}" ]]; then - _fail_phase="$(_notice_field phase)" - _fail_ts="$(_notice_field ts)" - _fail_first="$(_notice_field first_line)" + _fail_phase="$(_json_field phase "${OMNI_HOME}/${_TICK_NOTICE_REL}")" + _fail_ts="$(_json_field ts "${OMNI_HOME}/${_TICK_NOTICE_REL}")" + _fail_first="$(_json_field first_line "${OMNI_HOME}/${_TICK_NOTICE_REL}")" say " last tick outcome: ${_fail_phase:-unreported} at ${_fail_ts:-unknown} — ${_fail_first:-no cause recorded}" elif [[ -z "${OMNI_HOME:-}" ]]; then say " last tick outcome: UNRESOLVED — OMNI_HOME is unset, so ${_TICK_NOTICE_REL} cannot be located. No default is applied." else say " last tick outcome: no failure notice on disk — the last completed tick was clean, so the staleness is a MISSED fire, not a failed one." fi + + # A pending re-fire changes what the reader should DO. Printed after the + # cause and before the re-baseline command, because the command below is + # the right action when nothing is coming and the wrong one when a + # re-fire is minutes away. + if [[ -n "${OMNI_HOME:-}" && -r "${OMNI_HOME}/${_TICK_DEFERRAL_REL}" ]]; then + _refire_at="$(_json_field refire_at "${OMNI_HOME}/${_TICK_DEFERRAL_REL}")" + say " a re-fire IS scheduled for ${_refire_at:-an unrecorded time} — the tick deferred itself to the reset its refusal announced. Re-baseline by hand only if you need the goal before then." + fi print_rebaseline_command fi diff --git a/tests/hooks/test_session_start_goal_surface.py b/tests/hooks/test_session_start_goal_surface.py index 3646a3a6e3..3d964e68ce 100644 --- a/tests/hooks/test_session_start_goal_surface.py +++ b/tests/hooks/test_session_start_goal_surface.py @@ -639,6 +639,78 @@ def test_a_fresh_goal_does_not_read_the_tick_notice(tmp_path: Path) -> None: assert "last tick outcome" not in res.stdout +def _write_deferral(omni_root: Path, *, refire_at: str) -> Path: + """Write the record the tick leaves when it defers a re-fire (OMN-18961).""" + d = omni_root / ".onex_state" / "morning-workflows" / "deferred" + d.mkdir(parents=True, exist_ok=True) + path = d / "morning-ground-state.json" + path.write_text( + "{\n" + ' "note": "one deferral per original fire",\n' + ' "origin_fire_id": "20260921T024828Z-morning-ground-state",\n' + f' "refire_at": "{refire_at}",\n' + ' "waiter_pid": 4242,\n' + ' "workflow": "morning-ground-state"\n' + "}\n" + ) + return path + + +def test_a_pending_refire_is_announced_beside_the_staleness(tmp_path: Path) -> None: + """A stale goal with a re-fire coming is a different state. + + Without this line the two look identical, and the reader re-baselines by + hand against a limit that is about to clear on its own. + """ + kb = tmp_path / "kb" + omni = tmp_path / "omni" + _write_goal(kb, _iso(timedelta(hours=-30))) + _write_tick_notice( + omni, + phase="failed", + ts="2026-09-21T00:04:55Z", + first_line="session limit reached", + ) + _write_deferral(omni, refire_at="2026-09-21T02:23:00Z") + + res = _run(str(kb), omni_root=str(omni)) + + assert res.returncode == 0 + assert "a re-fire IS scheduled for 2026-09-21T02:23:00Z" in res.stdout + # The cause still prints; the deferral is an addition, not a replacement. + assert "session limit reached" in res.stdout + + +def test_no_deferral_record_means_no_refire_claim(tmp_path: Path) -> None: + """Silence is the honest output when nothing is scheduled. + + Announcing a re-fire that does not exist is worse than announcing none: + it tells the reader to wait for something nobody is going to send. + """ + kb = tmp_path / "kb" + omni = tmp_path / "omni" + omni.mkdir() + _write_goal(kb, _iso(timedelta(hours=-30))) + + res = _run(str(kb), omni_root=str(omni)) + + assert res.returncode == 0 + assert "re-fire IS scheduled" not in res.stdout + + +def test_a_fresh_goal_does_not_announce_a_deferral(tmp_path: Path) -> None: + """Same reasoning as the failure cause: the stale branch owns both.""" + kb = tmp_path / "kb" + omni = tmp_path / "omni" + _write_goal(kb, _iso(timedelta(hours=-1))) + _write_deferral(omni, refire_at="2026-09-21T02:23:00Z") + + res = _run(str(kb), omni_root=str(omni)) + + assert res.returncode == 0 + assert "re-fire IS scheduled" not in res.stdout + + def test_the_hook_declares_exactly_these_five_keys() -> None: """The key list in the script must match the one spelled here.