Skip to content

feat: add OMP (Oh My Pi) coding agent support#975

Open
KorenKrita wants to merge 6 commits into
tiann:mainfrom
KorenKrita:feature/omp-agent-support
Open

feat: add OMP (Oh My Pi) coding agent support#975
KorenKrita wants to merge 6 commits into
tiann:mainfrom
KorenKrita:feature/omp-agent-support

Conversation

@KorenKrita

Copy link
Copy Markdown
Contributor

Summary

Adds OMP (Oh My Pi coding agent, @oh-my-pi/pi-coding-agent) as a first-class agent flavor, with feature parity to the Claude Code integration on the core loop and OMP's unique capabilities surfaced through hapi's existing generic channels (no reducer or RPC-architecture changes).

What is included

Shared layer

  • 'omp' added to AGENT_FLAVORS; OMP_PERMISSION_MODES = ['yolo'] (OMP rpc mode has no runtime permission switching — --approval-mode is fixed at spawn).
  • FLAVOR_CAPS / FLAVOR_LABELS entry; getPermissionModesForFlavor('omp') returns the yolo-only list.
  • RPC_METHODS.ListOmpModels + OMP superset RPC method constants (OmpCompact, OmpSetSteeringMode, OmpSetInterruptMode, OmpSetFollowUpMode, OmpCycleModel, OmpCycleThinkingLevel, OmpGetSessionStats).
  • OmpModelSummary / OmpModelsResponse / OmpCommandSummary types (thinking info carried as efforts array + effortMap + defaultLevel, matching OMP's Model.thinking shape).
  • MetadataSchema gains ompSessionId / ompSessionFile / ompAvailableModels / ompSelectedModel.

CLI (cli/src/omp/, new directory)

OMP speaks the same JSONL-over-stdio RPC family as Pi (shared @oh-my-pi/pi-agent-core + pi-ai), so the module mirrors the existing per-agent directory layout. Protocol-specific differences handled:

  • ready handshake gate — OMP emits {"type":"ready"} before accepting commands; the transport buffers sends until ready (with a 10s timeout + immediate reject on spawn ENOENT).
  • Slash commands via push — OMP streams available_commands_update frames; the loop caches them and falls back to get_available_commands.
  • Model thinking extraction — schemas read model.thinking.efforts / effortMap / defaultLevel (per-model level list), not a flat map.
  • Resume--continue flag (OMP's SessionManager.continueRecent); new_session is skipped on resume so the restored context is not discarded.
  • Binary resolutionresolveOmpCommand() finds ~/.bun/bin/omp (the bun global install symlink is often off-PATH).
  • Spawn flags--approval-mode yolo fixed at spawn.

OMP-only events map onto hapi's existing generic web events (no new event types):

  • goal_updated -> thread_goal_updated (with budget-limited -> budgetLimited, dropped/null -> thread_goal_cleared), reusing the latestGoal reducer + presentation.
  • auto_compaction_start/end -> compact event.
  • thinking_level_changed -> updates session state + keepAlive (validated against the thinking-level enum).
  • subagent frames -> sidechain messages (reuse the existing isSidechain / parentUUID tracer).
  • Other events (todo_reminder, notice, auto_retry_*, ttsr_triggered, ...) pass through the AgentEvent catch-all.

Single-direction superset RPCs registered in runOmp: compact, steering/interrupt/follow-up mode switches, cycle model/thinking level, session stats.

Hub

  • callOmpRpc in rpcGateway + syncEngine; withOmpSession flavor guard + GET /sessions/:id/omp-models route.
  • extractAgentSessionId recognizes ompSessionId.

Web

  • useOmpModels hook + callOmpEndpoint client method + sessionOmpModels query key.
  • OmpModelPanel + ompModelGroups (provider grouping).
  • OmpThinkingLevelPanel + ompThinkingLevelOptions — filters levels by the model's efforts array (case-insensitive).
  • HappyComposer conditional rendering for the omp flavor; SessionChat wires the model query + metadata; AgentFlavorIcon omp badge; NewSession MODEL_OPTIONS entry; locales.

Out of scope

host_tool_call / host_uri_request / extension_ui_request are OMP's extension-ecosystem hooks — OMP does not send them unless the host registers tools/URI schemes, so they are not wired in this PR.

Tests

  • cli/src/omp/: 5 test files (schemas, accumulator, converter, transport, loop) — 80 tests covering the ready gate, markReady killed-guard, ENOENT rejecting ready() immediately, close-event code/signal delivery, goal field mapping, initialModelApplied race guard, malformed-response skipping, schema fault tolerance.
  • web/: ompThinkingLevelOptions.test.ts — 13 tests covering efforts filtering, case normalization, dedup, non-standard current-level retention.

Verification

  • bun typecheck — 0 errors across cli/hub/web.
  • bun run test — 80/80 omp tests + 13/13 web tests pass; full non-integration suite 1188/1188 pass.
  • Live smoke: OmpTransport spawn -> ready handshake -> prompt -> streaming text deltas -> turn_complete, confirmed against a real OMP process.

The only failing test in the suite is the pre-existing runner.integration.test.ts > stress test: spawn/stop flake; this branch does not touch runner code.

Files

  • 13 new files (cli/src/omp/ source + tests, cli/src/commands/omp.ts, web Omp panels + hook + tests)
  • 19 modified files (shared enums/types/schema, hub rpc gateway + routes + cache, web client + composer + session chat + types)

Add OMP (@oh-my-pi/pi-coding-agent) as a first-class agent flavor,
aligning with the existing Claude Code adaptation and surfacing OMP's
unique capabilities without restructuring hapi's core.

OMP shares Pi's RPC protocol family (both build on @oh-my-pi/pi-agent-core
+ pi-ai), so cli/src/omp/ mirrors cli/src/pi/ with targeted protocol-diff
changes:
- ready-frame handshake gate (OMP pushes {type:ready} before accepting cmds)
- slash commands via available_commands_update push (not get_commands)
- Model thinking info under model.thinking.efforts/effortMap/defaultLevel
- resume via --continue (not --session-id)
- omp binary resolved from ~/.bun/bin/omp (not on PATH)
- spawn fixed --approval-mode=yolo (rpc has no runtime permission switch)

OMP-only capabilities map onto hapi's existing generic mechanisms — no
reducer/RPC-architecture changes:
- goal_updated -> thread-goal-updated (reuse latestGoal reducer)
- subagent frames -> sidechain (reuse tracer)
- auto_compaction_* -> compact event (reuse presentation)
- other events -> AgentEvent catch-all
- steer/follow_up/compact/branch/bash/handoff + steering/interrupt mode
  switches via single-direction RPC handlers

Out of scope (OMP extension ecosystem, optional): host_tool_call,
host_uri_request, extension_ui_request — OMP does not send these unless
the host registers tools/URI schemes.

Verification: bun typecheck 0 error; bun run test 1119/1120 pass (sole
failure is the pre-existing runner.integration spawn/stop flake from
PR tiann#923, unrelated to omp); live smoke spawn->ready->prompt->streaming
text->turn_complete confirmed (OMP glm-5.2 replies).

6-round OCR code review converged to 0 comments; 29 real bugs fixed
across rounds (schema fault-tolerance, transport close/ENOENT/EPIPE/
stdin-error handling, ready/markReady race guards, initialModel/
initialThinkingLevel race flags, thinkingLevel runtime validation,
efforts case-normalization, cycle-response type guards, response-shape
validation). Style-only findings that mirror Pi's existing patterns were
left unchanged to avoid Pi/omp divergence.
Cover the OMP adapter's pure-logic surfaces that the OCR review kept
flagging (29 real bugs across 6 rounds were in exactly these spots):

cli/src/omp/ (80 tests, mirrors pi/'s 4 test files):
- schemas.test: OmpModelEntrySchema extraction from model.thinking.efforts/
  effortMap/defaultLevel, .catch(undefined) fault tolerance for
  thinking/model/enum fields, parseOmpModels/parseOmpCommands edge cases,
  OmpResponseEventSchema rejecting malformed responses
- ompMessageAccumulator.test: text/thinking delta accumulation, flush on
  message_end + turn_end safety net, contentIndex streamId, reset on
  message_start, no double-flush
- ompEventConverter.test: tool_execution_start/end -> tool_call/tool_result,
  turn_end -> usage + turn_complete, empty cases for goal/compact/ready/
  response (handled elsewhere), crash safety
- ompTransport.test: ready-frame gate (buffer until ready, flush after),
  markReady killed-guard (no flush to dying process), ENOENT rejects
  readyPromise immediately (no 10s wait), EPIPE handling, stdout 'end'
  does not steal close's real code/signal, JSONL split reassembly
- loop.test: get_state updates model/sessionId/sessionFile/modes,
  thinkingLevel enum validation, available_commands_update caching,
  goal_updated field mapping (budget-limited -> budgetLimited, dropped ->
  cleared, null -> cleared), auto_compaction -> compact event,
  thinking_level_changed validation, malformed response skipped (not
  passed to handleResponse), initialModelApplied race guard, turn_start
  FIFO drain, agent_end updates thinking state

web/ (13 tests):
- ompThinkingLevelOptions.test: efforts filtering + case normalization
  (High -> high), dedup, non-standard current level kept + not duplicated,
  auto/default -> null, getHighestOmpThinkingLevel casing

Verification: 80/80 omp tests + 13/13 web tests pass; full non-integration
suite 1188/1188 pass; typecheck 0 error. Sole remaining flake is the
pre-existing runner.integration spawn/stop test (unrelated to omp).

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Blocker] OMP web launches still spawn Claude — adding omp to the shared flavor list makes it selectable in New Session, but runner spawn argument construction has no omp branch, so buildCliArgs('omp', ...) falls through to claude. Creating an OMP session from the web/runner will start Claude instead of hapi omp. Evidence shared/src/modes.ts:10, related context cli/src/runner/run.ts:1076.
    Suggested fix:

    const agentCommand = agent === 'codex'
        ? 'codex'
        : agent === 'cursor'
            ? 'cursor'
            : agent === 'gemini'
                ? 'gemini'
                : agent === 'kimi'
                    ? 'kimi'
                    : agent === 'opencode'
                        ? 'opencode'
                        : agent === 'pi'
                            ? 'pi'
                            : agent === 'omp'
                                ? 'omp'
                                : 'claude'
    
    if (options.effort && (agent === 'claude' || agent === 'pi' || agent === 'omp')) {
        args.push('--effort', options.effort)
    }
  • [Major] OMP resume metadata is written but not used by resume flows — this PR adds ompSessionId, but resolveAgentResumeId() and the web-side resolveAgentSessionIdFromMetadata() still fall through to Claude for flavor === 'omp'; hapi resume also has no OMP dispatch branch and would fall through to Cursor once the hub starts returning OMP targets. Inactive OMP sessions with messages therefore show/return resume unavailable instead of using the stored OMP id. Evidence shared/src/schemas.ts:53, related context hub/src/sync/syncEngine.ts:752 and cli/src/commands/resume.ts:165.
    Suggested fix:

    // hub/src/sync/syncEngine.ts
    if (flavor === 'omp') return metadata.ompSessionId ?? null
    
    // web/src/lib/sessionResume.ts
    case 'omp': return metadata.ompSessionId ?? undefined
    
    // cli/src/commands/resume.ts
    if (target.flavor === 'omp') {
        const { runOmp } = await import('@/omp/runOmp')
        await runOmp({
            existingSessionId: base.existingSessionId,
            workingDirectory: base.workingDirectory,
            resumeSessionId: base.resumeSessionId,
            startedBy: base.startedBy,
            startingMode: 'remote',
            model: target.model ?? undefined,
            effort: target.effort ?? undefined,
        })
        return
    }

Summary

  • Review mode: initial
  • OMP core loop has coverage, but the flavor is not fully wired into runner spawn/resume entrypoints yet.

Testing

  • Not run (automation)

HAPI Bot

Comment thread shared/src/modes.ts
export const AGENT_MESSAGE_PAYLOAD_TYPE = 'codex' as const

export const AGENT_FLAVORS = ['claude', 'codex', 'cursor', 'gemini', 'kimi', 'opencode', 'pi'] as const
export const AGENT_FLAVORS = ['claude', 'codex', 'cursor', 'gemini', 'kimi', 'opencode', 'pi', 'omp'] as const

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[BLOCKER] Adding omp here exposes it through New Session, but the runner's buildCliArgs() has no omp branch and currently maps unknown agents to claude, so web/runner-created OMP sessions start Claude instead of OMP.

Suggested fix:

const agentCommand = agent === 'codex'
    ? 'codex'
    : agent === 'cursor'
        ? 'cursor'
        : agent === 'gemini'
            ? 'gemini'
            : agent === 'kimi'
                ? 'kimi'
                : agent === 'opencode'
                    ? 'opencode'
                    : agent === 'pi'
                        ? 'pi'
                        : agent === 'omp'
                            ? 'omp'
                            : 'claude'

Comment thread shared/src/schemas.ts
piSessionId: z.string().optional(),
// OMP session resume token — `get_state` returns sessionId/sessionFile.
// `--continue` restores the most-recent session in the cwd-derived dir.
ompSessionId: z.string().optional(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] ompSessionId is stored, but the resume helpers do not read it yet. SyncEngine.resolveAgentResumeId() still falls through to Claude for OMP, the web resolveAgentSessionIdFromMetadata() does the same, and hapi resume has no OMP dispatch branch. Inactive OMP sessions with messages will be treated as non-resumable.

Suggested fix:

if (flavor === 'omp') return metadata.ompSessionId ?? null
case 'omp': return metadata.ompSessionId ?? undefined
if (target.flavor === 'omp') {
    const { runOmp } = await import('@/omp/runOmp')
    await runOmp({ existingSessionId: base.existingSessionId, workingDirectory: base.workingDirectory, resumeSessionId: base.resumeSessionId, startedBy: base.startedBy, startingMode: 'remote', model: target.model ?? undefined, effort: target.effort ?? undefined })
    return
}

Addresses PR review findings:

[Blocker] runner spawn: buildCliArgs had no omp branch, so
buildCliArgs('omp', ...) fell through to 'claude' — creating an OMP
session from web/runner started Claude instead of 'hapi omp'. Add the
omp branch (agentCommand='omp', resume via --resume, --effort applies,
permission flags skipped since rpc mode fixes --approval-mode at spawn).

[Major] resume flows: ompSessionId was written but never consumed.
- hub syncEngine.resolveAgentResumeId: return ompSessionId for omp flavor
- web resolveAgentSessionIdFromMetadata: case 'omp' -> ompSessionId
- cli resume command: dispatch omp targets to runOmp (mirrors the pi
  branch; remote-only since omp has no local TUI input path)

Tests:
- buildCliArgs.test: omp spawns 'omp' not 'claude', --resume for resume,
  --effort passes, permission flags skipped
- resume.test: omp target resumes via runOmp with effort
- sessionResume.test: ompSessionId returned for omp flavor, cross-flavor
  ids ignored

typecheck 0 error; 106 cli + 43 web resume/spawn tests pass.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] OMP resume uses “most recent” instead of the selected session — resolveAgentResumeId() returns ompSessionId, but runOmp() only turns any resume id into --continue, which OMP resolves as the most recent session in that cwd-derived directory. If a directory has more than one OMP session, hapi resume <session-id> / web resume can attach the wrong conversation even though metadata also stores ompSessionFile. Evidence cli/src/omp/runOmp.ts:72, related context hub/src/sync/syncEngine.ts:759.
    Suggested fix:

    // hub/src/sync/syncEngine.ts
    if (flavor === 'omp') return metadata.ompSessionFile ?? metadata.ompSessionId ?? null
    
    // cli/src/omp/runOmp.ts, after transport.ready()
    if (opts.resumeSessionId) {
        transport.send({ type: 'switch_session', sessionPath: opts.resumeSessionId })
    } else {
        transport.send({ type: 'new_session' })
    }
  • [Major] OMP permission mode control always fails for active sessions — OMP exposes a yolo permission option, so the composer renders a permission-mode action and posts /permission-mode; the hub forwards { permissionMode: 'yolo' } through SetSessionConfig, but the OMP handler never includes permissionMode in applied, so applySessionConfig() rejects with Session did not apply permissionMode. Evidence shared/src/modes.ts:133, related context cli/src/omp/runOmp.ts:235.
    Suggested fix:

    const appliedPermissionMode = config.permissionMode === undefined ? undefined : 'yolo'
    if (config.permissionMode !== undefined && config.permissionMode !== 'yolo') {
        throw new Error('Invalid permission mode')
    }
    
    return {
        applied: {
            ...(appliedPermissionMode ? { permissionMode: appliedPermissionMode } : {}),
            model: appliedModel,
            effort: ompSession.currentThinkingLevel,
        },
    }

Summary

  • Review mode: follow-up after new commits
  • The previous runner-spawn and resume-dispatch findings are fixed in the latest head. Remaining issues are in OMP resume precision and the newly exposed OMP permission-mode path.

Testing

  • Not run (automation)

HAPI Bot

Comment thread cli/src/omp/runOmp.ts Outdated
// `--session-id` rpc flag (unlike Pi); precise resume is via the
// switch_session RPC at runtime, keyed off ompSessionFile metadata.
const transportArgs = defaultOmpArgs();
if (opts.resumeSessionId) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] OMP resume uses “most recent” instead of the selected session. resolveAgentResumeId() currently returns ompSessionId, but this branch ignores the id and only starts OMP with --continue, which OMP resolves as the most recent session in the cwd-derived directory. If the same directory has multiple OMP sessions, hapi resume <session-id> / web resume can attach the wrong conversation even though metadata stores ompSessionFile.

