From c0423bf5609caae034740e213844a9a1f65c0890 Mon Sep 17 00:00:00 2001 From: timkjr Date: Tue, 18 Aug 2026 23:57:26 -0500 Subject: [PATCH] fix(omp): complete omp wiring in UI files, skill docs, and tests after rebase --- skills/codeman/reference/endpoints.md | 24 +++++----- skills/codeman/reference/messaging.md | 4 +- skills/codeman/reference/recipes.md | 2 +- skills/codeman/reference/verbs.md | 6 +-- src/web/public/mobile.css | 6 ++- src/web/public/session-ui.js | 68 +++++++++++++++++++++------ test/render-index-html.test.ts | 10 ++++ 7 files changed, 87 insertions(+), 33 deletions(-) diff --git a/skills/codeman/reference/endpoints.md b/skills/codeman/reference/endpoints.md index 50548efa..d02d42a2 100644 --- a/skills/codeman/reference/endpoints.md +++ b/skills/codeman/reference/endpoints.md @@ -237,7 +237,7 @@ minutes, never retry the credential. flushed slightly *after* the `stop` hook fires, so a read taken the instant the wait returns is too early (verified live: empty on the first call, full prose seconds later). It is also `""` before the worker's first completed turn, and permanently `""` for -`shell`, `opencode`, `gemini`, `antigravity`, `pi` and `grok`, which write no transcript at +`shell`, `opencode`, `gemini`, `antigravity`, `pi`, `grok` and `omp`, which write no transcript at all. `deepseek` is NOT one of those — it is read from `$DSH_HOME/sessions/**` and lags for the same reason claude does (the harness finalizes the assistant message just after it reports `idle`), so poll it the same way. @@ -339,20 +339,20 @@ ESC=$(printf '\033') `POST /api/v1/quick-start` body (all optional): `{"caseName":"worker-1","mode":"claude","sessionName":"w9-worker","effort":"high"}` -, `mode` ∈ `claude|shell|opencode|codex|gemini|antigravity|pi|grok|deepseek`; response is +, `mode` ∈ `claude|shell|opencode|codex|gemini|antigravity|pi|grok|deepseek|omp`; response is `.data.{sessionId, caseName, casePath}`. Creates the case directory (a real directory on the user's disk) if missing, do not retry it in a loop, and remember the name. ⚠️ A `mode` whose CLI is **not installed on the server** fails the spawn with `OPERATION_FAILED`; it never falls back to claude. Probe first whenever you did not pick the mode yourself: `GET /api/v1/claude/status`, `GET /api/v1/opencode/status`, -`GET /api/v1/codex/status`, `GET /api/v1/gemini/status`, `GET /api/v1/antigravity/status`, `GET /api/v1/grok/status`, `GET /api/v1/deepseek/status` -and `GET /api/v1/pi/status` each return `.data.{available, path}` (no session needed). -Pi's and grok's also carry `.data.version`, because `pi` is a short generic name and -`grok` is a name with npm squatters, so an unrelated binary on `$PATH` can shadow either: -the resolver rejects one whose `--version` is not version-shaped, so `available:false` -there can mean "a different `pi`/`grok` is in front" rather than "nothing is installed". -`shell` has no CLI to probe. +`GET /api/v1/codex/status`, `GET /api/v1/gemini/status`, `GET /api/v1/antigravity/status`, `GET /api/v1/grok/status`, `GET /api/v1/deepseek/status`, +`GET /api/v1/pi/status` and `GET /api/v1/omp/status` each return `.data.{available, path}` (no session needed). +Pi's, grok's and OMP's also carry `.data.version`, because `pi` is a short generic name, +`grok` is a name with npm squatters, and `omp` is a similarly short name, so an unrelated +binary on `$PATH` can shadow any of them: the resolver rejects one whose `--version` is +not version-shaped, so `available:false` there can mean "a different `pi`/`grok`/`omp` is +in front" rather than "nothing is installed". `shell` has no CLI to probe. ⚠️ **Branch on `.success` before reading `.data.sessionId`.** On any failure the field is absent, `jq -r` prints the literal string `null`, and every later call then targets @@ -466,9 +466,9 @@ Quirks that will bite you: session answers with an empty timeline rather than a 404. - ⚠️ **`active-tools` proves presence, never absence.** It is fed by the BashToolParser, which reads Claude's rendered `● Bash(…)` lines, and `_processExpensiveParsers` - returns early for every external CLI mode (`session.ts:2261`), so it is permanently - `[]` on `opencode`/`codex`/`gemini`/`antigravity`/`pi`/`grok`/`deepseek`. ⚠️ **`shell` is NOT one of those** - (`isExternalCliMode`, `session.ts:174-183`, lists only those six), so the parser does + returns early for every external CLI mode (`session.ts:~2225`), so it is permanently + `[]` on `opencode`/`codex`/`gemini`/`antigravity`/`pi`/`grok`/`deepseek`/`omp`. ⚠️ **`shell` is NOT one of those** + (`isExternalCliMode`, `session.ts:176-187`, lists only those seven), so the parser does run on a shell worker, and `TEXT_COMMAND_PATTERN` (`bash-tool-parser.ts:89`) matches bare `tail|cat|head|less|grep|watch|multitail ` lines with no `● Bash(` wrapper: a shell worker running `cat build.log` really does populate this. In practice it stays diff --git a/skills/codeman/reference/messaging.md b/skills/codeman/reference/messaging.md index 414ef3d2..24101c88 100644 --- a/skills/codeman/reference/messaging.md +++ b/skills/codeman/reference/messaging.md @@ -56,7 +56,7 @@ own head: the worker enforcing the cap is the one who has to be told about it. | synchronize on end of turn | HTTP `wait until=stop` (fires for message-initiated turns too, verified live) | | liveness / death check | HTTP `wait?until=exit` | | interrupt a running turn (break-glass) | HTTP input, a bare `\x1b` with no `\r` | -| non-claude modes (`shell`/`opencode`/`codex`/`gemini`/`antigravity`/`pi`/`grok`/`deepseek`) | HTTP only (no other CLI has messaging) | +| non-claude modes (`shell`/`opencode`/`codex`/`gemini`/`antigravity`/`pi`/`grok`/`deepseek`/`omp`) | HTTP only (no other CLI has messaging) | | delete | HTTP, via SKILL.md's `delete_session` guard | ## Availability: probe, never assume @@ -347,7 +347,7 @@ Without a break-glass, a pair with a bad brief is a token bonfire with no off sw ### Mixed fleets: the pairing matrix -Non-claude workers (`shell`, `opencode`, `codex`, `gemini`, `antigravity`, `pi`, `grok`, `deepseek`) cannot be peers +Non-claude workers (`shell`, `opencode`, `codex`, `gemini`, `antigravity`, `pi`, `grok`, `deepseek`, `omp`) cannot be peers at all; no other CLI has this feature. Their tasks route over HTTP, and you never mention messaging in their briefs. The claude half of the fleet can use messaging among itself, subject to the namespace rule: **messaging works between two sessions that share one diff --git a/skills/codeman/reference/recipes.md b/skills/codeman/reference/recipes.md index d5e7f4df..58971ead 100644 --- a/skills/codeman/reference/recipes.md +++ b/skills/codeman/reference/recipes.md @@ -188,7 +188,7 @@ for _ in $(seq 1 10); do done printf '%s\n' "$TXT" # (.data is {text,timestamp}; text is also "" before the first completed turn and -# always "" for shell/opencode/gemini/antigravity/pi/grok, which have no transcript, use +# always "" for shell/opencode/gemini/antigravity/pi/grok/omp, which have no transcript, use # the terminal tail there, and here only to diagnose an unsubmitted prompt.) # 6. clean up: exact id, own list only, through the fail-closed preamble helper diff --git a/skills/codeman/reference/verbs.md b/skills/codeman/reference/verbs.md index f3219cd8..fa09d565 100644 --- a/skills/codeman/reference/verbs.md +++ b/skills/codeman/reference/verbs.md @@ -357,7 +357,7 @@ recovered by submitting it with `{"input":"\r"}`. only when the workspace actually has them, see [§5.1](#51-where-to-spawn)) **and for `deepseek`** — the one external CLI that reports its own lifecycle, so its `stop` is a real end-of-turn signal rather than a guess. On -`shell`/`opencode`/`codex`/`gemini`/`antigravity`/`pi`/`grok`, requesting them explicitly is a +`shell`/`opencode`/`codex`/`gemini`/`antigravity`/`pi`/`grok`/`omp`, requesting them explicitly is a 400, and lifecycle transitions there are coarse (a short shell command may emit **no** `idle` transition at all, verified live), so synchronize those with markers. @@ -399,7 +399,7 @@ from the transcript file, which is flushed slightly *after* the `stop` hook fire single read taken the instant send-and-wait returns comes back `""` even though the turn finished (verified live: empty on the first call, full text seconds later). `text` is also `""` before the worker's first completed turn, and always `""` for modes with -no transcript (`shell`, `opencode`, `gemini`, `antigravity`, `pi`, `grok`; the first four +no transcript (`shell`, `opencode`, `gemini`, `antigravity`, `pi`, `grok`, `omp`; the first four verified live, pi from the same source path), which is why the loop above is bounded rather than open-ended. A dsh worker lags too, for its own reason: the harness finalizes the assistant message just after it reports `idle`. Fall back to the terminal buffer @@ -485,7 +485,7 @@ turn), and both better than diffing terminal samples: ``` ⚠️ `active-tools` is parsed out of Claude's own output format, so it is **empty for -`opencode`/`codex`/`gemini`/`antigravity`/`pi`/`grok`/`deepseek`** (those parsers are skipped wholesale) and +`opencode`/`codex`/`gemini`/`antigravity`/`pi`/`grok`/`deepseek`/`omp`** (those parsers are skipped wholesale) and in practice empty for `shell`. Source-verified, not measured live. Only if neither helps: sample `terminal?tail=` twice a few seconds apart. A changing diff --git a/src/web/public/mobile.css b/src/web/public/mobile.css index f516ebb2..c9b0838d 100644 --- a/src/web/public/mobile.css +++ b/src/web/public/mobile.css @@ -3097,9 +3097,13 @@ html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="cat html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="catppuccin-latte"], [data-skin="rose-pine-dawn"]) :is(.btn-toolbar.btn-run.mode-pi, .btn-toolbar.btn-run-gear.mode-pi) { background: linear-gradient(135deg, #be185d, #db2777); border-color: #9d174d; + color: #ffffff; +} + html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="catppuccin-latte"], [data-skin="rose-pine-dawn"]) :is(.btn-toolbar.btn-run.mode-omp, .btn-toolbar.btn-run-gear.mode-omp) { background: linear-gradient(135deg, #4f46e5, #6366f1); - border-color: #4338ca; color: #ffffff; + border-color: #4338ca; + color: #ffffff; } html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="catppuccin-latte"], [data-skin="rose-pine-dawn"]) :is(.btn-toolbar.btn-run.mode-grok, .btn-toolbar.btn-run-gear.mode-grok) { diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 1372d70a..b78ce074 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -400,6 +400,9 @@ Object.assign(CodemanApp.prototype, { if (mode === 'antigravity') { return await this.runAntigravity(); } + if (mode === 'omp') { + return await this.runOmp(); + } if (mode === 'pi') { return await this.runPi(); } @@ -409,9 +412,6 @@ Object.assign(CodemanApp.prototype, { if (mode === 'deepseek') { return await this.runDeepSeek(); } - if (mode === 'omp') { - return await this.runOmp(); - } if (mode === 'shell') { return await this.runShell(); } @@ -709,6 +709,9 @@ Object.assign(CodemanApp.prototype, { if (runBtn) { runBtn.className = `btn-toolbar btn-run mode-${mode}`; } + if (gearBtn) { + gearBtn.className = `btn-toolbar btn-run-gear mode-${mode}`; + } if (label) { label.textContent = mode === 'opencode' ? 'Run OC' : mode === 'codex' ? 'Run CX' : mode === 'gemini' ? 'Run GM' : mode === 'antigravity' ? 'Run AG' : mode === 'pi' ? 'Run PI' : mode === 'grok' ? 'Run GK' : mode === 'deepseek' ? 'Run DS' : mode === 'omp' ? 'Run OMP' : mode === 'shell' ? 'Run SH' : 'Run'; } @@ -1383,24 +1386,17 @@ Object.assign(CodemanApp.prototype, { const isRemote = _runLoc === 'remote' || _runLoc === 'docker'; const ownsLaunchTerminal = this._beginSessionLaunchStatus(`Starting Pi session in ${caseName}...`); - async runOmp() { - const caseName = document.getElementById('quickStartCase').value || 'testcase'; - // Remote/docker cases run omp on the OTHER side — skip the local status probe - // and the local-only config below (quick-start rejects them for remote cases). - const _runLoc = (this.cases || []).find(c => c.name === caseName)?.location; - const isRemote = _runLoc === 'remote' || _runLoc === 'docker'; - - const ownsLaunchTerminal = this._beginSessionLaunchStatus(`Starting OMP session in ${caseName}...`); this.terminal.focus(); + this.terminal.focus(); try { if (!isRemote) { const statusRes = await fetch('/api/pi/status'); - const statusRes = await fetch('/api/omp/status'); const status = (await statusRes.json()).data; + const status = (await statusRes.json()).data; if (!status.available) { this._reportSessionLaunchError( ownsLaunchTerminal, 'Pi CLI not found. Install with: npm install -g --ignore-scripts @earendil-works/pi-coding-agent' - 'OMP CLI not found. Install with: curl -fsSL https://omp.sh/install | sh' ); + ); return; } } @@ -1418,6 +1414,47 @@ Object.assign(CodemanApp.prototype, { }); const data = await res.json(); if (!data.success) throw new Error(data.error || 'Failed to start Pi'); + await this._ensureCreatedSessionVisible(data.data.sessionId, data.data.session); + + if (data.data.sessionId) { + await this.selectSession(data.data.sessionId); + } + + this.terminal.focus(); + } catch (err) { + this._reportSessionLaunchError(ownsLaunchTerminal, err.message); + } + }, + + async runOmp() { + const caseName = document.getElementById('quickStartCase').value || 'testcase'; + // Remote/docker cases run omp on the OTHER side — skip the local status probe + // and the local-only config below (quick-start rejects them for remote cases). + const _runLoc = (this.cases || []).find(c => c.name === caseName)?.location; + const isRemote = _runLoc === 'remote' || _runLoc === 'docker'; + + const ownsLaunchTerminal = this._beginSessionLaunchStatus(`Starting OMP session in ${caseName}...`); + this.terminal.focus(); + + try { + if (!isRemote) { + const statusRes = await fetch('/api/omp/status'); + const status = (await statusRes.json()).data; + if (!status.available) { + this._reportSessionLaunchError( + ownsLaunchTerminal, + 'OMP CLI not found. Install with: curl -fsSL https://omp.sh/install | sh' + ); + return; + } + } + + const envOverrides = this.buildEnvOverrides(this.getCaseSettings(caseName), this.loadAppSettingsFromStorage()); + const res = await fetch('/api/quick-start', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + caseName, mode: 'omp', sessionName: `w${this._nextCaseSessionStartNumber(caseName)}-${caseName}`, ...(isRemote ? {} : { @@ -1426,7 +1463,8 @@ Object.assign(CodemanApp.prototype, { }) }); const data = await res.json(); - if (!data.success) throw new Error(data.error || 'Failed to start OMP'); await this._ensureCreatedSessionVisible(data.data.sessionId, data.data.session); + if (!data.success) throw new Error(data.error || 'Failed to start OMP'); + await this._ensureCreatedSessionVisible(data.data.sessionId, data.data.session); if (data.data.sessionId) { await this.selectSession(data.data.sessionId); @@ -1644,6 +1682,7 @@ Object.assign(CodemanApp.prototype, { const isAltMode = session.mode === 'opencode' || session.mode === 'codex' || session.mode === 'gemini' || session.mode === 'antigravity' || session.mode === 'pi' || session.mode === 'grok' || session.mode === 'deepseek' || session.mode === 'omp'; this.switchOptionsTab(isAltMode ? 'summary' : 'respawn'); + // Update respawn status display and buttons const respawnStatus = document.getElementById('sessionRespawnStatus'); const enableBtn = document.getElementById('modalEnableRespawnBtn'); const stopBtn = document.getElementById('modalStopRespawnBtn'); @@ -1673,6 +1712,7 @@ Object.assign(CodemanApp.prototype, { const isExternalCli = session.mode === 'opencode' || session.mode === 'codex' || session.mode === 'gemini' || session.mode === 'antigravity' || session.mode === 'pi' || session.mode === 'grok' || session.mode === 'deepseek' || session.mode === 'omp'; const claudeOnlyEls = document.querySelectorAll('[data-claude-only]'); claudeOnlyEls.forEach(el => { el.style.display = isExternalCli ? 'none' : ''; }); + // Reset duration presets to default (unlimited) this.selectDurationPreset(''); diff --git a/test/render-index-html.test.ts b/test/render-index-html.test.ts index 21c5f8a3..1d103508 100644 --- a/test/render-index-html.test.ts +++ b/test/render-index-html.test.ts @@ -20,6 +20,7 @@ import { isAntigravityAvailable } from '../src/utils/antigravity-cli-resolver.js import { isPiAvailable } from '../src/utils/pi-cli-resolver.js'; import { isGrokAvailable } from '../src/utils/grok-cli-resolver.js'; import { isDeepSeekAvailable, isDeepSeekRunnable } from '../src/utils/deepseek-cli-resolver.js'; +import { isOmpAvailable } from '../src/utils/omp-cli-resolver.js'; import { isCloudflaredAvailable } from '../src/utils/cloudflared-resolver.js'; import { isGitAvailable } from '../src/git-clone.js'; @@ -66,6 +67,10 @@ vi.mock('../src/utils/deepseek-cli-resolver.js', () => ({ listDeepSeekProfiles: vi.fn(() => []), resolveDefaultDeepSeekProfile: vi.fn(() => null), })); +vi.mock('../src/utils/omp-cli-resolver.js', () => ({ + isOmpAvailable: vi.fn(() => false), + resolveOmpDir: vi.fn(() => null), +})); vi.mock('../src/utils/cloudflared-resolver.js', () => ({ isCloudflaredAvailable: vi.fn(() => false), resolveCloudflaredPath: vi.fn(() => null), @@ -156,6 +161,9 @@ describe('WebServer.renderIndexHtml', () => { vi.mocked(isAntigravityAvailable).mockReturnValue(false); vi.mocked(isPiAvailable).mockReturnValue(true); vi.mocked(isGrokAvailable).mockReturnValue(false); + vi.mocked(isDeepSeekAvailable).mockReturnValue(false); + vi.mocked(isDeepSeekRunnable).mockReturnValue(false); + vi.mocked(isOmpAvailable).mockReturnValue(true); vi.mocked(isCloudflaredAvailable).mockReturnValue(true); vi.mocked(isGitAvailable).mockReturnValue(true); const { server } = makeServer({}); @@ -173,6 +181,7 @@ describe('WebServer.renderIndexHtml', () => { grok: false, deepseek: false, deepseekBinary: false, + omp: true, cloudflared: true, git: true, }); @@ -191,6 +200,7 @@ describe('WebServer.renderIndexHtml', () => { isGrokAvailable, isDeepSeekAvailable, isDeepSeekRunnable, + isOmpAvailable, isCloudflaredAvailable, isGitAvailable, ]) {