[jonahgabriel]

Ticket: OMN-19587 (GitHub quota T0.7, epic OMN-19470). Authority: RULING 2026-09-25T14:18:52Z, omni_home ledger line 4167. Operator answered Q3 yes: lane reads move to a read-only App token, and writes stay on the operator account.

What this adds

scripts/user-bin/gh_route.py is a stdlib-only router the gh user shim calls. The shim itself comes from T0.2 and T0.5 (OMN-19479, OMN-19585), which lane gh-quota-phase0-83 owns. This PR does not create or edit scripts/user-bin/gh.

Classification. The router classifies each call from argv.

Reads:

• pr view|checks|list|diff, run view|list, repo view and issue view|list, aimed at an OmniNode-ai repository. The repository comes from -R/--repo, a github.com URL argument, GH_REPO, or the cwd's git remotes.
• gh search scoped to the organization.
• gh api with an effective method of GET under repos/OmniNode-ai/, or under search/ with an organization-scoped q.
• GraphQL that has no mutation, no viewer, and names no owner outside the organization.

Everything else runs with argv and env unchanged: merge, edit, create, comment, labels, any non-GET method, mutations, and auth and user endpoints.

Some reads always stay on the operator account:

• Viewer-resolving calls (@me, viewer, pr status). On an installation token they would answer as the bot.
• Unscoped search. On the App it would return fewer results with no error.

Token. A read runs with GH_TOKEN set to the token that GH_READ_TOKEN_CMD prints. That command is the T0.8 mint, OMN-19588.

• Only an installation token (ghs_) is accepted.
• It is cached mode 0600 at ${XDG_CACHE_HOME:-$HOME/.cache}/omni/gh-read-token.json for at most 50 minutes.
• It is minted under an fcntl lock, so a burst of calls mints once.
• It is never logged, never put in argv and never printed.

Fallback. The fallback is recorded and never hidden. When there is no token (command unset, missing, failing, timing out, printing a non-installation token, or no cache home), the read runs on the operator account and is recorded as operator-fallback with the reason. When the App answers "Resource not accessible by integration", the call is retried once on the operator account and recorded the same way.

Off by default. Nothing routes unless ONEX_GH_READ_ROUTING=1. The App does not exist yet. The operator steps and the readback that must pass before the flag is turned on are in knowledge-base-internal#780.

Entry points.

• route_and_run(argv, real_gh, env, recorder).
• gh_route.py exec [--result-file F] -- <gh argv>. With --result-file, it writes identity reason for the bash shim to add to its own usage-log line.
• gh_route.py classify -- <gh argv>, which prints the decision as JSON.

The real gh is resolved on PATH past this directory and past any file carrying the ONEX_GH_USER_SHIM marker.

The follow-up is a small hook at the shim's pass-through site, stacked on the T0.2 and T0.5 PRs, as agreed by MSG (ledger rows 2026-09-25T14:23:58Z and 14:47:43Z).

Tests (RED first)

tests/scripts/test_gh_read_routing.py has 76 tests, all passing. They run against a fake real gh that records its argv and token, and a fake mint that counts its runs.

• RED: the suite failed at collection with no module (FileNotFoundError) before the implementation.
• The falsifiers select as follows. -k mutation selects 36, -k fallback 9 and -k cache 8. -k read selects the whole file, because the file name contains "read".
• Positive control: treating POST as a read (the method check replaced with if False) made 6 -k mutation cases fail.
• ruff format, ruff check and mypy --strict scripts/user-bin/gh_route.py are clean.

Lab-first readback (Mac harness change; plan section 4 says proof on this Mac)

• classify against real argv:
  • pr view 4079 -R OmniNode-ai/omnibase_infra is a read.
  • pr merge is not a read.
  • api graphql -f query={viewer{login}} is viewer-resolving.
  • api repos/OmniNode-ai/omniclaude/pulls is read-rest.
  • A bare pr view inside a clone resolves OmniNode-ai/omniclaude from the remotes.
• exec against the real gh with routing disabled served one read on the operator bucket (X-RateLimit-Used 1078). With routing on and no token command, it served the next read on the operator bucket (Used 1080) and logged identity=operator-fallback reason=token-cmd-unset. No token was involved.
• Not yet proven: an App-token read. That needs the App (operator browser step), T1.2 and T0.8.

Commit note

