[jonahgabriel]

OMN-17427: Add drainer alarm to session_start_workspace_sync.sh

Base: dev

Summary

Adds _drainer_alarm to plugins/onex/hooks/scripts/session_start_workspace_sync.sh. It runs beside the existing load-path alarm and above the session-intent gate, so quiet/tick intent does not silence it.

Behavior

Prints an ALARM line when both hold:

1. Drainer definition is absent:
   • Linux: ~/.config/systemd/user/ai.omninode.hook-emit-drainer.service (honouring XDG_CONFIG_HOME)
   • macOS: ~/Library/LaunchAgents/ai.omninode.hook-emit-drainer.plist
2. Journal holds at least one *.json record.

Journal dir resolution: ONEX_HOOK_EMIT_JOURNAL_DIR → ONEX_STATE_DIR/hook_emit_journal → ~/.onex_state/hook_emit_journal.

The alarm names the missing definition path, the journal path, and the repair: bash $OMNI_HOME/omniclaude/scripts/install-hook-emit-drainer.sh.

Cost

One stat on a healthy host; a find that stops at the first record only on an already-wrong host. Pure bash, exit 0 always.

Motivation

On 2026-10-07, host h202 had 38,653 pending hook journal records (oldest 2026-09-30) with no drainer unit, while h201 drained. The installer is a manual operator step; no bring-up path runs it, so nothing on h202 reported the gap.

Limit

Detects a never-installed drainer only. Does not install it; does not detect an installed-but-stuck drainer.

Tests

tests/hooks/test_workspace_reconcile.py gains 6 tests. Four alarm-positive tests fail against unmodified script, pass with change. Result: 91 passed across test_workspace_reconcile.py and test_hooks_baseline_omn_13244.py. Pre-commit clean on both changed files.

Evidence-Ticket: OMN-17427
Evidence-Source: OCC#13241

[jonahgabriel]

Lab: host=h202; lane=h202-hook-drain-2135; command: pytest tests/hooks/test_workspace_reconcile.py tests/hooks/test_hooks_baseline_omn_13244.py; observed: 91 passed; the 4 alarm-positive new tests fail against the unmodified script and pass with the change; pre-commit clean; head=c806e8f1d85d8446a5749c24c382a87dcab017f0

[github-actions]

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 11
Models succeeded: qwen3-review,gpt-oss-review

Findings