Suggested fix:

// hub/src/sync/syncEngine.ts
if (flavor === 'omp') return metadata.ompSessionFile ?? metadata.ompSessionId ?? null

// cli/src/omp/runOmp.ts, after transport.ready()
if (opts.resumeSessionId) {
    transport.send({ type: 'switch_session', sessionPath: opts.resumeSessionId })
} else {
    transport.send({ type: 'new_session' })
}

Comment thread shared/src/modes.ts
// no permission modes are offered.
return []
}
if (flavor === 'omp') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] This exposes a yolo permission-mode option for OMP, so the composer posts /permission-mode; for active sessions the hub forwards { permissionMode: 'yolo' } into OMP SetSessionConfig. The OMP handler never returns permissionMode in applied, so applySessionConfig() rejects with Session did not apply permissionMode and the visible control always fails.

Suggested fix:

const appliedPermissionMode = config.permissionMode === undefined ? undefined : 'yolo'
if (config.permissionMode !== undefined && config.permissionMode !== 'yolo') {
    throw new Error('Invalid permission mode')
}

return {
    applied: {
        ...(appliedPermissionMode ? { permissionMode: appliedPermissionMode } : {}),
        model: appliedModel,
        effort: ompSession.currentThinkingLevel,
    },
}

Addresses follow-up PR review findings:

[Major] resume precision: runOmp turned any resume id into --continue,
which restores OMP's most-recent session in the cwd-derived dir — wrong
when a directory holds more than one OMP session. Now start a fresh rpc
session and switch_session to the exact ompSessionFile. The hub resolves
resumeSessionId to ompSessionFile (falling back to ompSessionId) so the
CLI receives the precise session path. switch_session is awaited and
falls back to new_session on failure.

[Major] permissionMode applied: OMP exposes a 'yolo' permission option,
so the composer posts /permission-mode and the hub forwards
{permissionMode:'yolo'} through SetSessionConfig, but the OMP handler
never echoed it back in 'applied', so applySessionConfig rejected with
'Session did not apply permissionMode'. Now validate permissionMode==='yolo'
(reject others) and include applied.permissionMode='yolo' in the response.

typecheck 0 error; 106 omp+resume+buildCliArgs tests pass.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] OMP resume silently creates a fresh conversation when exact restore fails — when switch_session rejects or times out, this fallback sends new_session inside the existing HAPI session row. A missing/moved ompSessionFile or transient OMP RPC failure would make hapi resume <session-id> / web resume appear successful while the user is actually in an empty OMP conversation, risking prompts being sent with no prior context. Evidence cli/src/omp/runOmp.ts:490.
    Suggested fix:
    if (opts.resumeSessionId) {
        await sendOmpRpcAndWait(ompSession, transport, {
            type: 'switch_session',
            sessionPath: opts.resumeSessionId,
        })
    } else {
        transport.send({ type: 'new_session' })
    }