The omniclaude no-cloud-bus pre-commit hook hangs indefinitely under Homebrew bash 5.3.9. Under /bin/bash 3.2 it runs in 0.3 s with rc 0; under /opt/homebrew/bin/bash it times out at 60 s. The commit therefore ran with PATH prefixed by a directory whose bash links to /bin/bash. Every hook ran and passed, with no skip and no --no-verify. FRICTION is recorded in the ledger.

Runtime-affecting: no. This lane does not arm, merge or poll CI.

Evidence-Ticket: OMN-19587
Evidence-Source: OCC#11287

[github-actions]

✅ Hostile Reviewer — PASSED

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

Findings

Severity	Location	Finding
warning	scripts/user-bin/gh_route.py:1	ai-reviewer:deepseek-r1:security GraphQL owner check misses variable references and unquoted literals | The _GQL_OWNER_LITERAL regex only matches double-quoted string literals (owner: "..."). GraphQL queries can use single quotes, variable references ($owner), or unquoted enum values. A query like query($o: String!) { repository(owner: $o, name: "x") { id } } will pass the owner check because the variable is not a string literal, yet the variable value is not validated against the org. Similarly, repository(owner: 'OmniNode-ai', ...) w
warning	scripts/user-bin/gh_route.py:1	ai-reviewer:deepseek-r1:correctness API endpoint parsing misidentifies flags as positional args | In _classify_api, the loop that parses arguments does not correctly handle all flag forms. For example, gh api -i https://api.github.com/... is tested as a read, but the parser treats -i as a value flag only if it is in _API_VALUE_FLAGS. However, -i is not in _API_VALUE_FLAGS, so it is treated as a positional argument. The endpoint becomes -i, which is then checked against repos/ or search/ prefixes and fails, causing the call to be r
warning	scripts/user-bin/gh_route.py:1	ai-reviewer:deepseek-r1:correctness Short flag parsing in _classify_api is broken for combined flags | The code arg[:2] in {"-X", "-f", "-F", "-H", "-q", "-t", "-p"} assumes that short flags are always two characters. However, gh api -XPUT is a valid combined flag (method PUT), and the parser will treat -X as the flag and PUT as the value, which is correct. But gh api -fstate=open (no space) will be parsed as -f with value state=open, which is correct. The issue is that the parser does not handle the case where a short flag is c
warning	scripts/user-bin/gh_route.py:1	ai-reviewer:deepseek-r1:security Token cache file is not protected against symlink attacks | The _write_cached function creates a temporary file and then renames it to the cache file. However, if an attacker can create a symlink at the cache file path before the rename, the rename will follow the symlink and overwrite the attacker's file. This is a classic symlink attack. The code does not use O_NOFOLLOW or check for symlinks before writing. | Evidence: tmp.replace(path) | Fix: Use O_NOFOLLOW when opening the temporary file, and check that
info	scripts/user-bin/gh_route.py:1	ai-reviewer:deepseek-r1:performance Git remote resolution is called on every classification | The _remote_repo function is called for every classification that does not have an explicit repo, which means it runs git remote -v on every gh call. This is a subprocess call that can be slow, especially in a large repository. The result is not cached, so it is repeated on every call. This can add significant latency to every gh invocation. | Evidence: def _remote_repo(env: Mapping[str, str]) -> str | None: try: out = subprocess.run(
info	tests/scripts/test_gh_read_routing.py:1	ai-reviewer:deepseek-r1:testing No test for the case where the App token is expired but the cache is still valid | The test test_cache_expires_after_fifty_minutes tests that the cache expires after 50 minutes, but it does not test the case where the App token itself is expired (i.e., the token is still in the cache but the App has revoked it). In this case, the router will use the expired token, and the App will return an error. The router will then fall back to the operator, but the fallback reason will be 'integration-refused', not 'tok
info	scripts/user-bin/gh_route.py:1	ai-reviewer:deepseek-r1:completeness No handling for the case where the real gh is not found | The main function returns 127 if the real gh is not found, but it does not log this event. The router is expected to log every call, but this case is not logged. This is a gap in the logging. | Evidence: if real_gh is None: sys.stderr.write("gh_route: no real gh found on PATH past the shim\n") return 127 | Fix: Log this event to the usage log, or at least to stderr with a clear message. Consider adding a test for this case.
hint	scripts/user-bin/gh_route.py:1	ai-reviewer:deepseek-r1:style Inconsistent use of type hints | The code uses str | None in some places and Optional[str] in others. This is inconsistent and can be confusing. The code should use a consistent style. | Evidence: repo: str | None | Fix: Use a consistent style for type hints. Consider using Optional[str] for all optional types, or str | None for all of them.
error	scripts/user-bin/gh_route.py: _classify_api, graphql branch:1	ai-reviewer:qwen3-review-b:security GraphQL classification bypass via variable definitions | The GraphQL classification logic only inspects owner literals embedded directly in the query string and values from typed fields (-F/--field). It does not inspect variable definitions or variable values passed via untyped fields (-f/--raw-field). An attacker or user can define a variable for the owner and pass its value via -f, bypassing the owner check. For example, `gh api graphql -f 'query=query($o: String!) { repository(owner: $o, name: "x") { id
warning	scripts/user-bin/gh_route.py: _classify_api, graphql branch:1	ai-reviewer:qwen3-review-b:security GraphQL search scope check is incomplete | The check for unscoped GraphQL search only inspects the query string and field values for org:/repo:/user: qualifiers. However, GraphQL search queries can use variables for the search query string. If the q parameter is a variable, its value is not inspected, and the search may be unscoped. The code checks re.search(r"\bsearch\s*\(", query) and then calls _search_scoped(texts, []) where texts includes the query and field values, but if the search quer
warning	scripts/user-bin/gh_route.py: _classify_api, REST branch:1	ai-reviewer:qwen3-review-b:correctness API endpoint classification misses URL-encoded paths | The API endpoint classification splits the path on '/' and checks if the first segment is 'repos' or 'search'. However, if the endpoint is a full URL (e.g., https://api.github.com/repos/OmniNode-ai/x/pulls), the code strips the scheme and host, but does not handle URL-encoded characters in the path. More critically, if the endpoint is a relative path with a leading slash (e.g., /repos/OmniNode-ai/x/pulls), the code strips the leading slash, but if t
warning	scripts/user-bin/gh_route.py: _classify_api, argument parsing:1	ai-reviewer:qwen3-review-b:correctness Flag value parsing is fragile for combined short flags | The _flag_value function and the API argument parser assume that short flags are either standalone or combined with their value (e.g., -Rx). However, gh api supports combined short flags where multiple flags are grouped (e.g., -XGET is handled, but -fkey=val is not). The code checks arg[:2] in {"-X", "-f", ...} which only matches if the first two characters are exactly the flag. For -fkey=val, arg[:2] is -f, so it works, but for `-XG
info	scripts/user-bin/gh_route.py: _remote_repo:1	ai-reviewer:qwen3-review-b:performance Git remote resolution is called on every classification | The _remote_repo function is called for every classification that does not have an explicit repo, URL, or GH_REPO. This spawns a git remote -v subprocess with a 5-second timeout. If the user is in a directory without a git repo, this will fail quickly, but if they are in a git repo, it will spawn a subprocess on every call. This can be a performance issue if the router is called frequently. | Evidence: def _remote_repo(env: Mapping[str, str]) ->
info	tests/scripts/test_gh_read_routing.py: READS:1	ai-reviewer:qwen3-review-b:completeness Missing test for GraphQL variable bypass | The test suite does not include a test for the GraphQL variable bypass described in the first finding. The tests only cover literal owner values in the query string and typed field values, but not variable definitions. | Evidence: The READS list does not include a case with a variable for the owner. | Fix: Add a test case that uses a variable for the owner in a GraphQL query and verify that it is classified as operator-only.
info	scripts/user-bin/gh_route.py: _write_cached:1	ai-reviewer:qwen3-review-b:correctness Token cache file is not atomic on all platforms | The _write_cached function writes to a temporary file and then renames it to the final name. This is atomic on POSIX systems, but on Windows, os.rename may fail if the destination file exists. The code uses tmp.replace(path) which is atomic on POSIX but may not be on Windows. Since the code uses fcntl (POSIX-only), this is not an issue, but the code should be documented as POSIX-only. | Evidence: tmp.replace(path) | Fix: Add a comment that the code i
info	scripts/user-bin/gh_route.py: acquire_read_token:1	ai-reviewer:qwen3-review-b:security Token command output is not validated for newlines | The acquire_read_token function reads the token from the stdout of the mint command and strips whitespace. If the mint command outputs multiple lines, the token will include newlines, which will fail the _INSTALLATION_SHAPE regex. However, if the mint command outputs a token followed by a newline and then other text, the token will be the first line, which is correct. But if the mint command outputs a token with trailing whitespace, the strip() will

---

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)