Repository navigation
fix(omp): wait for the integration wherever omp loads it, and not under --no-extensions (#1486) - #2159
Conversation
…er --no-extensions (#1486) The startup wait (#1475, #1825) only looked for herdr's omp integration in ~/.omp/agent/extensions (or a --profile's). omp 18.8.7 also loads it from the project's <cwd>/.omp/extensions, from a file -e/--extension/--hook/--trusted-extension names, and from the agent directory PI_CODING_AGENT_DIR, PI_PROFILE or PI_CONFIG_DIR names, so such an omp was sent to while it started. With --no-extensions it loads none of the discovered ones, yet the send waited the full 30 s for a report that never came. - server/omp.ts: ompLaunch reads the extension files and --no-extensions; ompConfigDir resolves the agent directory the way ompStoreLayout does (shared ompAgentDirectory); ompLoadsIntegration answers whether the integration loads. - server/core/composer.ts: the wait uses it with herdr's process cwd, and a wait that runs out without a report says so in the server log. - Tests: each new place waits for the report, --no-extensions sends at once, the timed-out wait logs once, and a post-wait lookup that finds no omp still sends (the gap the issue listed). Refs #1486
Review follow-up (omo reviewer and Codex, MEDIUM): omp 18.8.7 turns off extension discovery under --trusted-extension, so an installed integration is not loaded and the send waited 30 s for a report that never came. The PI_CODING_AGENT_DIR startup case runs on Linux only, where the server can read another process's environment, and the timed-out wait's log line comes after the replacement check, so it never says "sending anyway" for a send it refuses. Refs #1486
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 10 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Merging 2272ef4: omo-native-code-reviewer and Codex both pass this head. The new location cases fail on main and pass here, and the argv model matches omp 18.8.7's own parser. The required checks are green, and CodeRabbit did not review this head. Codex found two MEDIUMs, which go to a follow-up issue with the LOW notes:
|
What it does for the user
A chat message sent to an omp that is still starting now waits for herdr's omp integration to report the session wherever omp actually loads that integration from, not only from
~/.omp/agent/extensions. Before, an omp that loaded it from the project, from a file on its command line or from another agent directory got the message while it was still starting, so the message could stay typed in the half-started input box. An omp started with--no-extensionsor--trusted-extensionloads no installed integration, so it now gets the message at once instead of after the full 30 s wait for a report that never comes. A wait that runs out without a report now leaves one line in the server log.Why
The startup wait (#1475, #1825) only sends early when the integration is not loaded, and it decided that by looking in the default (or
--profile) agent directory. I read omp 18.8.7's bundled code to see where it really loads extensions from:<cwd>/.omp/extensions(skipped when cwd is the home directory) and in<agentDir>/extensions.--profile,OMP_PROFILE,PI_PROFILE,PI_CONFIG_DIRandPI_CODING_AGENT_DIR. These are the same rulesompStoreLayoutalready encodes, so that resolution is now shared (ompAgentDirectory).-e/--extension/--hookfiles load too, even under--no-extensions, which turns discovery off.--trusted-extensionloads only its own files and also turns discovery off.Changes
server/omp.ts:ompLaunchnow reads the extension files and--no-extensions.ompConfigDirresolves the agent directory from the environment. The newompLoadsIntegrationanswers whether the integration is loaded.server/core/composer.ts: the wait usesompLoadsIntegrationwith the cwd herdr reports for the process. A wait that times out logs[omp] <pane>: … reported no session within 30 s …, after the replacement check, so the line never appears for a send that is refused.Refs #1486. That issue also listed these:
How it was verified
bun test server/core/omp-submit.test.ts server/omp.test.ts: 62 pass. New startup cases: the integration in the project's.omp/extensions, a file named by-e, a file named by--hookunder--no-extensions, andPI_CODING_AGENT_DIR(Linux only, where the server can read another process's environment). Each waits for the report and then sends once.--no-extensionsand--trusted-extension /opt/other.tssend at once. A timed-out wait logs exactly one line.origin/main'scomposer.tsandomp.tsrestored, 7 of the new cases fail. With the--trusted-extensionchange taken out, 3 cases fail.bun run check fast: all green on the head (workflow syntax, generated files, typecheck, build, unit tests).--trusted-extensiondid not disable discovery; both reviewers), a MEDIUM (thePI_CODING_AGENT_DIRcase would fail on macOS, whereprocessEnvironis null) and a LOW (the timeout line logged before the replacement check). All three are fixed in 2272ef4, and Codex's re-review of that commit is PASS.settings.jsonare not seen.-epath with unquoted spaces is split by omp's argv and not recognized.No workflow change.