Questions

  • None.

Summary

  • Review mode: follow-up after new commits
  • The prior exact-resume and OMP permission-mode findings are addressed, but the new resume fallback still lets a failed exact restore continue as a fresh OMP session under the selected HAPI session.

Testing

  • Not run (automation)

HAPI Bot

Comment thread cli/src/omp/runOmp.ts Outdated
}
} catch (error) {
logger.debug(`[omp] switch_session failed, falling back to fresh session: ${error instanceof Error ? error.message : String(error)}`);
transport.send({ type: 'new_session' });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] OMP resume silently creates a fresh conversation when exact restore fails. If switch_session rejects or times out, this sends new_session inside the existing HAPI session row, so hapi resume <session-id> / web resume can appear successful while the selected conversation context was not restored. That is especially risky after the hub now passes ompSessionFile for exact resume: a missing/moved file should fail visibly instead of starting empty.

Suggested fix:

if (opts.resumeSessionId) {
    await sendOmpRpcAndWait(ompSession, transport, {
        type: 'switch_session',
        sessionPath: opts.resumeSessionId,
    })
} else {
    transport.send({ type: 'new_session' })
}

Address third-round PR review finding:

[Major] resume fallback created an empty conversation on failure —
switch_session rejection/timeout fell back to new_session inside the
existing HAPI session row, so a missing/moved ompSessionFile or transient
RPC failure made resume look successful while the user landed in an empty
OMP conversation (prompts sent with no prior context). Drop the fallback:
let switch_session failure propagate so resume fails loudly, and throw on
extension-cancelled switch too.

