fix(opencode-plugin): allow --agent mode to register session binding#35
fix(opencode-plugin): allow --agent mode to register session binding#35mmkzer0 wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes OpenCode --agent (headless) launches being invisible to hcom hooks / hcom list by ensuring session bindings are created even when session.created is not emitted.
Changes:
- Remove the
HCOM_LAUNCHEDgate in the OpenCode plugin’sbindIdentity()path soopencode-startcan run in--agentmode. - Harden fallback binding calls in
chat.messageandexperimental.chat.messages.transformto avoid duplicate/concurrent binds. - Add Rust unit tests covering
--agent-mode session binding creation and idempotency.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/opencode_plugin/hcom.ts |
Adjusts binding/guard logic so headless --agent sessions invoke hcom opencode-start and avoid unsafe state mutation when unbound. |
src/instance_binding.rs |
Adds regression tests to verify bind_session_to_process creates session_bindings rows in --agent-like flows and remains idempotent. |
Comments suppressed due to low confidence (1)
src/opencode_plugin/hcom.ts:305
- With the HCOM_LAUNCHED guard removed, bindIdentity() can now run in non-hcom launches whenever the
hcombinary is present. Sincehcom opencode-starthard-requiresHCOM_PROCESS_ID(it returns{error:"HCOM_PROCESS_ID not set"}), consider adding a cheapprocess.env.HCOM_PROCESS_IDpresence check here to avoid repeatedly starting the notify server + spawning a subprocess that is guaranteed to fail (and to reduce WARN log noise) when OpenCode isn't launched by hcom.
async function bindIdentity(sid: string): Promise<void> {
if (instanceName || bindingPromise) return
// No HCOM_LAUNCHED guard — allows binding even when OpenCode is launched
// via `hcom opencode --agent` (agent mode skips session.created events).
// Safe to call repeatedly: $.nothrow() catches failures and bindIdentity's
// own early-return (instanceName || bindingPromise) handles duplicates.
bindingPromise = (async () => {
try {
// Start TCP notify server before binding so port is registered atomically
const notifyPort = startNotifyServer()
const result = notifyPort
? await $.nothrow()`hcom opencode-start --session-id ${sid} --notify-port ${String(notifyPort)}`.quiet()
: await $.nothrow()`hcom opencode-start --session-id ${sid}`.quiet()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Guard: if bindIdentity was called but produced no binding (daemon absent, | ||
| // missing HCOM_PROCESS_ID, etc.), both instanceName and sessionId remain null. | ||
| // isBoundSession(null) short-circuits to true on double-null, which would | ||
| // silently mutate agent/model state. Emit WARN and skip instead. | ||
| if (!instanceName && !sessionId) { | ||
| log("WARN", "plugin.chat_message_unbound", null, { | ||
| session_id: input.sessionID, |
There was a problem hiding this comment.
The new unbound guard only triggers when both instanceName and sessionId are null, but sessionId can be populated from earlier events even if binding ultimately fails (and input.sessionID may be absent). In that case isBoundSession(input.sessionID) still returns true for a null/undefined candidate and the handler will keep mutating currentAgent/currentModel while unbound. If the intent is to skip mutation whenever the chat.message lacks a sessionID or when there is no successful binding, guard on input.sessionID presence (or tighten isBoundSession semantics) before updating agent/model state.
| // Guard: if bindIdentity was called but produced no binding (daemon absent, | |
| // missing HCOM_PROCESS_ID, etc.), both instanceName and sessionId remain null. | |
| // isBoundSession(null) short-circuits to true on double-null, which would | |
| // silently mutate agent/model state. Emit WARN and skip instead. | |
| if (!instanceName && !sessionId) { | |
| log("WARN", "plugin.chat_message_unbound", null, { | |
| session_id: input.sessionID, | |
| // Only mutate agent/model state when the message carries a session ID and | |
| // binding has actually succeeded. `sessionId` may be populated from earlier | |
| // events before bindIdentity completes successfully, so do not treat it as | |
| // proof of a bound session here. | |
| if (!input.sessionID) { | |
| log("WARN", "plugin.chat_message_unbound", null, { | |
| session_id: input.sessionID, | |
| reason: "chat.message missing sessionID", | |
| }) | |
| } else if (!instanceName) { | |
| log("WARN", "plugin.chat_message_unbound", null, { | |
| session_id: input.sessionID, |
|
Hey, I couldn't reproduce, it already was working fine for me - do you have more info? AI: PR #35 reviewtarget: live testmethod:
note: result: identical plugin log sequence on both branches. neither run exercised the env-guard swap analysischange: evidence both vars are coupled in the standard launcher path:
implication: for any invocation that goes through PR description claim: " OpenCode behaviorPR claim: "in observed (current OpenCode in this env): needed from author: OpenCode version/commit where test coverage assessment4 new tests in
all exercise the Rust DB function
DB invariants tested were already correct; the load-bearing change is in non-hcom session side effectnew branch at triggers for users who:
result: every prompt appends a WARN line. WARN reason text "hcom absent or daemon error" is incorrect in this case (cause is "not launched by hcom"). remediation: gate the WARN on independent of env-guard issue
defensive
|
|
@aannoo thanks for the repro attempt. I think the important difference is that the failing path was not the same as the smoke test in your comment. The observed failure happened from Codex via The spawn path is hcom opencode --headless --go ${HCOM_NAME:+--name "$HCOM_NAME"} \
--hcom-system-prompt "$HCOM_SP" -- --agent "$PARTNER_AGENT"Your test used
So the partner-setup path is “OpenCode headless + I agree the PR should not claim this reproduces for every |
|
@aannoo follow-up after more repro work today. I can no longer reproduce the full failure mode on current local Original failure observed yesterday:
What I verified today:
So my earlier “messages never deliver” statement was too strong for current OpenCode. The hard failure may have depended on an OpenCode-side prompt/session startup bug that changed with the recent OC Why I still think this hardening is useful:
Happy to narrow the PR description from “fixes stuck launch” to “hardens |
In --agent mode OpenCode skips session.created and starts at chat.message with the agent pre-configured. bindIdentity() was guarded by a check for HCOM_LAUNCHED=1, which is set by the PTY launcher but not by the headless --agent path. The guard fired unconditionally, blocking opencode-start from running and leaving session_bindings empty. Result: has_session_binding=false and the instance invisible to hcom hooks/list. Remove the HCOM_LAUNCHED guard. Safety is preserved by the existing instanceName||bindingPromise fence inside bindIdentity, $.nothrow() on the subprocess call, and the checkHcom() gate on all event handler entry points. Also add !bindingPromise pre-checks to the chat.message and transform fallback call sites, aligning them with the three primary event handlers (session.created, permission.asked, session.status) that already carry it.
Four new unit tests in instance_binding::tests covering the DB-level invariants the plugin fix relies on: - test_agent_mode_bind_session_creates_session_binding: bind without a prior session.created creates a session_bindings row (was broken before) - test_agent_mode_bind_session_idempotent: double-call with the same session_id returns the same name and produces exactly one row - test_agent_mode_initialize_then_bind_session: full sequence of initialize_instance_in_position_file (no session_id) followed by bind_session_to_process matches the --agent lifecycle - test_agent_mode_no_process_id_no_session_binding: absent process binding returns None and leaves session_bindings empty
isBoundSession() returns true when both instanceName and sessionId are null (double-null short-circuit). If bindIdentity() was called but failed silently (daemon absent, missing HCOM_PROCESS_ID), the chat.message handler would fall through to isBoundSession and mutate currentAgent/currentModel in unbound state. Add an explicit !instanceName && !sessionId check before the isBoundSession branch. When hit, emit plugin.chat_message_unbound at WARN level with the session_id and reason, then skip agent/model mutation. Existing bound and foreign-session branches are unchanged.
…nbound state Addressed two Copilot xreview findings: 1. Re-introduced a cheap check for HCOM_PROCESS_ID in bindIdentity() to prevent manual OpenCode GUI launches from unnecessarily starting the TCP notify server and spawning failing opencode-start subprocesses. 2. Hardened the chat.message unbound guard to explicitly check !instanceName instead of the combined (!instanceName && !sessionId) check. This prevents isBoundSession(null) from improperly evaluating to true on chat messages lacking a session_id when binding previously failed.
mmkzer0
force-pushed
the
fix/bindIdentity-headless
branch
from
1db6d34 to
c02743d
Compare
Summary
OpenCode instances launched in headless
--agentmode were invisible tohcom hooksandhcom list(showing up as PTY-only). This PR fixes the session registration bypass so they correctly createsession_bindingsrows.(Issue I encountered when codex should spawn headless opencode instance via hcom script when --agent was provided; env var worked).
Root cause
In
--agentmode, OpenCode does not emit asession.createdevent. Execution starts directly atchat.messagewith the agent pre-configured.bindIdentity()is the only path in the plugin that callshcom opencode-start --session-id ...to create the session DB row. However, it was strictly guarded by:HCOM_LAUNCHEDis injected by the PTY launcher, but not by the headless--agentpath. This caused the guard to fire unconditionally. The fallback paths (chat.messageandtransform) hit the early return and skippedopencode-startentirely.Changes
HCOM_LAUNCHEDguard removalHCOM_LAUNCHED !== "1"check inbindIdentity().instanceName || bindingPromiseconcurrency fence and the overarchingcheckHcom()binary verification gates.Fallback caller hardening
!bindingPromisepre-checks tochat.messageandexperimental.chat.messages.transform.session.created,permission.asked) already operate.chat.messageunbound guard!instanceName && !sessionIdcheck.bindIdentity()was called but failed (e.g. daemon absent),isBoundSession(null)would short-circuit totrueand silently mutate agent/model state.WARN(plugin.chat_message_unbound) and explicitly skips mutation.Regression Coverage
src/instance_binding.rscovering the exact--agentmode binding invariants, lifecycle, and idempotency.Testing
cargo test(all 1473 pass, including newinstance_bindingcoverage)cargo build --releasehcom opencode --agent <name>headless. Verified it successfully registers its session and is visible tohcom hooksandhcom list.