Skip to content

Commit 8cc1bf8

Browse files
committed
PR Graphify-Labs#3506: feat(omp): bridge hook-guard denials and context through native hooks
Squashed from origin/feat/omp-native-integration (0fd7486) onto upstream/v8.
1 parent 4c73561 commit 8cc1bf8

15 files changed

Lines changed: 1569 additions & 5 deletions

‎.github/workflows/ci.yml‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,71 @@ jobs:
7878
uv run --frozen graphify --help
7979
uv run --frozen graphify install
8080
81+
omp-bridge:
82+
# tests/omp.test.ts drives the real OMP extension: bun resolves the
83+
# path-utils helpers it imports from the pinned @oh-my-pi/pi-coding-agent
84+
# devDependency in graphify/omp/ (the only package the host provides;
85+
# graphify/omp/index.ts vendors isReadableUrlPath locally instead of
86+
# importing it from @oh-my-pi/pi-tui, which the host never resolves), and
87+
# the policy assertions run against the real installed Python CLI, so
88+
# this job needs both toolchains.
89+
runs-on: ubuntu-latest
90+
steps:
91+
- uses: actions/checkout@v6
92+
93+
- name: Install uv
94+
uses: astral-sh/setup-uv@v8.1.0
95+
with:
96+
python-version: "3.12"
97+
98+
- name: Install bun
99+
uses: oven-sh/setup-bun@v2
100+
101+
- name: Install dependencies
102+
run: uv sync --frozen
103+
104+
- name: Install OMP extension dependencies
105+
run: bun install --frozen-lockfile
106+
working-directory: graphify/omp
107+
108+
- name: Run OMP bridge tests
109+
run: GRAPHIFY_TEST_CLI=$PWD/.venv/bin/graphify bun test tests/omp.test.ts
110+
111+
- name: Check for upstream symbol moves (advisory, no execution)
112+
# The frozen run above only proves the bridge works against the
113+
# exact pinned version. Upstream OMP has moved exported symbols
114+
# between minor releases before (isReadableUrlPath moved from
115+
# pi-coding-agent to pi-tui) without the frozen lockfile ever seeing
116+
# it; index.ts now vendors that predicate locally instead of
117+
# tracking whichever package currently exports it. This step used
118+
# to install @latest AND execute the whole suite against it --
119+
# unpinned, mutable third-party code running on every CI invocation.
120+
# It now only downloads the published package (--ignore-scripts:
121+
# no install hooks run) and statically compares index.ts's imported
122+
# path-utils symbols against that file's declared exports as plain
123+
# text -- the downloaded code is never imported or executed, but a
124+
# future symbol move still surfaces here.
125+
# continue-on-error: an unrelated upstream release must not block a
126+
# merge; the frozen run above stays the blocking gate.
127+
continue-on-error: true
128+
run: |
129+
bun add --cwd graphify/omp -d @oh-my-pi/pi-coding-agent@latest --ignore-scripts
130+
bun -e '
131+
const fs = require("node:fs");
132+
const index = fs.readFileSync("graphify/omp/index.ts", "utf8");
133+
const block = index.match(/import\s*{([^}]+)}\s*from\s*"@oh-my-pi\/pi-coding-agent\/tools\/path-utils"/);
134+
if (!block) throw new Error("no path-utils import block found in index.ts");
135+
const imported = block[1].split(",").map((s) => s.trim()).filter(Boolean);
136+
const source = fs.readFileSync("graphify/omp/node_modules/@oh-my-pi/pi-coding-agent/src/tools/path-utils.ts", "utf8");
137+
const exported = new Set([...source.matchAll(/^export\s+(?:async\s+function|function)\s+(\w+)/gm)].map((m) => m[1]));
138+
const missing = imported.filter((name) => !exported.has(name));
139+
if (missing.length) {
140+
console.error(`@oh-my-pi/pi-coding-agent@latest no longer exports: ${missing.join(", ")}`);
141+
process.exit(1);
142+
}
143+
console.log(`ok: every imported symbol is exported by @latest (${imported.join(", ")})`);
144+
'
145+
81146
security-scan:
82147
# The dev deps include bandit and pip-audit. Run them in CI so a new
83148
# HIGH-severity finding or vulnerable dependency is caught on the PR that

‎.gitignore‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,9 @@ paper/
4040