typecheck 0 error; omp tests pass.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] OMP resume can switch to the wrong provider when model IDs collide — the startup model reapply only matches modelId, even though this PR persists ompSelectedModel specifically because duplicate model IDs can exist across providers. On resume, the hub/local resume paths pass the legacy session.model string, then switch_session restores the OMP session and get_state may report the correct provider, but this later get_available_models branch can immediately call set_model for the first provider with that modelId. That silently changes the resumed conversation's model/provider. Evidence cli/src/omp/loop.ts:234.
    Suggested fix:
    const providerHint = session.initialModelProvider ?? session.currentProvider
    const match = providerHint
        ? models.find((m) => m.provider === providerHint && m.modelId === session.initialModel)
        : models.find((m) => m.modelId === session.initialModel)

Questions

  • None.

Summary

  • Review mode: follow-up after new commits
  • The previous resume fallback issue is fixed. One remaining OMP resume/model-state issue can still change provider when duplicate modelIds exist.

Testing

  • Not run (automation); local bun typecheck could not run here because bun is not installed in this runner image.

HAPI Bot

Comment thread cli/src/omp/loop.ts Outdated
// so resolving the get_available_models RPC itself is not blocked.
if (session.initialModel && transport && !session.initialModelApplied) {
session.initialModelApplied = true;
const match = models.find((m) => m.modelId === session.initialModel);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] OMP resume can switch to the wrong provider when model IDs collide. This branch only matches the startup request by modelId, but the PR also adds ompSelectedModel because OMP model IDs are not globally unique across providers. After switch_session, get_state can restore the original provider, then this get_available_models handler may immediately choose the first provider with the same modelId and send set_model, silently changing the resumed session's provider.

Suggested fix:

const providerHint = session.initialModelProvider ?? session.currentProvider
const match = providerHint
    ? models.find((m) => m.provider === providerHint && m.modelId === session.initialModel)
    : models.find((m) => m.modelId === session.initialModel)

Resume restores the OMP session's provider via get_state before
get_available_models arrives. The startup-model apply matched only by
modelId, so a modelId existing under multiple providers resumed to the
first provider — silently switching the resumed conversation's provider.

Prefer currentProvider when known; fall back to modelId-only when not.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • No issues found.

Questions

  • None.

Summary
Review mode: follow-up after new commits
No new high-confidence issues found in the latest full diff. The prior provider-collision finding appears addressed by matching the startup OMP model against currentProvider before falling back to model ID. Residual risk: this review did not execute the external OMP runtime.

Testing

  • Not run (automation; review-only, did not execute PR code).

HAPI Bot

Sign up for free to 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.

1 participant