Repository navigation
feat(agents): detect OMP / Oh My Pi as a first-class agent - #15
Conversation
Adds AgentTypeOMP and OMPAgent for github.com/can1357/oh-my-pi, a Pi fork installed under ~/.omp with its own omp binary. Detection uses the omp binary in the exec path and AGENT_ENV=omp/oh-my-pi. OMP inherits Pi's PI_CODING_AGENT marker from shared cli.ts ancestry, so PiAgent.DetectRuntime now yields when the launcher is omp, keeping the two runtime detectors mutually exclusive by construction (the registry is a map with no ordering guarantee to break a tie).
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdded first-class Oh My Pi support. The change defines the OMP agent, detects OMP runtimes, resolves configuration and context paths, registers the agent, and prevents Pi detection from matching OMP launchers. ChangesOMP agent support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Launcher
participant OMPAgent
participant PiAgent
Launcher->>OMPAgent: provide executable path and AGENT_ENV
OMPAgent-->>Launcher: detect OMP runtime
Launcher->>PiAgent: provide executable path and Pi markers
PiAgent-->>Launcher: reject omp and oh-my-pi launchers
``
</details>
<!-- walkthrough_end -->
<!-- finishing_touch_checkbox_start -->
<details>
<summary>✨ Finishing Touches</summary>
<details>
<summary>📝 Generate docstrings</summary>
- [ ] <!-- {"checkboxId": "7962f53c-55bc-4827-bfbf-6a18da830691"} --> Create stacked PR
- [ ] <!-- {"checkboxId": "3e1879ae-f29b-4d0d-8e06-d12b7ba33d98"} --> Commit on current branch
</details>
<details>
<summary>🧪 Generate unit tests (beta)</summary>
- [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Create PR with unit tests
- [ ] <!-- {"checkboxId": "6ba7b810-9dad-11d1-80b4-00c04fd430c8", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Commit unit tests in branch `feat/omp-detection`
</details>
</details>
<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->
---
<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>
<!-- tips_end -->
|
Greptile SummaryThis change adds OMP agent metadata, registration, configuration, session behavior, and runtime detection alongside Pi support. Runtime detection incorrectly treats a non-OMP executable as OMP when any parent directory contains Confidence Score: 4/5Not ready to merge until OMP runtime detection identifies the launcher by basename rather than matching arbitrary parent directories. A focused executable reproduction exercised both the OMP runtime check and registry selection. It showed the current code selecting OMP for an unrelated launcher, while a basename-only predicate selected the configured Pi agent for the identical input. Files Needing Attention: agents/omp.go needs the runtime predicate corrected and a regression test should cover a non-OMP executable below a directory containing
 T-Rex LogsWhat T-Rex did
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if strings.Contains(strings.ToLower(execPath), "oh-my-pi") { | ||
| return true, nil |
There was a problem hiding this comment.
DetectRuntime treats any executable whose full path contains oh-my-pi as OMP. A non-OMP launcher at /srv/oh-my-pi-workspace/bin/non-omp-executable therefore wins phase-one detection, preventing the valid Pi environment detector from running and selecting the requested agent. Match oh-my-pi against filepath.Base(execPath) rather than using a substring search over every directory component, and add this path-layout regression case.
Artifacts
- The program registers OMP and Pi, supplies the problematic full path plus a Pi override, and invokes runtime and public detector paths, proving the claimed scenario is executable.
- Running the authored reproducer against the repository code reports `omp_runtime=true` and `selected_type=omp`, showing phase one incorrectly preempts Pi.
Exact-basename comparison output
- Running the same reproducer after a temporary exact-basename-only predicate reports `omp_runtime=false` and `selected_type=pi`, confirming the narrow cause and expected outcome.
Restoration and focused test output
- The command confirms the original substring predicate was restored and `go test ./agents` completed successfully, showing no repository source was permanently modified.
Summary
Tools built on agentx can now recognize Oh My Pi (OMP) — a Pi fork that installs under
~/.ompand ships its ownompbinary — as a first-class agent instead of misreading it as upstream Pi. This is the agentx counterpart to the oxox-adapter-ompwork (sageox/ox#779): ox currently has to declare OMP as a local constant and bypass agentx's detection gate precisely because agentx didn't know OMP existed.Approach: add
AgentTypeOMP+OMPAgent(detects via theompbinary in the exec path andAGENT_ENV=omp/oh-my-pi), and make the OMP and Pi runtime detectors mutually exclusive so the map-ordered registry never has to break a tie between two forks that share thePI_CODING_AGENTmarker.The one subtlety worth your eyes
OMP inherits Pi's
PI_CODING_AGENTenv marker from the sharedcli.tsancestry, and the registry stores agents in amap— soList()order is nondeterministic and phase-1 detection has no tiebreak if both runtime detectors fire. Rather than rely on registration order (which guarantees nothing),PiAgent.DetectRuntimenow yields when the launching binary isomp, keeping the two detectors mutually exclusive by construction.Honest limitation, documented in code: a node-launched OMP process that only exports the inherited
PI_CODING_AGENT(noompin_) can still resolve to Pi in phase 1.AGENT_ENV=omp(which the ox integration always sets) orDetectByTypeis authoritative there.What changed
agent.goAgentTypeOMP = "omp"constant +SupportedAgentsentryagents/omp.goOMPAgent— fullAgentinterface,~/.ompconfig, tail-mode capabilities (no hooks)agents/pi.goDetectRuntimeyields to OMP when the launcher isomp/oh-my-piagents/register.go,setup/setup.goOMPAgentin both pathsagents/omp_test.goagents/pi_test.go,agent_test.goCHANGELOG.md[0.1.14]Test Plan
make check— fmt + lint (golangci-lint: 0 issues) + test (1164 tests, 2 skipped, 0 failures)TestOMPDetect— 10 cases incl. the Pi-marker rejections (PI_CODING_AGENT/PI_CODING_AGENT_DIR/AGENT_ENV=pi→ not OMP) and substring guard (compose→ no match)TestOMPPiMutualExclusion— asserts OMP and Pi never both fire, including the pathologicalomplauncher + inheritedPI_CODING_AGENTTestPiDetect— new cases prove Pi yields to theomplauncher; all prior Pi cases still pass (no regression)Design notes
LifecycleEventMapperand reportsHooks: false: the SageOx integration drives it via native.omp/AGENTS.mdguidance and tails its JSONL transcript, not an extension-bridge hook (unlike Pi).SessionIDprefersOMP_SESSION_ID, falls back to Pi'sPI_SESSION_ID(shared ancestry), else""(sessions are discovered by file).github.com/can1357/oh-my-pi) taken from the oxcmd/ox-adapter-pi/rules.goreference, not invented.Summary by CodeRabbit
New Features
Bug Fixes