[jonahgabriel]

OMN-19607: a PR merge fast-forwards the matching canonical clones, and a launchd timer catches merges made elsewhere

Ticket: OMN-19607 (parent epic OMN-19470, GitHub quota). Operator, 2026-09-25 about 14:28Z: add a hook that pulls the canonical clones after every merge. Under the same day's quota ruling, lanes query the canonical clones instead of spending API reads, which only works while the clones are current.

Why the existing sync was not enough (measured 2026-09-25)

• The only automatic clone sync on the Mac was the OMN-17190 PostToolUse tick. It delegates to omnibase_infra reconcile-host.sh, which couples the clone pull to the venv reconcile. One run held the reconcile lock from 13:25Z inside reconcile-workspace-venvs.sh, every tick after it declined (receipt log, exit 4 at 13:35Z through 14:25Z), and no clone moved for over an hour. The SessionStart line still read "in sync as of 13:15Z".
• That tick covers only the five sibling clones, not the 58 clones under both registry roots.
• The omnibase_infra ai.omninode.bare-clone-sync agent is not installed, runs the whole of pull-all.sh (venv reconcile, plugin cache, branch switches), and its plist hardcodes a volume path. Extending it would have tied clone freshness to the same heavy stages again.

What this adds

• plugins/onex/hooks/lib/canonical_clone_sync.py: one standard-library engine with two entry points.
  • hook: reads a PostToolUse payload. On a merge (gh pr merge by number, URL or branch, with or without the repo flag or GH_REPO; the REST merge endpoint by PUT; the bulk throttle's arm operation; the GitHub MCP merge tool) it resolves the repository (from the command, else from the session cwd's origin) and starts a detached sync for it. A failed or interrupted tool call, a GET of the merge endpoint, and every other gh call are ignored. An auto-merge arm waits 30 s before syncing, and the timer catches the rest.
  • sync: for every canonical clone of the repository under OMNI_HOME, each ONEX_REGISTRY_ROOTS root and each root= line of the registry-roots file (so launchd needs only OMNI_HOME), it fetches the checked-out tracking branch and runs git merge --ff-only onto the fetched sha, then re-reads HEAD and compares it with the target. It never resets, checks out, stashes, cleans or forces.
  • Refusals, each logged with its reason and nothing moved: bare, detached, on a non-tracking branch, no upstream, a merge/rebase/cherry-pick/revert/bisect in progress, a live index.lock, ahead of or diverged from upstream, staged changes, and tracked changes. The one relaxation is a shared tree that uses the scripts/commit_lock.py protocol (the registry root's own clone, rule 19). There the fast-forward runs under that commit lock, and unstaged changes refuse only when the incoming commits touch the same path, because rule 19 names the ff-only merge as that tree's sanctioned sync and its rolling ledger is almost never clean.
  • Log: one JSON line per clone in $ONEX_STATE_DIR/logs/canonical-clone-sync.jsonl (before, after and target sha, result, reason), plus a TRIGGERED line from the hook and a RUN_COMPLETE line with counts.
• plugins/onex/hooks/scripts/post_tool_use_merge_clone_sync.sh: registered on PostToolUse with matcher ^(Bash|mcp__github__merge_pull_request)$, right after the reconcile tick. For a payload without the word "merge" it exits after one bash string match. Otherwise it starts Python only to parse the payload and returns while the fetch runs detached. It exits 0 on every path and prints nothing. It is registered in hooks.json with a carve-out record (owner, reason, expiry, restoration), in the hook inventory and in the distribution manifest.
• scripts/canonical-clone-sync-tick.sh and scripts/launchd/ai.omninode.canonical-clone-sync.plist: the catch-all for merges this Mac did not make (auto-merge completions, other people). StartInterval 180 s over every clone. tick-bundle-install.sh --only <label> installs this one agent without handing the host the whole bundle.
• Neither path calls the GitHub API. git fetch is the git transport, and the REST and GraphQL rate limits do not count it.

Which branch a clone follows

The branch it has checked out. pull-all.sh leaves every clone whose origin has dev on dev and the others on main (OMN-16502), and on the release-synced repos PRs land on dev while main only moves at a release. dev is therefore where merged code first appears, and it is the right branch to query. The engine never switches branches: that is the one move the canonical-clone ref guard refuses, and pull-all is the sanctioned way to make it.

How the webhook path replaces the timer

The GitHub-quota plan (OMN-19470, ruled 2026-09-25) creates the read-only onexbot-pr-reader App, whose webhook sends pull_request and push deliveries to dev.api.omninode.ai. They reach the lab bus as onex.cmd.github.webhook-delivery.v1 through the ingress node (T2.1, OMN-19492) and the gateway forwarder (T2.4, OMN-19593). When that path is live, a clone-sync effect node on each host consumes the push or merged-PR event for its repositories and calls this same engine's sync for that one repository. The 180 s poll then stops (unregister the launchd agent) and the PostToolUse hook is retired in the same change. The engine, its refusal rules and its log stay as they are. The hooks.json carve-out record names this as its expiry and restoration.

Tests (focused, local)

• tests/hooks/test_canonical_clone_sync.py: 63 passed. Matcher cases for every verb and every ignored form. Engine cases against hermetic repos whose remotes resolve to github.com/<owner>/<name>: advance, up to date, untracked files carried, and refusals for dirty, staged, diverged, ahead, detached, lane branch, bare and mid-merge, each asserting HEAD and the working file did not move. Also main-only clones, the shared commit-lock tree (unrelated dirt carried, overlapping dirt refused), repo filtering, a NO_CLONE line for an unknown repo, all roots plus the roots file, and linked worktrees skipped. Hook cases: silent and under one second on a non-merge, on four malformed payloads and with no OMNI_HOME, the TRIGGERED line, repo resolution from cwd, and a real advance through the detached child.
• tests/hooks/test_hooks_baseline_omn_13244.py, tests/hooks/test_hook_inventory.py, tests/hooks/test_workspace_reconcile.py and the six other suites that read hooks.json PostToolUse: all passed. validate_hook_inventory.py: OK, 31 expected hooks.
• mypy --strict on the engine is clean, and pre-commit is clean on every changed file.
• tests/scripts/test_tick_bundle_install.sh exited 141 on origin/dev before this change: an awk that exits early SIGPIPEs its echo under pipefail, which kills the script under set -e, and the same pattern can read a Disabled=true template as enabled. Both awk programs now read their whole input. Tests 1 to 3 pass locally. Tests 4 and 5 write heredocs that hang in this host's sandbox, and no CI job runs this file.

Lab-first readback (this Mac, 2026-09-25)

• A manual full pass over 58 clones in both roots took 15 s: 23 ADVANCED, 7 REFUSED with correct reasons (the registry root's own clone because one uncommitted path is also changed upstream; knowledge-base-internal and omnibot dirty; RSD-private and omniios diverged; omnicursor ahead), and the rest UP_TO_DATE. The canonical-clone reference-transaction guard refused none of the advances.
• A real merge, not made by this lane: omnibase_infra dev moved to f43e7d9b3a38 (#4123, OMN-19338). At 15:31:28Z the canonical clone still sat at 21d5d62161c5, because the timer's pass at 15:29Z had lost a ref-lock race to a concurrent fetch. The hook script from this branch, fed a PostToolUse payload for a PUT to that repo's merge endpoint, returned exit 0 in 117 ms. The clone reached f43e7d9b3a38 4 s later, and the log shows ADVANCED with trigger hook, verb gh-api-merge and after equal to the merge commit. That race is why the fetch now retries once on a lost ref lock (second commit).
• launchd: the agent is installed on this Mac from this worktree, and launchctl list shows it loaded. The first pass under ProcessType Background took 200 s at host load 60, and the knowledge-base-internal fetch hit the 120 s timeout. With the default process type the next pass took 12.8 s and exited 0. Later passes at load 60 took 45 to 105 s and advanced onex_change_control in both roots (75a944dc544e to 0ffbf67cc90b) with no hook trigger involved.
• Hook wall time on a merge payload is 117 ms, just over the 100 ms PostToolUse budget. It is paid only when the payload contains "merge", and the fetch itself runs detached.

Deploy note

Hooks load from the omniclaude canonical clone's working tree, so after merge that clone needs to be at the merge commit, and a new session is needed before the hook is live. The launchd agent does that fast-forward itself. After merge, repoint the agent from this worktree to the canonical path with bash omniclaude/scripts/tick-bundle-install.sh --only ai.omninode.canonical-clone-sync.

Not runtime-affecting: the change covers session hooks and host tooling only. No node, contract or container image changes, so there is no dev-lane lab pass to run.

Evidence-Ticket: OMN-19607
Evidence-Source: OCC#11306

[onexbot-occ-writer]

No OCC evidence companion was minted for this PR.

product PR is a draft; companion suppressed (F-17)

Matched in the state: draft

To clear this: Mark the PR ready for review. The ready_for_review trigger re-fires the born path and the companion mints then.

Reported by node_occ_companion_effect (decline code pr_draft, OMN-16665). This decision was made against the PR's LIVE state at compute time, not at publish time — a green publisher job only means the command reached the broker.

[github-actions]

✅ Hostile Reviewer — PASSED

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

Findings

Severity	Location	Finding
warning	scripts/launchd/ai.omninode.canonical-clone-sync.plist:1	ai-reviewer:deepseek-r1:security Sensitive data leakage in /tmp logs | The launchd plist writes StandardOutPath and StandardErrorPath to /tmp/canonical-clone-sync-stdout.log and /tmp/canonical-clone-sync-stderr.log. /tmp is world-readable on macOS. The engine logs JSON lines containing git SHAs, repository paths, and refusal reasons to $ONEX_STATE_DIR/logs/canonical-clone-sync.jsonl, but the tick script's stderr (which may contain git error messages, file paths, or other diagnostic info) is redirected to /tmp. An attacker with local access
warning	plugins/onex/hooks/lib/canonical_clone_sync.py:1	ai-reviewer:deepseek-r1:correctness Race condition between hook and launchd tick | The PostToolUse hook and the launchd tick both invoke the same engine to fast-forward canonical clones. The engine uses a file lock (CLONE_LOCK_NAME = 'onex-canonical-clone-sync.lock') to serialize access to individual clones, but the lock is per-clone, not global. If the hook and tick run concurrently on different clones, they may both attempt to fetch from the same remote simultaneously. While git fetch is generally safe to run concurrently, the fast-forward
warning	plugins/onex/hooks/lib/canonical_clone_sync.py:1	ai-reviewer:deepseek-r1:performance ThreadPoolExecutor with MAX_WORKERS=8 may cause resource exhaustion | The engine uses ThreadPoolExecutor with MAX_WORKERS=8 to fetch and fast-forward multiple clones in parallel. Each thread spawns a git fetch subprocess, which can be CPU and I/O intensive. On a machine with many canonical clones (the docstring mentions 58 clones), 8 concurrent git fetches may saturate the disk I/O and network, causing the launchd tick to exceed its 180-second interval. The docstring notes that ProcessType Background thrott
warning	tests/hooks/test_canonical_clone_sync.py:1	ai-reviewer:deepseek-r1:completeness Missing test for stale lock cleanup | The test suite does not include a test for the case where a stale lock file (CLONE_LOCK_NAME) is present. If a process is killed while holding the lock, the lock file remains, and subsequent syncs will wait for CLONE_LOCK_WAIT_SECONDS (120s) before failing. This is a critical edge case that can cause the entire sync mechanism to be blocked for 2 minutes on every tick. The tests should verify that stale locks are detected and cleaned up, or that the lock wait times out g
info	plugins/onex/hooks/lib/canonical_clone_sync.py:1	ai-reviewer:deepseek-r1:architecture Hardcoded registry-roots path may not resolve in all environments | The engine reads registry roots from REGISTRY_ROOTS_FILE = Path('scripts') / 'git-hooks' / 'registry-roots'. This is a relative path, which is resolved against the current working directory. The hook and tick scripts set the working directory implicitly, but if the engine is invoked from a different CWD, the path may not resolve. The docstring states that the engine runs on any Python 3.12+ interpreter, but the relative path assumption is n
info	scripts/tick-bundle-install.sh:1	ai-reviewer:deepseek-r1:correctness awk script in tick-bundle-install.sh may misparse multi-line ProgramArguments | The awk script that extracts ProgramArguments[0] from a plist template uses a flag to track when it is inside the ProgramArguments block. The updated awk script uses a 'done' variable to stop after the first element, which is correct for extracting the first argument. However, if a plist has multiple elements in ProgramArguments, the script only extracts the first one, which is the intended behavior. The commen
info	tests/hooks/test_canonical_clone_sync.py:1	ai-reviewer:deepseek-r1:testing Test for hook exit code does not verify stdout silence | The test suite verifies that the hook exits 0 on every path, but it does not explicitly verify that the hook produces no stdout output. The hook's contract states 'stdout silent', but the tests only check the exit code. A regression that adds a print statement to the hook would not be caught by the existing tests. The tests should capture stdout and assert that it is empty. | Evidence: The test file tests/hooks/test_canonical_clone_sync.py contains te
hint	plugins/onex/hooks/contracts/hook_inventory.yaml:1	ai-reviewer:deepseek-r1:style Inconsistent use of 'her' in hook_inventory.yaml | The hook_inventory.yaml uses 'hermetic tests that drive the real script against local repositories' and 'OMN-19607 carries hermetic tests', but the surrounding text uses 'its' and 'the'. The use of 'her' is a gendered pronoun that is inconsistent with the rest of the document, which uses neutral or possessive forms. This is a minor style issue. | Evidence: OMN-19607 carries hermetic tests that drive the real script against local repositories. | Fix: Change
warning	plugins/onex/hooks/scripts/post_tool_use_merge_clone_sync.sh:1	ai-reviewer:qwen3-review-b:security Unvalidated shell command execution via shlex.split on tool payload | The hook script pipes the raw PostToolUse payload directly into the Python engine. The engine parses the tool input (e.g., the Bash command string) and uses shlex.split to extract arguments. If the tool input is a complex shell command containing shell metacharacters, subshells, or environment variable expansions, shlex.split may not accurately represent the execution context or may misparse arguments, leading to incorrect repository iden
warning	plugins/onex/hooks/lib/canonical_clone_sync.py:1	ai-reviewer:qwen3-review-b:correctness Race condition between launchd tick and PostToolUse hook | Both the launchd agent (running every 180s) and the PostToolUse hook invoke the same sync engine. The engine uses a file lock (CLONE_LOCK_NAME) to prevent concurrent modifications to the same clone. However, the lock is per-clone, not global. If the launchd tick is in the middle of fetching a clone when a merge occurs and the hook triggers a sync for the same clone, the hook will block for up to CLONE_LOCK_WAIT_SECONDS (120s). Given that the h
info	plugins/onex/hooks/scripts/post_tool_use_merge_clone_sync.sh:1	ai-reviewer:qwen3-review-b:performance Inefficient string matching in hook script | The hook script uses a case statement to check if the payload contains the substring 'merge'. This is a broad match that will trigger the Python engine for any tool call that includes the word 'merge' in its output or input, not just actual merge operations. This could lead to unnecessary Python invocations and log entries for non-merge operations that happen to contain the word 'merge' (e.g., a git commit message mentioning 'merge'). | Evidence: plugins/onex/h
info	plugins/onex/hooks/scripts/post_tool_use_merge_clone_sync.sh:1	ai-reviewer:qwen3-review-b:completeness Missing error handling for Python interpreter resolution | The hook script attempts to find a Python interpreter by checking PLUGIN_PYTHON_BIN, python3.13, and python3. If none of these are available, the script exits silently. This could lead to the hook failing silently in environments where Python is not installed or not in the expected location, without any logging or alerting. | Evidence: plugins/onex/hooks/scripts/post_tool_use_merge_clone_sync.sh: [[ -n "$_py" ]] || exit 0 | Fix: Log a warnin
info	tests/hooks/test_canonical_clone_sync.py:1	ai-reviewer:qwen3-review-b:testing Insufficient test coverage for concurrent lock scenarios | The test suite includes hermetic tests for the engine and hook, but there is no explicit test for the race condition between the launchd tick and the PostToolUse hook when they attempt to sync the same clone concurrently. The tests do not verify that the lock mechanism prevents data corruption or that the hook does not block indefinitely. | Evidence: tests/hooks/test_canonical_clone_sync.py: No tests found that simulate concurrent lock acquisition.
hint	plugins/onex/hooks/lib/canonical_clone_sync.py:1	ai-reviewer:qwen3-review-b:architecture Hardcoded path to registry-roots file | The engine uses a hardcoded relative path Path("scripts") / "git-hooks" / "registry-roots" to locate the registry roots file. This assumes a specific directory structure and may fail if the engine is run from a different working directory or if the file is located elsewhere. | Evidence: plugins/onex/hooks/lib/canonical_clone_sync.py: REGISTRY_ROOTS_FILE = Path("scripts") / "git-hooks" / "registry-roots" | Fix: Use an environment variable or a configuration file to

---

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]

Fix commit eecdcc5 (landing lane repo-drain-omniclaude, pass 1).

Real red diagnosed on the GitHub-hosted `Hooks System Tests` job (ubuntu-24.04 runner): 3 of the new suite's tests failed deterministically, both on the original head and on a rerun -
`test_hook_records_the_trigger_and_returns_fast`, `test_hook_resolves_the_repo_from_the_session_cwd` (both `ValueError: not enough values to unpack (expected 1, got 0)`), and `test_hook_advances_the_clone_through_the_detached_child` (asserted the wrong head sha).

Root cause: `Registry.env()` in the test fixture copies the full ambient `os.environ`, so it silently inherited `OMNICLAUDE_MODE=full` from the authoring machine's shell profile. `mode.sh`'s auto-detect falls back to `$PWD` (full only under a canonical-registry checkout) and then an `omnibase_core` dev-install probe, defaulting to "lite" otherwise. A bare CI checkout's cwd matches neither, so mode resolved to "lite" and `post_tool_use_merge_clone_sync.sh` exited before ever invoking the Python engine - no TRIGGERED log row, no detached sync child, exactly the three observed failures.

Reproduced locally: ran the three tests from a cwd outside any canonical-registry directory with `OMNICLAUDE_MODE` unset - all three failed with the identical errors seen in CI. Fix: `Registry.env()` now pins `OMNICLAUDE_MODE=full` explicitly so the tests assert on the hook's own logic rather than the invoking machine's directory layout or shell profile.

Verification (worktree at the exact head, clean tree):

• `tests/hooks/test_canonical_clone_sync.py`: 63 passed (including the three previously-failing tests, both in-place and reproduced from an external cwd with the env var unset)
• `ruff check` + `ruff format --check` on the changed file: clean
• `mypy` on the changed file: clean
• Local `pre-commit run` (via `git commit`, not `--no-verify`): all hooks passed, including `public-repo-hygiene`, `mypy`, `pyright`

Not runtime-affecting (session hooks + host tooling only, matches the PR body's own classification). Companion `onex_change_control#11306` (Evidence-Source autobind) already merged.