Severity	Location	Finding
warning	plugins/onex/hooks/scripts/session_start_workspace_sync.sh:1	ai-reviewer:qwen3-review:security Unsanitized environment variable interpolated into shell command string | The _drainer_alarm function interpolates the OMNI_HOME environment variable directly into a double-quoted shell command string that is printed to stdout. If OMNI_HOME contains shell metacharacters (e.g., $(rm -rf ~) or backticks), the printed repair instruction becomes a shell injection vector if the user copy-pastes it into a terminal. The variable is read from the environment without any sanitization or quoting of its value
warning	plugins/onex/hooks/scripts/session_start_workspace_sync.sh:1	ai-reviewer:qwen3-review:correctness find -quit returns empty string when no match, breaking the alarm condition | The line [[ -n "$(find "$journal" -maxdepth 1 -name '*.json' -print -quit 2>/dev/null) ]] || return 0 uses find -print -quit. When no matching file exists, find prints nothing and exits 0, so the command substitution yields an empty string, the test is false, and the function returns 0 (silent). This is the intended behavior for the 'no records' case. However, when the journal directory does not exist, the earlier `[[ -d "$j
info	plugins/onex/hooks/scripts/session_start_workspace_sync.sh:1	ai-reviewer:qwen3-review:performance find invoked on every session start even when journal is absent | The _drainer_alarm function runs on every invocation of the session-start hook. On hosts where the journal directory does not exist, the [[ -d "$journal" ]] check short-circuits, which is fine. However, on hosts where the journal directory exists but is empty, find is still invoked. More importantly, the function is called unconditionally via _drainer_alarm || true with no gating on whether the journal subsystem is actually active. If
info	tests/hooks/test_workspace_reconcile.py:1	ai-reviewer:qwen3-review:completeness Test does not cover the case where ONEX_STATE_DIR is set but journal subdirectory is absent | The test suite covers the default journal path, the ONEX_HOOK_EMIT_JOURNAL_DIR override, and the empty-journal case. It does not cover the case where ONEX_STATE_DIR is set to a custom path and the hook_emit_journal subdirectory does not exist under it. The shell script resolves the journal path via a three-way cascade (ONEX_HOOK_EMIT_JOURNAL_DIR, ONEX_STATE_DIR, HOME), and the test only exercises two of the three b
info	tests/hooks/test_workspace_reconcile.py:1	ai-reviewer:qwen3-review:correctness Test _drainer_env pops ONEX_STATE_DIR implicitly but does not set it, leaving host leakage | The _drainer_env helper pops XDG_CONFIG_HOME and ONEX_HOOK_EMIT_JOURNAL_DIR from the environment to prevent host leakage, but it does not pop ONEX_STATE_DIR. If the host running pytest has ONEX_STATE_DIR set, the test's environment will inherit it, and the shell script's journal path resolution will use the host's ONEX_STATE_DIR instead of the test's controlled path. This can cause the test to pass or fail
hint	plugins/onex/.claude-plugin/plugin.json:1	ai-reviewer:qwen3-review:style Version bump skips 2.4.51 without explanation | The version is bumped from 2.4.50 to 2.4.52 in both marketplace.json and plugin.json, skipping 2.4.51. There is no changelog entry or comment explaining the skipped version. This may indicate a reverted or abandoned 2.4.51 release, but the diff provides no context. | Evidence: "version": "2.4.50", -> "version": "2.4.52", | Fix: Add a changelog entry or commit message note explaining why 2.4.51 was skipped, or use 2.4.51 if the skip was accidental.
warning	plugins/onex/hooks/scripts/session_start_workspace_sync.sh:1	ai-reviewer:gpt-oss-review:performance Repeated filesystem scan on each hook invocation | The newly added _drainer_alarm function executes a find command on the journal directory for every session start hook execution. On hosts with large numbers of journal files this introduces O(N) filesystem traversal latency, potentially slowing down hook processing and affecting overall system responsiveness. | Evidence: Line added in session_start_workspace_sync.sh: `[[ -n "$(find "$journal" -maxdepth 1 -name '*.json' -print -quit 2>/dev/null)" ]] || ret
info	plugins/onex/hooks/scripts/session_start_workspace_sync.sh:1	ai-reviewer:gpt-oss-review:security Potential path injection via environment-controlled journal directory | The journal directory path is derived from environment variables (ONEX_HOOK_EMIT_JOURNAL_DIR, ONEX_STATE_DIR, HOME) without validation. A malicious user could set these variables to a path containing special characters or symlinks that cause the find command to traverse unintended locations, leading to information disclosure. | Evidence: Lines constructing journal variable based on untrusted env vars and later used in `find "$journa
info	plugins/onex/hooks/scripts/session_start_workspace_sync.sh:1	ai-reviewer:gpt-oss-review:correctness Missing error handling for find failures | find errors (e.g., permission denied) are suppressed to /dev/null and the function proceeds as if no records exist, potentially hiding a real alarm condition. | Evidence: find "$journal" -maxdepth 1 -name '*.json' -print -quit 2>/dev/null | Fix: Capture the exit status of find; if non‑zero, log a warning or treat it as an alarm condition.
info	plugins/onex/hooks/scripts/session_start_workspace_sync.sh:1	ai-reviewer:gpt-oss-review:architecture Race condition between drainer definition check and journal state | The function first checks for the existence of the drainer unit file, then separately checks for journal records. If the drainer is installed concurrently after the first check but before the alarm is raised, the hook may emit a false alarm. | Evidence: Sequence: [[ -e "$definition" ]] && return 0 followed by journal existence checks. | Fix: Re‑evaluate the drainer definition existence after confirming journal records, or perform both che
info	plan:1	ai-reviewer:gpt-oss-review:completeness Version bump not reflected in dependent metadata | The plugin version is updated from 2.4.50 to 2.4.52 in two manifest files, but any external tooling that pins to the previous version may now encounter mismatched expectations. | Evidence: Changes in plugins/onex-dev-marketplace/.claude-plugin/marketplace.json and plugins/onex/.claude-plugin/plugin.json. | Fix: Audit all CI/CD pipelines, deployment scripts, and documentation for hard‑coded version references and update them accordingly.

---

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)

[jonahgabriel]

Lab: host=h201; lane=cause-claude-bump-2245; command: pytest tests/hooks/test_workspace_reconcile.py tests/hooks/test_hooks_baseline_omn_13244.py, then python3 scripts/validation/validate_plugin_version_bump.py --base origin/dev; observed: 91 passed; the plugin version bump gate exits 0 with plugin.json and the dev marketplace entry at 2.4.52 over dev 2.4.49 (dev 246826a merged in by a merge commit); pre-commit clean; head=ce3454c51f0b5823a2ab23d528c24c79b2c34865