4141
# macOS Finder metadata
4242
.DS_Store
43+
44+
# OMP extension test dependency tree (graphify/omp)
45+
graphify/omp/node_modules/
46+
47+
# Local symlink for the root-level bun tests (temporary, per AGENTS.md)
48+
/node_modules

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ Full release notes with details on each version: [GitHub Releases](https://githu
103103

104104
## 0.9.58 (2026-09-10)
105105

106+
- Feature: ship a native Oh My Pi guard package with `graphify omp install`; reuse the installed hook-guard policy for bounded tool-call denials and per-call guidance delivered with each tool result as a leading `<system-reminder source="graphify">` block, matching how Claude Code labels PreToolUse context, without building indexes.
106107
- Fix: a call to a Python function defined nested inside another function now resolves to that inner definition per lexical scope, instead of leaking to a same-named function elsewhere; direct recursion is preserved as a self-loop (#3410, thanks @hopstreax).
107108
- Fix: submodule imports inside a PEP 420 namespace package (a directory with no `__init__.py`) now resolve to the target module instead of being dropped (#3429, thanks @flaukowski).
108109
- Fix: a bare-name import of a module sitting next to the importing file (a flat script dir with no package) now resolves to that sibling — matching CPython's `sys.path[0]` behavior — without over-resolving a genuine third-party name (#3430, thanks @hopstreax).

‎README.md‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ for example `graphify claude install --project` or `graphify codex install --pro
232232
| Agent Skills (cross-framework) | `graphify install --platform agents` (alias `--platform skills`) |
233233
| Kiro IDE/CLI | `graphify kiro install` |
234234
| Pi coding agent | `graphify install --platform pi` |
235+
| Oh My Pi (native guard) | `graphify omp install` |
235236
| Cursor | `graphify cursor install` |
236237
| Devin CLI | `graphify devin install` |
237238
| Google Antigravity | `graphify antigravity install` |
@@ -305,6 +306,7 @@ Run this once in your project after building a graph:
305306
| Agent Skills (cross-framework) | `graphify agents install` (alias `graphify skills install`) |
306307
| Kiro IDE/CLI | `graphify kiro install` |
307308
| Pi coding agent | `graphify pi install` |
309+
| Oh My Pi (native guard) | `graphify omp install` |
308310
| Devin CLI | `graphify devin install` |
309311
| Google Antigravity | `graphify antigravity install` |
310312

@@ -323,7 +325,55 @@ This writes a small config file that tells your assistant to consult the knowled
323325

324326
**Cursor** writes `.cursor/rules/graphify.mdc` with `alwaysApply: true`, so Cursor includes it in every conversation automatically, no hook needed.
325327

326-
To remove graphify from all platforms at once: `graphify uninstall` (add `--purge` to also delete `graphify-out/`). Or use the per-platform command (e.g. `graphify claude uninstall`).
328+
### Oh My Pi (OMP) native guard
329+
330+
Install Graphify into a persistent environment with `uv tool install graphifyy` or
331+
`pipx install graphifyy`, and install OMP separately. Then run:
332+
333+
```bash
334+
graphify omp install
335+
```
336+
337+
This delegates to OMP's supported `omp plugin install <local-package-directory>`
338+
route. The Python wheel and source distribution contain `graphify/omp/package.json`
339+
with explicit `omp.extensions` and its TypeScript entry point; OMP links that
340+
directory and discovers the extension. No npm adapter package, shell hook, Pi API
341+
shim, or hand-edited OMP settings are required. `graphify omp path` prints the same
342+
directory for manual `omp plugin install` or one-session `omp -e` use. Restart OMP
343+
after installing/upgrading, and rerun the installer if the Python environment
344+
moves. Do not link from an ephemeral `uvx` environment.
345+
346+
Before native `read`, `glob`, `grep`, and search-style `bash` calls, the extension
347+
runs the installed `graphify hook-guard read|search` CLI. The existing Python
348+
policy owns fresh/stale graph decisions and strict-mode denials: start OMP with
349+
`GRAPHIFY_HOOK_STRICT=1` to enable its once-per-session indexed-read block.
350+
Denials become OMP `block`/`reason`; each qualifying call carries its own
351+
guidance, appended to that call's persisted tool result (Claude
352+
`PreToolUse` additionalContext parity), and pending deliveries are cleared for
353+
each new user run and session navigation. In-flight hooks
354+
are cancelled on these boundaries. No graph is created or updated automatically.
355+
The guard package intentionally declares no skills; the existing cross-framework
356+
skill remains available separately through `graphify agents install`.
357+
358+
Only local filesystem targets are inspected, using OMP's selector/path helpers.
359+
URLs and internal resources are excluded. Hooks require an installed `graphify`
360+
on an absolute PATH entry outside the project (including outside a project-local
361+
virtualenv); there is no project Python or command fallback. Each native tool call
362+
has a 256 KiB JSON-input cap, a 64 KiB output cap and a two-second subprocess budget
363+
with forced termination. Missing commands, invalid output and failures fail open.
364+
365+
The extension checks `ctx.isProjectTrusted()` before execution and context
366+
injection, but **current OMP exposes this compatibility method as always true**:
367+
it is not an enforced trust sandbox. Enable this integration only in projects you
368+
trust. The lifecycle contract is based on OMP integration commit
369+
`6aef0e8ad51b3bc5ea7a5f2a255c3d48e4c5af72`; the `18.1.17` version string alone
370+
does not establish those lifecycle fixes.
371+
372+
Remove this host-managed link with `omp plugin uninstall graphify-omp` **before**
373+
uninstalling the Python package. `graphify uninstall` handles Graphify-managed
374+
platform files, not OMP's plugin registry.
375+
376+
To remove Graphify-managed platform files at once: `graphify uninstall` (add `--purge` to also delete `graphify-out/`). Or use the per-platform command (e.g. `graphify claude uninstall`). OMP links are removed separately as described above.
327377

328378
---
329379

@@ -764,6 +814,9 @@ graphify kiro install # .kiro/skills/ + .kiro/steering/graphify.md
764814
graphify kiro uninstall
765815
graphify pi install # skill file (Pi coding agent)
766816
graphify pi uninstall
817+
graphify omp install # native guard package, registered by OMP
818+
graphify omp path # shipped package directory for manual linking
819+
omp plugin uninstall graphify-omp # remove the host-managed link
767820
graphify devin install # skill file + .windsurf/rules/graphify.md (Devin CLI)
768821
graphify devin uninstall
769822
graphify antigravity install # .agents/rules + .agents/workflows (Google Antigravity)

‎graphify/__main__.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ def _run_cli() -> None:
572572
# Skip during install/uninstall (hook writes trigger a fresh check anyway).
573573
# Skip during hook-check — it runs on every editor tool use and must be silent.
574574
# Deduplicate paths so platforms sharing the same install dir don't warn twice.
575-
_silent_cmds = {"install", "uninstall", "hook-check", "hook-guard"}
575+
_silent_cmds = {"install", "uninstall", "hook-check", "hook-guard", "omp"}
576576
if not any(arg in _silent_cmds for arg in sys.argv):
577577
# Resolve each platform's real user-scope destination so per-platform
578578
# overrides (gemini, opencode, devin, antigravity, amp) check the dir
@@ -591,6 +591,7 @@ def _run_cli() -> None:
591591
print(" install [--platform P] copy skill to platform config dir (claude|windows|codebuddy|codex|opencode|aider|amp|agents|claw|droid|trae|trae-cn|gemini|cursor|antigravity|hermes|kiro|pi|devin)")
592592
print(" uninstall remove graphify from all detected platforms in one shot")
593593
print(" --purge also delete graphify-out/ directory")
594+
print(" omp [install|path] install the native Oh My Pi guard, or print its package path")
594595
print(" path \"A\" \"B\" shortest path between two nodes in graph.json")
595596
print(" --graph <path> path to graph.json (default graphify-out/graph.json)")
596597
print(" explain \"X\" plain-language explanation of a node and its neighbors")

‎graphify/cli.py‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import time
1414
from graphify.paths import GRAPHIFY_OUT as _GRAPHIFY_OUT
1515
from pathlib import Path, PurePosixPath, PureWindowsPath
16+
from urllib.parse import unquote, urlsplit
1617

1718

1819
_SEARCH_NUDGE = json.dumps({
@@ -894,10 +895,26 @@ def _run_hook_guard(kind: str, strict: bool = False) -> None:
894895
if explicit:
895896
in_project = False
896897
for v in explicit:
898+
v = _normalize_hook_path(v)
899+
# A bare www.host/... value is remote only when nothing
900+
# identically named exists locally, mirroring OMP's own
901+
# "an existing local path wins over URL" precedence
902+
# (resolveToolSearchScope). Decided here, not inside
903+
# _is_cwd_relative, which has no cwd/root of its own.
904+
if _is_external_www_target(v, root):
905+
continue
897906
p = Path(v)
898907
if _is_cwd_relative(v):
899908
in_project = True # relative -> anchored at cwd == in project
900909
break
910+
# _is_cwd_relative already rejects a *whole-value*
911+
# scheme://... prefix above (a rootless URL never reaches
912+
# here). This catches the same URL after some upstream
913+
# host has already glued it onto an absolute prefix
914+
# (`<root>/local:/x`), which looks exactly like a real
915+
# file to the containment check below.
916+
if _has_embedded_url_scheme_segment(v):
917+
continue
901918
try:
902919
p.resolve().relative_to(root)
903920
in_project = True
@@ -942,6 +959,73 @@ def _run_hook_guard(kind: str, strict: bool = False) -> None:
942959
pass
943960

944961

962+
_URL_SCHEME_PREFIX_RE = re.compile(r"^[a-zA-Z][a-zA-Z0-9+.-]*://")
963+
_URL_SCHEME_SEGMENT_RE = re.compile(r"^[a-zA-Z][a-zA-Z0-9+.-]*:$")
964+
_FILE_URL_RE = re.compile(r"^file://", re.IGNORECASE)
965+
_WWW_HOST_RE = re.compile(r"^www\.", re.IGNORECASE)
966+
967+
968+
def _normalize_hook_path(value: str) -> str:
969+
r"""Trim padding whitespace and a matching pair of outer double quotes,
970+
then strip a *local* leading ``file://`` scheme down to the local path
971+
it names.
972+
973+
Mirrors what OMP's own path pipeline already does to a raw tool argument
974+
before a hook would ever see it: ``normalizePathLikeInput`` (trim +
975+
de-quote) and ``file://`` -> ``url.fileURLToPath`` (its own
976+
``strictExternalUrlRe`` deliberately omits ``file``, routing it through
977+
the ordinary local-file path instead of the external-URL one). The guard
978+
must classify identical text identically whether or not a given host
979+
bothered to normalize it first -- a classifier that only agrees with its
980+
own host after trimming/de-quoting is exactly the kind of gap a prior
981+
review flagged.
982+
983+
Per RFC 8089, a ``file://`` URL is local only when its authority is
984+
empty (``file:///path``) or ``localhost``; Node's own
985+
``url.fileURLToPath`` enforces exactly this, throwing
986+
``ERR_INVALID_FILE_URL_HOST`` for any other host. Any other authority
987+
names a *remote* host, not a local path -- reducing it here would
988+
discard the host and let ``file://evil.com/<in-project path>`` alias a
989+
real local file, so it is left untouched for ``_is_foreign_url_scheme``
990+
to classify instead.
991+
"""
992+
value = value.strip()
993+
if len(value) > 1 and value[0] == value[-1] == '"':
994+
value = value[1:-1]
995+
if _FILE_URL_RE.match(value):
996+
split = urlsplit(value)
997+
if split.hostname and split.hostname.lower() != "localhost":
998+
return value
999+
path = unquote(split.path) or "/"
1000+
# file:///C:/proj/a.py -> C:/proj/a.py: drop the URL's extra root
1001+
# slash in front of a Windows drive letter.
1002+
if os.name == "nt" and re.match(r"^/[A-Za-z]:", path):
1003+
path = path[1:]
1004+
return path
1005+
return value
1006+
1007+
1008+
def _is_foreign_url_scheme(value: str) -> bool:
1009+
"""Whether *value* is itself a ``scheme://...`` value -- everything
1010+
except a *local* ``file://`` (empty or ``localhost`` authority, RFC
1011+
8089), which ``_normalize_hook_path`` has already reduced to a plain
1012+
local path by the time this runs, so it never matches here. A
1013+
``file://`` with any other authority is NOT reduced by
1014+
``_normalize_hook_path`` and so matches here like any other foreign
1015+
scheme, naming a remote host rather than a local file.
1016+
1017+
Deliberately does not enumerate OMP's (or any other harness's) internal
1018+
scheme allow-list (``local://``, ``artifact://``, ...): this guard is
1019+
embedded by multiple hosts (#522), and hard-coding one of them would
1020+
silently stop matching the day that host adds a scheme. Any scheme this
1021+
function doesn't specifically know to be local -- only a local
1022+
``file://`` is -- is treated as not-a-local-source-file: a missed nudge
1023+
on a remote URL is a far safer wrong answer than a wrong nudge on an
1024+
unrelated file.
1025+
"""
1026+
return bool(_URL_SCHEME_PREFIX_RE.match(value))
1027+
1028+
9451029
def _is_cwd_relative(value: str) -> bool:
9461030
r"""Whether *value* is anchored at the current working directory.
9471031
@@ -964,11 +1048,63 @@ def _is_cwd_relative(value: str) -> bool:
9641048
so ``paths.is_absolute_any_platform`` (for stored, portable paths) is
9651049
deliberately not used. On POSIX ``root`` is set exactly when the path is
9661050
absolute and ``drive`` is always empty, so this is unchanged there.
1051+
1052+
A URL is rootless and driveless by that exact same test -- ``https://x``
1053+
and ``myscheme://x`` have no ``root`` and no ``drive`` in either flavour
1054+
-- so without checking the scheme first, this rule alone short-circuits
1055+
the caller straight to "in project" for a value that names no filesystem
1056+
path at all (the bug this guards against, #522 follow-up).
1057+
``_normalize_hook_path`` and ``_is_foreign_url_scheme`` reject that
1058+
shape, and strip a ``file://`` wrapper down to its local path, before
1059+
the root/drive test below ever runs.
9671060
"""
1061+
value = _normalize_hook_path(value)
1062+
if _is_foreign_url_scheme(value):
1063+
return False
9681064
pure = PureWindowsPath(value) if os.name == "nt" else PurePosixPath(value)
9691065
return not pure.root and not pure.drive
9701066

9711067

1068+
def _has_embedded_url_scheme_segment(value: str) -> bool:
1069+
"""Whether an already-absolute *value* still carries a collapsed
1070+
``scheme://`` marker as one of its OWN path segments, e.g.
1071+
``<root>/local:/x`` -- what a naive host-side path-join leaves behind
1072+
when it glues a rootless ``scheme://...`` value onto a root/cwd prefix
1073+
instead of routing it through a URL handler, defeating the whole-value
1074+
check in ``_is_cwd_relative`` above.
1075+
1076+
Only meaningful once a value is already known to have a root or drive
1077+
(``_is_cwd_relative`` returned ``False``): the leading component IS that
1078+
root/drive marker, so it is skipped here on purpose -- a genuinely
1079+
relative, colon-bearing POSIX filename like ``C:/proj/a.py`` never
1080+
reaches this function at all (it takes the cwd-relative shortcut above
1081+
and stays in-project, matching POSIX semantics where a colon has no
1082+
special meaning).
1083+
"""
1084+
pure = PureWindowsPath(value) if os.name == "nt" else PurePosixPath(value)
1085+
parts = pure.parts[1:] if (pure.root or pure.drive) else pure.parts
1086+
return any(_URL_SCHEME_SEGMENT_RE.match(part) for part in parts)
1087+
1088+
1089+
def _is_external_www_target(value: str, root: "Path") -> bool:
1090+
"""Whether *value* is a bare ``www.host/...`` external-read shape with no
1091+
identically named local file to override it.
1092+
1093+
Mirrors OMP's own ``isReadableUrlPath``/``resolveToolSearchScope``
1094+
precedence: a ``www.`` value carries no ``://`` at all, so no scheme rule
1095+
catches it, and it is remote only when nothing local shares its exact
1096+
name ("an existing local path wins over URL"). A project that genuinely
1097+
has a top-level path named e.g. ``www.example.com`` stays classified as
1098+
local; only the common case -- no such path -- is treated as external.
1099+
"""
1100+
if not _WWW_HOST_RE.match(value):
1101+
return False
1102+
try:
1103+
return not (root / value).exists()
1104+
except OSError:
1105+
return False
1106+
1107+
9721108
def _target_is_indexed(file_path: str, root: "Path") -> bool:
9731109
"""Guard the strict deny: only block a read of a file the graph actually indexes.
9741110
Reads manifest.json (cheap, capped); on any doubt (missing/corrupt/oversized

0 commit comments

Comments
 (0)