Skip to content

feat: add OMP (Oh My Pi) as a new session backend - #353

Merged
Ark0N merged 19 commits into
Ark0N:masterfrom
timkjr:omp-mode
Aug 30, 2026
Merged

feat: add OMP (Oh My Pi) as a new session backend#353
Ark0N merged 19 commits into
Ark0N:masterfrom
timkjr:omp-mode

Conversation

@timkjr

Copy link
Copy Markdown

Summary

Adds omp (Oh My Pi, https://github.com/can1357/omp) as an eighth CLI backend,
alongside Claude Code, OpenCode, Codex, Gemini, Antigravity, Pi, Grok and DeepSeek
Harness. It follows the existing external-CLI pattern exactly (own PTY, own tmux
session, own tab identity, isExternalCliMode()), and gets full support across
every surface that pattern touches: schemas, spawn-command building, Docker cases,
remote-SSH cases, multi-user clamping, the UI run-mode picker, and skill docs.

Two things make this more than routine plumbing:

  • Exact-conversation pinning.--continue alone is ambiguous the moment any
    other omp conversation has touched the same working directory more recently.
    omp-session-resolver.ts resolves and pins the real session id once, so every
    later respawn uses --resume <id> instead of re-guessing.
  • Kill-survival.omp-transcript.ts scans ~/.omp/agent/sessions/**/*.jsonl
    directly, so a conversation's history is recoverable in Past Sessions even when
    both the Codeman session record and the underlying tmux pane are gone —
    verified live against a full OS reboot, not just a killed pane.

Notable bugs found and fixed along the way

  • A boot-recovery bug where claudeSessionId (the field also used to alias an
    OMP session's own id) failed to resolve on a plain-reattach path. Root cause
    was two compounding issues: an unconditional second assignment that clobbered
    the correctly-resolved value, and a directory-mangling helper that assumed omp
    mirrors Claude Code's naming convention (keeps the $HOME prefix) when omp
    actually strips $HOME first. This second bug meant continuation pinning was
    silently a no-op for every real case directory under $HOME — only /tmp-based
    manual testing had ever exercised the code path, and it happened to produce the
    right answer there by coincidence.
  • OMP was never actually installed in docker/agent.Dockerfile, and had no
    credential-isolation entry, despite the docs already claiming full Docker
    support. Both fixed and verified against a real --no-cache image build.
    OMP is also the one CLI in this family where sessions/ needs to be a
    shared mount rather than seeded, since Codeman reads it host-side for the
    resume-pinning and history-recovery mechanisms above.

Note for reviewers: while verifying the Docker changes above, I found the
agent image's DeepSeek (dsh) plugin-install step currently fails on a fresh
build against unmodified master ("pnpm not found on PATH") — confirmed via
git diff origin/master that the failing line is untouched by this branch.
Unrelated to OMP; filed separately as #352.

Testing

  • Full suite: 6317 passed, 0 failures (npm test)
  • lint, typecheck, check:frontend-syntax, check:public-assets,
    check:lockfile all clean
  • Live-tested: session create/respawn/resume, TUI-exit + remove-tab,
    TUI-exit + tmux kill-session + restart, and a full server/OS reboot —
    all recover correctly, including first-turn content
  • Docker: verified omp/18.0.8 installs and runs inside a freshly built
    agent image

Known limitations (documented in docs/omp-integration.md)

  • No idle/completion hook — falls back to output-stabilization like every
    other external CLI
  • Killing a pane mid-turn (before an in-TUI /exit) loses the conversation for
    real; omp hasn't flushed its session file yet at that point. This isn't
    something Codeman can compensate for from outside the process.
  • Directory-mangling behavior on a symlinked $HOME is unverified

timkjr added 3 commits August 26, 2026 20:05
The rebase hand-repair dropped the closing brace of .welcome-btn-pi:hover
and .btn-toolbar.btn-run.mode-pi:hover before the inserted OMP rules.
The browser CSS parser drops every rule after an unclosed block, so the
deployed UI rendered as unstyled text bars (only ~456 of ~2583 rules
applied). Verified clean via esbuild --minify (no css-syntax-error) and
rebuilt dist.
@timkjr

Copy link
Copy Markdown
Author

Pushed one more fix after opening this PR: 13a19f7. A fresh "Run OMP" click was silently resuming an old conversation instead of starting clean, in any working directory with prior OMP history — found live while testing, root-caused, fixed, and verified in production. Full details in the commit message; test suite still green (6319 passed).

@Ark0N

Copy link
Copy Markdown
Owner

Thanks for this, and for the write-up. The core is in good shape: it follows the external-CLI pattern faithfully, the resolver reuses createCliExecutableResolver instead of hand-rolling a fifth copy, the Docker cred-store entry reasons correctly about shared-vs-seeded, and omp-transcript.ts reading the session header instead of reverse-engineering the mangled directory name is exactly right (dsh taught us that mangling changes form). CI is green and the tests you added cover what you built.

I read the diff against omp upstream too (the repo is can1357/oh-my-pi, 28k stars, MIT, releasing daily, so a backend for it is worth having). A few things need to change first, and one of them is a correctness bug.

1. The newest-mtime pin breaks with two omp tabs in one case

findLatestOmpSessionId() is "newest file in the directory, full stop", and the comment argues callers only invoke it where that is unambiguous. Two of them are not:

  • _resolvedOmpRespawnConfig() runs from the eager respawnPaneOptions: this._buildRespawnPaneOptions() in startInteractive(), and your own guard comment notes _muxSession is already set on a boot-recovery reattach. So on every server restart it resolves and pins for each recovered omp session, including ones whose pane is alive and whose file is not the newest. Two omp sessions in the same case (w1-foo, w2-foo, the normal fan-out shape here) both alias onto the same id, and a later respawn of one resumes the other's conversation, with two processes appending to one session file.
  • _maybeCaptureOmpSessionId() on first idle is the same race in a narrower window.

This is the rule CLAUDE.md already carries for dsh: pair by the transcript's own header plus a launch window, never by newest mtime, because newest-mtime handed a fresh worker its predecessor's answer in the same case dir. You already parse that header in omp-transcript.ts ({"type":"session","id","cwd"}), so the pieces are there: snapshot the directory at spawn, accept only a file whose header cwd matches and that appeared after this session launched, and skip an id a live sibling already pinned.

Related: the pin is a side effect of building respawn options, which is what bit you in 13a19f7. Resolving where the respawn is actually issued would remove that whole class of bug rather than guarding it.

2. The env-allowlist and clamp claims do not match omp's docs

docs/omp-integration.md says omp "has no documented vendor-key namespace of its own (its provider credentials live in ~/.omp config files, not environment variables)", and CLAUDE.md gains "the multi-user clamp has nothing to gate". Per omp's own docs/environment-variables.md, neither holds:

  • omp reads roughly 40 provider keys from the environment (ANTHROPIC_API_KEY, OPENAI_API_KEY, XAI_API_KEY, HF_TOKEN, ...). That is pi's 34-key problem in the same shape. Keeping them out of the allowlist is correct, the stated reason is not.
  • omp's own knobs are mostly PI_*, not OMP_*, and PI_ is already allowlisted globally because pi mode needs it. So an omp session today already accepts PI_CONFIG_DIR ("config root dirname under home, default .omp"), PI_CODING_AGENT_DIR, PI_CODING_AGENT_SESSION_DIR, OMP_PROFILE/PI_PROFILE, PI_SUBPROCESS_CMD ("overrides subagent spawn command") and PI_SHELL_PREFIX. The first three also move the tree omp-session-resolver.ts and omp-transcript.ts hardcode, so pinning and history quietly stop working, which makes resolveOmpHome()'s "no known env override exists (unlike DSH_HOME); revisit if omp adds one" wrong as of today.
  • The OMP_ prefix this PR adds brings in OMP_AUTH_BROKER_URL + OMP_AUTH_BROKER_TOKEN, which is where omp resolves credentials from. That is the shape DEEPSEEK_BASE_URL gets dropped for in clampEnvOverridesForOwner().

None of this matters in single-user mode. It matters for a non-granted owner in multi-user mode, which is why pi and dsh both have explicit clamp branches. Please make the decision explicit instead of "nothing to gate": either add the config-root and broker keys to the clamp for omp, or write down why they are acceptable. Worth stating in the doc as well that omp's documented default tools.approvalMode is yolo, so an omp pane auto-approves exec with no flag from us.

3. Three changes in one PR

Besides the omp backend there are two riders:

  • resumeHistorySession() now creates the session in the row's own mode and DELETEs the old row. That changes behavior for opencode, pi, grok, deepseek, codex, gemini and antigravity users, not just omp. Mode fidelity is a real improvement (resuming a codex row as claude was silly). But codex, gemini and antigravity are not in the modeConfigKey map, so for them Resume now starts a fresh session with no continuation and retires the row it came from, which is a strange pairing. Either give those three their continuation flag, or only retire the row when the new session actually continues something.
  • DELETE /api/sessions/:id gained the persisted-only fallback that the above needs, and reimplements the ownership check inline rather than going through findSessionOrFail. I would rather extend the helper, since that function is where ownership enforcement is supposed to live. The persisted branch also does not broadcast session_deleted, so other tabs keep the row until their next fetch.
  • The per-device "Hide CLI buttons" setting is a feature of its own. Its mode list is ['claude','opencode','codex','gemini','antigravity','pi','omp'] in both the load and the save path, so grok and deepseek cannot be hidden.

I would rather take these as two or three PRs. If they stay together, the changeset has to describe all of it: right now it covers only the backend and lists the siblings as "Claude Code, OpenCode, Codex, Gemini, and Antigravity" with Pi, Grok and DeepSeek missing. That text lands verbatim in the release notes.

4. Smaller, all concrete

  • https://github.com/can1357/omp 404s. It is can1357/oh-my-pi, in the PR body and in docs/omp-integration.md.
  • CLAUDE.md now links #external-cli-modes-...-deepseek-omp, but docs/architecture-invariants.md still carries the -deepseek heading and is not touched by this PR, so the link is dead and omp has no entry in the deep-detail doc every other CLI has.
  • The doc says the installer places the binary in ~/.omp/bin, but your own Dockerfile comment says that guess was wrong, and upstream's installer is INSTALL_DIR="${PI_INSTALL_DIR:-$HOME/.local/bin}". The resolver's search order should probably lead with ~/.local/bin.
  • omp is the ninth CLI backend and tenth SessionMode, not the eighth (docs and changeset).
  • The case 'omp' added to appendResumeFlag() looks unreachable: it keys off the top-level resumeSessionId, which is validated against ~/.claude/projects for every mode, while the omp pin lands in ompConfig.resumeSessionId. Docker panes are built from defaultDockerCommandForMode and never see ompConfig, so the Docker section overstates it: host-side history recovery works there, --resume pinning does not reach the in-container omp. Worth a live check.
  • scanOmpSessionsHistory() is fully synchronous (readdirSync plus readFileSync of whole files, up to 2MB each, up to 2000 of them) inside gatherUnifiedInputs(), which the home screen hits on every visit. The claude scanner beside it is async with 16KB/128KB head windows for exactly that reason.
  • Stray comment fragment landed in the quick-start docker branch: // configured project. Skipped for external CLIs (they use their own systems). right after if (docker && docker.hooksEnabled && mode === 'claude') {.
  • Two jammed CSS lines survive from the block-closing commit: color: #eef2ff; transform: translateY(-1px); and color: #eef2ff;}.
  • The doc's version example says omp/17.4.0 while you tested 18.0.8.

Things I would keep as they are: the version probe demanding omp/<semver> (right instinct for a three-letter binary name), the $HOME-stripping mangling comment with the "silently returns null" warning, the shared-vs-seeded reasoning in CRED_STORES, and the honest Known Gaps section including the mid-turn kill.

Also: thanks for filing the dsh plugin-install breakage separately as #352 instead of folding it in here.

timkjrand others added 11 commits August 28, 2026 11:32
…esolver
- omp-cli-resolver.ts already uses createCliExecutableResolver; add dedicated
test/omp-cli-resolver.test.ts mirroring pi's (version-probe accept/reject,
negative-cache backoff, VITEST hermeticity gate)
- dependency-registry omp entry now requires OMP_VERSION_REGEX match like pi,
so codeman doctor and the run-mode resolver agree on what counts as installed
- system-routes /api/omp/status surfaces version
Two prose lists in skills/codeman/ named some but not all external CLI
modes after the omp-mode rebase, which is exactly the drift
test/agent-skill-mode-lists.test.ts exists to catch: SKILL.md's
no-hook-signals list was missing omp, and endpoints.md's version-probe
sentence named pi/grok/omp as a bare 3-mode run with no matching class.
resumeHistorySession() never sent mode when recreating a session from a
history/session-manager row, so the server default silently opened a
plain Claude session for every non-claude row -- reproduced live: OMP
rows spawned Claude sessions on click. Thread the row's mode through
every call site (welcome list, session manager, mobile overview) and
only send the Claude-specific resumeSessionId for claude rows.
Codeman has no live PTY-reattach outside server boot, and it's moot for
OMP anyway (exiting it kills the pane's only process), so route the
non-claude relaunch through each CLI's own continue-most-recent flag
instead of a context-free fresh start. OMP never got one: buildOmpCommand
only implemented --model/--resume despite omp --help documenting
-c/--continue. Added continueSession to OmpConfig end-to-end (type,
schema, builder) mirroring the existing opencode/pi/grok/deepseek
fields, and wired resumeHistorySession to use it.
Verified live: told a real omp session a secret, exited it, closed the
tab without killing tmux, relaunched with --continue in the same
directory, and had it recall the secret.
…here
respawnPane() -- the path used when a session's pane died (crash, idle
respawn, or the user's own /exit) but the Codeman session object is
still tracked -- never had ompConfig wired through at all, in either
its options destructure or its inner buildSpawnCommand() call. This is
a gap in the original OMP patch, distinct from the resumeHistorySession
fix (which only covers a session that has been fully closed and shows
up as a history row): reselecting a tab whose CLI process just exited
goes through this path instead, and always launched a bare, contextless
`omp` no matter what.
Beyond the wiring, respawning a dead pane is semantically different
from creating a brand-new session: the conversation is still "this
session" to the user, so _buildRespawnPaneOptions() now defaults
ompConfig to continueSession:true unless the session already carries
an explicit resumeSessionId (which still wins in buildOmpCommand).
Verified live: told a session a secret, exited OMP so the pane died
(session and tmux both left alone), forced the exact dead-pane-respawn
path, and the new process replied with the secret -- confirming
`omp --continue` fired instead of a blank omp.
…ed-only sessions
Every non-claude "Resume" click creates a brand-new Codeman session
(there is no id to reattach to), but the old row was never cleaned up
-- click resume on the same conversation a few times and the session
list fills up with duplicate rows sharing one name. resumeHistorySession
now retires the row it resumed from after the new one starts.
That retirement needs DELETE to actually work on a row that was never
live in the first place (the normal case for anything showing up in
"Resume Conversation"): findSessionOrFail only checks the in-memory
live-session map, so DELETE 404s on a persisted-only entry today. Give
the route a fallback: when the id isn't live, look it up in persisted
state instead and demote/remove it there (respecting the existing
pinned-session protection). Verified live against a real persisted-only
row via the API, and added route-test coverage for both the success
and still-truly-unknown-id cases (which needed a demoteOrRemoveSession
mock the route harness didn't have).
Also includes an unrelated pre-existing prettier drift fix picked up
by npm run format (omp-cli-resolver.ts, antigravity/opencode import
wrapping in session-routes.ts).
Claude conversations survive "Kill Tmux & Claude" because Codeman reads
them back independently from ~/.claude/projects, not from its own
session bookkeeping. omp conversations had no equivalent: kill the
Codeman session and the conversation vanished from Past Sessions
entirely, even though omp itself never forgot it on disk.
Adds omp-transcript.ts, a scanner over omp's own
~/.omp/agent/sessions/<mangled-cwd>/<uuid>.jsonl files (the same shape
as Claude Code's own transcript scanner, but simpler -- these files are
small enough to read whole instead of doing head/tail windows). Each
file's own "session" header line carries the real cwd and session id
directly, so unlike Claude's mangled-directory-name decoding this
never has to guess. Wired into gatherUnifiedInputs() as a second
history source alongside the Claude scan, and HistoryInput/
mergeUnifiedSessions() now carry an optional `mode` so a non-claude
history-only row still gets a real mode badge.
Also fixes the ambiguity behind the "continue picks the wrong
conversation" report from this session's testing: omp mints its OWN
session uuid, unrelated to Codeman's, so a live/persisted row and its
own history-scan row would otherwise show up as two separate entries
for the same conversation the moment the id gets resolved. Reuses the
existing claudeSessionId alias field (mergeUnifiedSessions' fold-into-
owner mechanism) to point at the resolved omp id, threading it through
every place `_claudeSessionId` gets (re)computed -- the constructor,
_resolvedOmpRespawnConfig, and a new _maybeCaptureOmpSessionId() that
opportunistically resolves it the first time a brand-new omp session
(one that has never gone through a respawn) goes idle.
Also closes a THIRD instance of the "ompConfig never got wired in
here" gap this session kept finding: restoreMuxSessions() in server.ts
restores every sibling CLI's config from persisted state on boot except
omp's, so a boot-recovered omp session always lost its resolved resume
id and fell back to guessing again.
Verified live end-to-end: told a session a secret, killed it fully
(Kill Tmux equivalent, killMux=true -- the Codeman session AND its tmux
pane both gone), and the conversation still showed up in the unified
list as a history-sourced row with the real first prompt as its title
and an omp mode badge, keyed by omp's own session id.
Known remaining gap, not fixed here: the claudeSessionId alias doesn't
yet resolve reliably on every boot-recovery path for a session that
was never respawned while alive (e.g. a plain re-attach to a pane that
was never dead) -- worth a follow-up, but doesn't affect the two things
that matter most: the conversation surviving a kill, and continuation
correctness once an id has been resolved (which happens on the very
next respawn either way).
Two bugs compounded to break continuation pinning on every real OMP
case (only /tmp-based manual testing happened to work by coincidence):
1. startInteractive() had a second, unconditional claudeSessionId
assignment after the mux branch that clobbered its correctly
resolved value back to the session's own id on every mux path.
2. mangleOmpWorkingDir() assumed omp mirrors Claude Code's directory
naming (home prefix kept), but omp actually strips $HOME first.
findLatestOmpSessionId() was silently returning null for every
case under ~/codeman-cases/, so resumeSessionId never resolved for
any real case dir - only /tmp paths (outside $HOME) worked, which
is every dir this feature was previously tested against.
Verified live: killed and relaunched the omp-verify server process
mid-session (plain reattach, pane stayed alive) and confirmed
claudeSessionId now resolves to the real omp transcript uuid instead
of the Codeman session's own id.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…validation
Follow-up from a full-branch review pass (Opus) of the omp-mode integration:
- Add pinning tests for resolveOmpConfigForCreate() (session-routes.ts),
exported to make it testable: the exact "resume this OMP row from
history" pipeline that mangleOmpWorkingDir's earlier bug lived in had
zero coverage despite being the resolver module's whole reason to exist.
- Log a warning when findLatestOmpSessionId() finds nothing on disk and
continuation silently degrades to omp's own ambiguous --continue,
in both call sites (session create and respawn pinning) - previously
silent, making the degradation invisible to anyone debugging it.
- Require an absolute cwd before trusting a session file's working
directory in omp-transcript.ts's parser, so a corrupted/malformed
session file can't point a downstream resume at a relative or empty
path.
- Document (don't speculatively fix) an unverified symlinked-$HOME edge
case in mangleOmpWorkingDir(): the review's suggested realpath() fix
assumes omp itself resolves symlinks before mangling, which is
unconfirmed - guessing wrong there would trade one silent mismatch
for a different one.
- Incidental: fixed unrelated pre-existing prettier drift in
session-routes.ts (antigravity/opencode dynamic import line-wrapping)
that was blocking the pre-commit formatting gate on this file.
Confirmed as a non-issue: the model-name regex allowing "/" is
intentional (provider/model ids like "crof/glm-5.2" were used
successfully in live testing).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ials
OMP had full routing at the Docker layer (default pane command, schema) but
was never actually installed in docker/agent.Dockerfile, and had no
credential-isolation entry in docker-hosts.ts's CRED_STORES - a Docker-mode
OMP session would have failed with "omp: command not found", and even with
the binary present would have had no config/auth seeded, despite the README
already claiming OMP has "seamless auth, isolated credentials" in Docker.
- docker/agent.Dockerfile: install omp via its own installer (standalone
binary, same shape as grok/antigravity - not on npm). Verified against a
real --no-cache build: the installer actually targets ~/.local/bin, not
~/.omp/bin as the resolver's OMP_SEARCH_DIRS ordering would suggest -
confirmed omp/18.0.8 installs and runs correctly inside the image.
- src/docker-hosts.ts: add a .omp/agent CRED_STORES entry. Unlike every
sibling CLI in this family, sessions/ is SHARED (RW), not seeded: Codeman
reads ~/.omp/agent/sessions/**/*.jsonl host-side for history recovery and
--resume pinning (omp-transcript.ts, omp-session-resolver.ts), the same
reason codex's sessions/ is shared rather than seeded. Seeding it instead
would silently break the kill-survival feature for Docker cases. Only the
small config files (config.yml/mcp.json/models.yml/settings.yml) are
seeded; the SQLite caches and terminal-sessions/ stay container-local.
- test/docker-hosts.test.ts: pin the new CRED_STORES entry's behavior.
Found in passing (NOT fixed here, unrelated and pre-existing on master): the
agent image's DeepSeek (dsh) plugin-install step currently fails on a fresh
build ("pnpm not found on PATH"), confirmed via git diff against
origin/master that this line is untouched by this branch. Worth a separate
issue/PR.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
OMP was the one external CLI mode with no dedicated user-guide doc, unlike
opencode/pi/grok/deepseek which each have one. Covers install, auth (omp
owns its own entirely - no Codeman-side login flow or bypass switch),
what Codeman wires up (OmpConfig), the exact-id pinning mechanism and the
directory-mangling bug behind it, kill-survival via transcript scanning,
terminal behavior, Docker/remote-SSH cases, and known gaps (no idle hook,
mid-turn kill data loss, unverified symlinked-$HOME behavior).
Cross-referenced from README.md's Multi-CLI doc list and docs/docker-cases.md's
credential-seeding summary (which now also documents OMP's sessions/-is-shared
exception to the seed-everything pattern the other CLIs use).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…onversation
Found live 2026-08-27 by Tim: clicking Run OMP to start a brand-new session
in a case directory with prior omp history launched --resume <old-id>
instead of a clean `omp` invocation.
Root cause: Session._resolvedOmpRespawnConfig() resolves-and-pins the
newest on-disk omp conversation as a side effect on this._ompConfig. That
is correct when reattaching to an ALREADY-TRACKED mux session (a dead-pane
respawn, or a boot-recovery reattach - the constructor sets _muxSession
from persisted state before startInteractive() ever runs there), but it
ran unconditionally. startInteractive() computes
`respawnPaneOptions: this._buildRespawnPaneOptions()` eagerly in the same
object literal that builds `createSessionOptions.ompConfig: this._ompConfig`,
so for a genuinely brand-new session (no muxSession in its create config,
_muxSession still null) the resolve-and-pin side effect ran and poisoned
this._ompConfig before that field was even read.
Fix: gate the resolve-and-pin logic on `this._muxSession` already being
set. A fresh session has no muxSession yet and now passes through
untouched; a real reattach (muxSession present since construction) keeps
resolving and pinning exactly as before.
Verified live in production against the exact reported scenario (a fresh
omp session in a case dir with 8+ hours of prior omp history) - confirmed
both via the API (ompConfig stays empty, claudeSessionId equals the
session's own id) and visually in the GUI. Regression test constructs a
real Session + TmuxManager to exercise the actual private-method
interaction directly, since no existing test called startInteractive() at
all.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@timkjr

Copy link
Copy Markdown
Author

Pushed an update (ab83d8f) addressing the scope-creep part of #3: the per-device "Hide CLI buttons" setting is pulled out of this PR entirely (both commits dropped, force-pushed, 14 commits left, all OMP). That was a personal-fork feature of mine that never should have ridden along here — apologies for the noise. I'll bring it back as its own small PR if/when it's ready, with grok/deepseek included in the mode list.

On the other two riders in #3: I'm going to keep those in this PR rather than split them, because they're bug fixes I found live-testing OMP, not features bolted on alongside it — resumeHistorySession() was silently opening a plain Claude session for every non-claude "Resume" click (reproduced with OMP, but it hits opencode/pi/grok/deepseek/codex/gemini/antigravity too), and the DELETE fallback exists to support retiring the stale row that bug leaves behind. I'll fix the gaps you found instead: give codex/gemini/antigravity a continuation flag (or only retire the row when the new session actually continues something), extend findSessionOrFail rather than reimplementing the ownership check inline, and add the missing session_deleted broadcast on the persisted-delete path.

Will follow up with fixes for #1 (mtime-pin correctness bug) and #2 (env clamp) as separate commits, plus the #4 items, shortly.

timkjr added 5 commits August 28, 2026 13:18
…wn time
findLatestOmpSessionId()'s newest-mtime pin ran eagerly inside
_buildRespawnPaneOptions(), which startInteractive() calls unconditionally
on every boot-recovery reattach — before anything checks whether the pane
is actually dead. With two omp tabs in the same case dir, this could pin
an ALIVE pane's session onto whichever sibling's file happened to be
newest on disk, purely as a side effect of building options that might
never lead to a respawn (reported in Ark0N#353 review).
Move resolution out of the eager builder into _pinOmpRespawnId(), called
explicitly only where a respawn is actually confirmed: the dead-pane
branch in _setupOrAttachMuxSession() and reattachRemote(). Add
resolveAndClaimOmpSessionId(), which verifies each candidate's own file
header (cwd) rather than trusting the mangled-directory match alone, and
tracks claimed ids in a process-wide registry so two ambiguous resolutions
can't both pick the same sibling's conversation.
…docs
The docs claimed omp "has no documented vendor-key namespace of its own"
and "the multi-user clamp has nothing to gate" for omp — both false. Per
omp's own docs/environment-variables.md, it reads ~40 provider keys from
env (pi's known 34-key problem in the same shape), and its own knobs are
mostly PI_* (already globally allowlisted): PI_CONFIG_DIR,
PI_CODING_AGENT_DIR, PI_CODING_AGENT_SESSION_DIR, PI_SUBPROCESS_CMD,
PI_SHELL_PREFIX. The first three also move the ~/.omp tree
omp-session-resolver.ts/omp-transcript.ts hardcode, silently degrading
pinning/history — a known gap shared with pi, documented but not fixed
here.
The OMP_* prefix this PR adds brings in OMP_AUTH_BROKER_URL/
OMP_AUTH_BROKER_TOKEN, where omp resolves credentials from — the same
shape DEEPSEEK_BASE_URL is already dropped for in
clampEnvOverridesForOwner(). Add both to OWNER_CLAMPED_ENV_KEYS so a
non-granted owner in multi-user mode can't redirect them, and correct the
false claims in CLAUDE.md, docs/omp-integration.md, and the stale
resolveOmpHome() comment. Also documents omp's default
tools.approvalMode: yolo, which was previously unstated.
…fix DELETE ownership dup + missing broadcast
resumeHistorySession() creates the resumed row in its own mode via a
modeConfigKey map (opencode/pi/grok/omp -> continueSession, deepseek ->
resumeSession) and retires the old row afterward. codex, gemini and
antigravity were missing from that map, so resuming one of their rows
started a brand-new session with NO continuation while still deleting
the row it came from -- silent data loss dressed as the duplicate-row
fix. Gate row retirement on continuesSomething (true only for modes that
actually got a continuation config) instead of wiring an unverified
sessionId->native-conversation-id assumption for the three affected CLIs.
DELETE /api/sessions/:id reimplemented the ownership 404 check inline in
two places instead of going through findSessionOrFail, and its
persisted-only-session branch never broadcast session:deleted, so other
open tabs kept the retired row until their next unrelated fetch. Extract
the shared 404 into sessionNotFoundError(), add findPersistedSessionOrFail()
alongside findSessionOrFail() in route-helpers.ts (same ownership
contract, returns a SessionState instead of a live Session), and use both
from the route instead of inline checks. Add the missing broadcast.
…y comment + CSS
Small cleanup items from upstream review (Ark0N#353):
- OMP_SEARCH_DIRS now leads with ~/.local/bin, matching omp.sh's real
installer target (~/.omp/bin was an earlier unverified guess, confirmed
wrong against a real --no-cache Docker build).
- docs/omp-integration.md: fixed the dead GitHub URL (can1357/omp ->
can1357/oh-my-pi), corrected the CLI count (ninth backend, tenth
SessionMode incl. shell -- not eighth), matched the install-path guidance
to the resolver fix, updated the version example to the actually-tested
18.0.8, and added a Docker-section caveat: --resume pinning does not
currently reach an in-container omp process, since Docker panes never see
ompConfig.
- docs/architecture-invariants.md: fixed a heading missing ", OMP" (CLAUDE.md
already linked to the -omp anchor, so the link was dead) and added an OMP
specifics paragraph -- the one external CLI missing an entry in this doc.
- .changeset/omp-backend.md: corrected the sibling-CLI list (was missing Pi,
Grok, and DeepSeek Harness) and the backend count.
- Removed a stray orphaned comment fragment in the quick-start docker branch
and split two CSS lines that had two declarations jammed onto one line.
Every other CLI (claude/opencode/codex/gemini/antigravity/pi/grok/dsh) has
a check_*/get_*_path pair wired into install.sh's detection loop and the
"no AI CLI found" aggregate checks. OMP had neither -- a user with only
omp installed would be told no CLI was found and offered to install
Claude Code or OpenCode.
Added OMP_SEARCH_PATHS (mirrors src/utils/omp-cli-resolver.ts's
OMP_SEARCH_DIRS) and check_omp()/get_omp_path(), wired into both
aggregate conditions (the interactive install-menu trigger and the
end-of-run reminder) and added omp's real vendor curl one-liner to the
reminder block. The DeepSeek Harness line was never in that reminder to
begin with -- confirmed it has no vendor one-liner (dsh installs via
Codeman's own API after the server is already up), so it stays out, with
an explanatory line instead.
Also fixed the "Skip" menu text, which was missing Gemini and DeepSeek
Harness from its example list independent of the omp gap, and the same
stale sibling-CLI-list bug (missing DeepSeek Harness and OMP, "the
eight"/"这七个") in README.md and the repo's existing README.zh-CN.md.
@timkjr

Copy link
Copy Markdown
Author

Pushed fixes for all four items:

#1 (mtime-pin correctness bug)c4f6eb1e. Moved session-id resolution out of the eager _buildRespawnPaneOptions() builder (which ran on every boot-recovery reattach regardless of whether the pane was actually dead) into _pinOmpRespawnId(), called only at the two places a respawn is confirmed. Added resolveAndClaimOmpSessionId(): verifies each candidate's own file header (not just the mangled-directory match) and tracks claimed ids in a process-wide registry so two omp tabs in the same case dir can't alias onto each other's conversation.

#2 (env-allowlist claims)2ee2eacb. You were right on both counts. Added OMP_AUTH_BROKER_URL/OMP_AUTH_BROKER_TOKEN to the multi-user clamp (same shape DEEPSEEK_BASE_URL gets) and corrected the false "nothing to gate" / "no env override exists" claims in CLAUDE.md, docs/omp-integration.md, and the resolveOmpHome() comment. Also documented the tools.approvalMode: yolo default and the shared PI_* config-root gap (left unfixed — it's pi's pre-existing issue surfacing through the shared prefix, flagged rather than silently widening scope).

#3 (resumeHistorySession + DELETE)f18dccac. For the missing continuation on codex/gemini/antigravity: rather than guess at whether the unified history list's sessionId reliably carries each CLI's own native conversation id (unverified for those three, unlike omp/claude), I took the safer option you offered — row retirement is now gated on continuesSomething, so those three rows are left alone (duplicate stays) instead of being deleted out from under a conversation with no continuation. DELETE's ownership check is deduplicated into a new findPersistedSessionOrFail() alongside findSessionOrFail(), and the persisted-only delete path now broadcasts session:deleted.

#4 (small items)65e994d2. Dead URL, CLI count (ninth backend / tenth SessionMode, not eighth — changeset and docs), OMP_SEARCH_DIRS now leads with ~/.local/bin matching the real installer target, version example updated to the actually-tested 18.0.8, stray comment and the two jammed CSS lines removed, and docs/architecture-invariants.md gets an OMP section (the heading was missing , OMP too, so the anchor CLAUDE.md already linked to was dead).

One thing I did NOT fix, flagged instead: appendResumeFlag()'s case 'omp' for Docker is confirmed unreachable in practice — Docker panes never see ompConfig, only the top-level resumeSessionId, which nothing populates for omp. Host-side history/pinning via the shared sessions/ mount still works; in-container --resume doesn't. Now documented in docs/omp-integration.md's Docker section rather than guessed at.

Full CI-equivalent gate (typecheck/lint/format/frontend-syntax/public-assets/lockfile) plus the full test suite (6330 passed) green after each commit.

One more, found after the above while double-checking the sibling-CLI counts: install.sh never had check_omp/get_omp_path at all — every other CLI (claude/opencode/codex/gemini/antigravity/pi/grok/dsh) has detection wired into the installer, omp didn't. A user with only omp installed would've been told no CLI was found and offered Claude Code/OpenCode instead. Fixed in b6d0f1fa, along with the same missing-DeepSeek-and-OMP gap in the top-level README (English and the existing Chinese translation).

@Ark0N
Ark0N merged commit da91b43 into Ark0N:masterAug 30, 2026
2 checks passed
@timkjr
timkjr deleted the omp-mode branch August 30, 2026 16:49
Ark0N pushed a commit that referenced this pull request Sep 5, 2026
…iner
feat(docker): attach a case to an already-running container
Conflicts came from work that landed after the PR was opened, and each is
resolved onto the newer abstraction rather than by keeping the older code:
- `defaultDockerCommandForMode` is registry-driven since #347, so the PR's
`runsAsRoot` arm became `overlays.docker.rootCommand` (claude only). Claude
Code still refuses `--dangerously-skip-permissions` as root in 2.1.261 and the
refusal is visible only inside the container, so an adopted root container
otherwise just shows a dead pane. Which flag to drop is a per-CLI fact, and
`test/cli-registry-no-id-branching.test.ts` forbids expressing it as a branch.
- The probe's mode list and its mode -> binary table both duplicated the
registry. They now read `enabledCliIds()` / `discovery.binaries[0]`, which is
also what fixes the merge's silent regression: the hand-written list predates
`omp`, and the run menu gates every docker case on this probe, so owned
containers would have lost that mode. `shell` needs no arm — it declares no
binary, so it is dropped from the lookup and reported available regardless.
- The per-mode `mode === 'claude' && !cliDir` chain in `tmux-manager.ts` is one
`missingCliMessage(mode)` gate since #347; the PR's docker exemption moved onto
it. Its test now pins the single gate instead of counting seven arms.
- The create arm keeps #349's swap-limit warning filter, which the adopted arm
never reaches; the run-mode list gains `omp` from #353.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TecFD9hvPYJ1mkkMtBQbT1
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@timkjr@Ark0N