[jonahgabriel]

Summary

Ticket OMN-20873. Before: scripts/hook_process_canary.py ran one ps -axwwo pid=,ppid=,uid=,command= over every process of every user; on the loaded operator Mac it timed out after 30 s, so each run raised state=ALERT kinds=canary-error with every count NA (6 occurrences in the ledger). After: default_sampler reads (1) ps -x -U <uid> -o pid=,ppid=,uid= (user's own processes, no command column), (2) pgrep -U <uid> -f '/hooks/(scripts|lib)/', (3) ps -ww -p <pids> -o pid=,ppid=,uid=,command= for only the pgrep-selected pids. One 30 s deadline (the existing timeout, not raised) covers all three calls. pgrep/ps exit 1 with no match is a clean zero. measure() still re-applies the same hook regex, so counts and all thresholds (orphans>20, hook procs>150, user procs>60% of limit) are unchanged.

Changes

• A listing that times out raises ListingTimeoutError carrying a frozen ListingTimeout(error_class="canary-error", timeout_seconds, command).
• Canary.run_once turns it into a canary-error alarm whose detail states the timeout; ALERT.json gets a listing_timeout object; the ledger STATUS row is state=CANARY-ERROR | ... | class=canary-error timeout_s=30 command=ps | detail=... with no NA count cells.
• A cycle with a real reading still writes state=ALERT with counts.
• Exit code 5 for canary-error is unchanged.
• cannot-measure (fork failure) rows are unchanged.
• Stdlib dataclass used because the canary runs under the bare brew python3.13 / system python3 with no pydantic.
• Script is not canonical-shape; not converted here (separate ticket).

Tests

• New tests/hooks_system/test_hook_canary_listing_omn20873.py, 8 tests (4 named hook_process_canary_bounded_listing, 4 hook_process_canary_listing_timeout_typed); 6 failed before the change and 8 pass after.
• With the existing tests/hooks_system/test_hook_canary.py and tests/ci/test_hook_system_tests_gate_omn20109.py: 41 passed, 1 skipped.
• ruff format/check clean, mypy --strict on the script clean.
• contracts/OMN-20873.yaml binds AC1 (uv run pytest tests -q -k hook_process_canary_bounded_listing) and AC2 (uv run pytest tests -q -k hook_process_canary_listing_timeout_typed) via dod_evidence ids dod-omn-20873-ac1 and dod-omn-20873-ac2.

Lab

Lab: host omninode-pc (lab host h201), command PATH=<planted slow ps>:$PATH python3 scripts/hook_process_canary.py --once (the stub ps sleeps 40 s), result: 30.1 s, exit 5, reading=none alarms=canary-error, ledger row state=CANARY-ERROR ... class=canary-error timeout_s=30 command=ps, no NA count. The operator Mac's real load cannot be reproduced on a lab host.

• Host omninode-pc (lab host h201), load average 53.79 41.28 42.15 on 32 cores, 2163 processes.
• Old selector ps -axwwo pid=,ppid=,uid=,command= took 0.10-0.14 s; new selectors ps -x -U 1000 -o pid=,ppid=,uid= 0.04-0.05 s plus pgrep -U 1000 -f 0.07-0.09 s (182 user processes, 20 pgrep-selected).
• On Linux both are fast and the narrowing is no faster in total: the Mac's slow ps cannot be reproduced on a lab host and the speed gain on the Mac is unmeasured.
• Real canary --once on the host: canary hook=19 py=19 orphans=0 user=184 limit=348677 alarms=none, exit 0.
• Details of the planted slow ps run (a stub ps on PATH that sleeps 40 s), python3 scripts/hook_process_canary.py --once: took 30.1 s, exit 5, summary reading=none alarms=canary-error, ledger row STATUS | lane=hook-process-canary | state=CANARY-ERROR | host=omninode-pc | kinds=canary-error | class=canary-error timeout_s=30 command=ps | detail=ps listing timed out after 30 seconds (class=canary-error), ALERT.json listing_timeout {command: ps, error_class: canary-error, timeout_seconds: 30}, reading null.
• Unverified: ps -x -U <numeric uid> and pgrep -U -f option behaviour on macOS was not run (no Mac here); the choice of selectors is justified by narrowing the command-column read to the hook pids, not by a Mac measurement.

Evidence-Ticket: OMN-20873
Evidence-Source: OCC#13612

[github-actions]

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 11
Models succeeded: qwen3-review,local-studio-planner

Findings

Severity	Location	Finding
warning	scripts/hook_process_canary.py, default_sampler function:1	ai-reviewer:qwen3-review:security pgrep -f pattern matching is overly broad and can match non-hook processes | The new default_sampler uses pgrep -U -f to identify hook processes. The -f flag matches against the full command line, not just the process name. This means any process whose command line contains the _HOOK_PATH.pattern string will be matched, including shell wrappers, grep commands, or editor sessions that have the hook path in their arguments. This can lead to false positives in the hook process count, potentiall
warning	scripts/hook_process_canary.py, default_sampler function:1	ai-reviewer:qwen3-review:correctness Race condition between pgrep and ps -p can miss processes or include stale pids | The sampler first runs pgrep to get hook pids, then runs ps -p on those pids. Between these two calls, a hook process may exit (causing ps -p to fail or return fewer rows) or a new hook process may start (not captured by the pgrep snapshot). This creates a TOCTOU (time-of-check-to-time-of-use) window where the count is inconsistent. The previous single ps call was atomic with respect to the process table; the new two-step appr
info	scripts/hook_process_canary.py, default_sampler function:1	ai-reviewer:qwen3-review:performance Three subprocess calls instead of one increases latency and failure surface | The old code made one ps call; the new code makes up to three subprocess calls (ps, pgrep, ps -p). Each call has process startup overhead (fork+exec), and the shared deadline is consumed by all three. On a loaded system, the cumulative overhead may still approach the 30-second timeout even if no single call is slow. The test test_hook_process_canary_bounded_listing_one_deadline_bounds_every_call demonstrates this: two 0.7-second c
info	scripts/hook_process_canary.py, default_sampler function; tests/hooks_system/test_hook_canary_listing_omn20873.py:1	ai-reviewer:qwen3-review:completeness No test for the case where pgrep succeeds but ps -p fails with a non-zero exit code | The _listing function raises CalledProcessError if the return code is not in ok_codes. For ps -p, ok_codes is (0, 1), so exit code 1 (no such process) is treated as success. But if ps -p fails with exit code 2 (invalid syntax) or 3 (no matching processes, which is different from exit code 1 on some systems), the error is not handled. The test suite does not cover this edge case. If ps -p returns an unexpected exit code, th
info	scripts/hook_process_canary.py, _listing function:1	ai-reviewer:qwen3-review:correctness The ListingTimeout.command field is set to argv[0], which is 'ps' or 'pgrep', not the full command | The ListingTimeout dataclass has a command field that is set to argv[0] in the _listing function. This means the timeout error will report 'ps' or 'pgrep' as the command, not the full command line. This is misleading in the ledger row and ALERT.json, which will show command=ps instead of the actual ps invocation. The test test_hook_process_canary_listing_timeout_typed_lands_in_alert_json asserts command=ps,
info	tests/hooks_system/test_hook_canary_listing_omn20873.py, _fake_tools function:1	ai-reviewer:qwen3-review:testing The fake pgrep always returns pids 4002 and 4003, which may not match the actual ps output | The _fake_tools helper creates a fake pgrep that always echoes '4002\n4003', regardless of the actual ps output. This means the test is not verifying that pgrep correctly identifies hook processes from the ps output. If the ps output changes (e.g., different pids), the fake pgrep will still return 4002 and 4003, which may not be in the ps output, leading to a ps -p call that fails. The test passes because the fake p
hint	scripts/hook_process_canary.py, ListingTimeout class:1	ai-reviewer:qwen3-review:architecture The ListingTimeout dataclass is frozen but the command field is a string, not a list | The ListingTimeout dataclass has a command field that is a string (argv[0]). This is inconsistent with the other fields, which are more descriptive (error_class, timeout_seconds). The command field should either be a list of strings (the full argv) or a descriptive string that includes the full command. As it stands, it is not useful for debugging which call timed out. | Evidence: @DataClass(frozen=True) class ListingTime
error	scripts/hook_process_canary.py:1	ai-reviewer:local-studio-planner:correctness Missing command column breaks hook process detection | The new default_sampler first lists processes with ps -x -U <uid> -o pid=,ppid=,uid=, which omits the command field. The parse_ps function (inferred from the ProcRow dataclass and the old ps call) populates ProcRow.command from the 4th column. Since the command column is absent, ProcRow.command will be empty or default for all rows from the first ps call. The measure function (inferred from the test `test_hook_process_canary_bounded_
warning	scripts/hook_process_canary.py:1	ai-reviewer:local-studio-planner:security pgrep pattern injection risk | The pgrep command is constructed with _HOOK_PATH.pattern. If _HOOK_PATH.pattern contains characters that are special to pgrep or the shell (though subprocess.run with a list avoids shell injection), it could match unintended processes. More critically, pgrep -f matches against the full command line. If the pattern is not anchored or escaped correctly, it could match processes that are not hook processes, leading to false positives in the canary. The diff does not s
info	scripts/hook_process_canary.py:1	ai-reviewer:local-studio-planner:performance Multiple subprocess calls increase overhead | The new default_sampler makes up to 3 subprocess calls (ps, pgrep, ps) instead of 1. While each call is bounded by the deadline, the total time is the sum of the individual call times. If each call takes close to the deadline, the total time could exceed the intended 30-second budget. The _listing function uses a shared deadline, so the remaining time decreases with each call. This is correct, but it means the canary is more sensitive to latency. If th
info	tests/hooks_system/test_hook_canary_listing_omn20873.py:1	ai-reviewer:local-studio-planner:testing Test does not cover pgrep pattern mismatch | The tests do not cover the case where pgrep matches a process that measure does not consider a hook, or vice versa. This is a critical edge case given the reliance on pgrep for hook identification. The test test_hook_process_canary_bounded_listing_keeps_the_same_counts assumes pgrep and measure are aligned, but does not test divergence. | Evidence: def test_hook_process_canary_bounded_listing_keeps_the_same_counts(...): ... assert reading.hook_process

---

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)