[jonahgabriel]

OMN-18961. The companion to the driver change in the registry repository: when the morning tick has deferred itself to an announced usage-limit reset, the session-start goal surface says so.

Why

A stale goal with a re-fire coming and a stale goal nobody is coming back to are different states, and until now they printed identically. The reader of the second should re-baseline by hand. The reader of the first should usually wait, because a re-baseline run against a limit that is minutes from clearing burns the same budget the deferral exists to preserve.

The line prints after the failure cause and before the re-baseline command, because that command is the right action when nothing is coming and the wrong one when a re-fire is imminent.

What changed

The stale branch reads the deferral record the driver writes and prints the scheduled time. Absent record, nothing is said: announcing a re-fire that does not exist is worse than announcing none, because it tells the reader to wait for something nobody is going to send. A fresh goal never reads either file, on the same reasoning that already governs the failure cause.

The JSON field reader is now general over a file rather than hardcoded to the notice, so both records go through one decoder and one set of escape rules. No behaviour change to the failure-cause path.

Tests

Three cases added to tests/hooks/test_session_start_goal_surface.py, each a real subprocess run: a pending re-fire announced beside the staleness with the cause still printed; no record meaning no claim; and a fresh goal staying silent. 33 passed, 0 skipped.

Still pure bash, no interpreter, no network, no writes, exit 0 on every user-visible outcome. The existing static test that asserts all of that passes unchanged.

Evidence-Ticket: OMN-18961
Evidence-Source: OCC#10621

[github-actions]

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 10
Models succeeded: deepseek-r1,qwen3-review-b

Findings

Severity	Location	Finding
warning	plugins/onex/hooks/scripts/session_start_goal_surface.sh:1	ai-reviewer:deepseek-r1:correctness JSON field extraction via grep/sed is fragile against multi-line values | The refactored _json_field function uses grep -m1 to extract a single line containing the key. If the JSON value for a field (such as first_line or refire_at) contains a newline character that was escaped as \n by json.dump, the sed pipeline will only process the first physical line of the value. More critically, if the JSON is formatted with the value on a separate line from the key (e.g., "key":\n "value"), grep -m1 will match the
info	plugins/onex/hooks/scripts/session_start_goal_surface.sh:1	ai-reviewer:deepseek-r1:security Unsanitized JSON field values in output | The _json_field function extracts values from JSON files and prints them to stdout. While the script claims to be 'print-only', the values are read from files under $OMNI_HOME. If an attacker can write to these files (or if the files are symlinked to malicious content), the extracted values could contain escape sequences or control characters that might interfere with the terminal or downstream consumers of the hook's output. The sed pipeline handles some escapes (\
warning	tests/hooks/test_session_start_goal_surface.py:1	ai-reviewer:deepseek-r1:testing Missing test for malformed or missing deferral JSON | The new code path reads from ${OMNI_HOME}/${_TICK_DEFERRAL_REL}. The tests cover the case where the file exists and is valid, and where the file does not exist. However, there is no test for the case where the file exists but is malformed JSON, or where the refire_at key is missing. In such cases, _json_field will return an empty string, and the script will print 'a re-fire IS scheduled for an unrecorded time'. This is a reasonable fallback, but it is no
info	plugins/onex/hooks/contracts/hook_inventory.yaml:1	ai-reviewer:deepseek-r1:completeness Documentation does not mention the new deferral file in the 'Reads' section | The header comment in the script was updated to mention the new deferral file, but the YAML contract file (hook_inventory.yaml) only updates the description of the hook's behavior. It does not explicitly list the new file in a 'reads' or 'inputs' section if such a section exists in the contract. The diff shows the YAML description was updated to mention the deferral, but it is not clear if the contract schema enforces a list of in
hint	plugins/onex/hooks/scripts/session_start_goal_surface.sh:1	ai-reviewer:deepseek-r1:performance Redundant file existence checks | The script checks [[ -n "${OMNI_HOME:-}" && -r "${OMNI_HOME}/${_TICK_DEFERRAL_REL}" ]] before calling _json_field. The _json_field function itself does not check for file existence; it just runs grep. If the file does not exist, grep will fail and output nothing, which is handled by the fallback. The explicit check is good for clarity, but it is slightly redundant with the behavior of grep. This is a minor style issue. | Evidence: if [[ -n "${OMNI_HOME:-}" && -r "${O
warning	plugins/onex/hooks/scripts/session_start_goal_surface.sh:1	ai-reviewer:qwen3-review-b:correctness JSON field extraction via grep/sed is fragile and breaks on multi-line or nested values | The _json_field function uses grep -m1 to find a line containing the key, then sed to strip the key and surrounding quotes. This approach fails if the JSON value spans multiple lines (e.g., a multi-line string with embedded newlines escaped as \n), if the key appears in a nested object before the top-level one, or if the value contains a colon followed by a quote. The sed regex s/^[^:]:[[:space:]]// assumes the value
info	plugins/onex/hooks/scripts/session_start_goal_surface.sh:1	ai-reviewer:qwen3-review-b:security Unvalidated JSON field values are interpolated into user-facing output without sanitization | The _refire_at value extracted from the deferral JSON is printed directly to stdout via say. If the deferral file is written by an untrusted process or is corrupted, the value could contain ANSI escape sequences, terminal control characters, or other content that affects the terminal. While the hook is described as 'print-only' and 'no network', the output is consumed by a terminal or log viewer. The existing _fail
info	tests/hooks/test_session_start_goal_surface.py:1	ai-reviewer:qwen3-review-b:completeness No test for deferral file with missing or malformed refire_at field | The tests cover the happy path (deferral file present with refire_at) and the absence of the deferral file. There is no test for the case where the deferral file exists but the refire_at key is missing, or the file is malformed JSON. In these cases, _json_field will return an empty string, and the output will say 'an unrecorded time'. This behavior is not tested, and it is unclear whether this is the desired fallback. A malformed deferral
info	tests/hooks/test_session_start_goal_surface.py:1	ai-reviewer:qwen3-review-b:architecture Deferral file path is hardcoded to a specific workflow name, reducing reusability | The _TICK_DEFERRAL_REL variable is constructed using ${_WORKFLOW_NAME}, which is presumably 'morning-ground-state'. The test helper _write_deferral hardcodes 'morning-ground-state.json' in the path. If the workflow name changes or if this pattern is reused for other workflows, the test will break. The script itself uses the variable correctly, but the test does not parameterize the workflow name. | Evidence: def _write_defer
hint	plugins/onex/hooks/scripts/session_start_goal_surface.sh:1	ai-reviewer:qwen3-review-b:performance Two separate grep/sed invocations per field extraction are inefficient | Each call to _json_field spawns two processes (grep and sed). The stale branch now calls _json_field three times for the failure notice and once for the deferral file, resulting in eight process spawns. While this is a print-only hook that runs once per session, the overhead is unnecessary. A single awk script could extract all fields from a file in one pass. | Evidence: _fail_phase="$(_json_field phase "${OMNI_HOME}/${_TICK_NOTICE_REL

---

Gate semantics (pilot phase)

Verdict	Meaning	Blocks merge?
passed	No critical/error findings	No
blocked	CRITICAL or ERROR findings found	Yes
degraded	Reviewer ran but every model failed mid-run	No (pilot)
infra_error	cli_review failed to run at all (install/crash) after 2 attempts	Yes
unavailable_fork_isolation	Fork PR — the LAN-only model is unreachable and the fleet is off limits	Yes

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)