From f285b6a48fce898245da681d0b159af08d3bfdc2 Mon Sep 17 00:00:00 2001 From: Lennart Kotzur Date: Fri, 1 May 2026 13:11:33 +0200 Subject: [PATCH 1/4] fix(opencode-plugin): allow --agent mode to register session binding 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. --- src/opencode_plugin/hcom.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/opencode_plugin/hcom.ts b/src/opencode_plugin/hcom.ts index d966d579..682b5caf 100644 --- a/src/opencode_plugin/hcom.ts +++ b/src/opencode_plugin/hcom.ts @@ -292,8 +292,10 @@ export const HcomPlugin: Plugin = async ({ client, $ }) => { async function bindIdentity(sid: string): Promise { if (instanceName || bindingPromise) return - if (process.env.HCOM_LAUNCHED !== "1") 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 @@ -451,7 +453,7 @@ export const HcomPlugin: Plugin = async ({ client, $ }) => { sessionId = input.sessionID } if (bindingPromise) await bindingPromise - if (input.sessionID && !instanceName) { + if (input.sessionID && !instanceName && !bindingPromise) { await bindIdentity(input.sessionID) } if (isBoundSession(input.sessionID)) { @@ -475,7 +477,9 @@ export const HcomPlugin: Plugin = async ({ client, $ }) => { try { if (!checkHcom()) return if (bindingPromise) await bindingPromise - if (!instanceName && sessionId) await bindIdentity(sessionId) + if (!instanceName && !bindingPromise && sessionId) { + await bindIdentity(sessionId) + } if (!instanceName || !sessionId) return // OpenCode transform mutations are prompt-local, not persisted to stored From 40fd948b07c2c14b90c7e749db64be45ea95a0c3 Mon Sep 17 00:00:00 2001 From: Lennart Kotzur Date: Fri, 1 May 2026 13:12:02 +0200 Subject: [PATCH 2/4] test(binding): add --agent mode regression tests 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 --- src/instance_binding.rs | 139 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/src/instance_binding.rs b/src/instance_binding.rs index 115a938f..7c519744 100644 --- a/src/instance_binding.rs +++ b/src/instance_binding.rs @@ -1199,4 +1199,143 @@ mod tests { cleanup(path); } + + // ── --agent mode regression tests ──────────────────────────────────────── + // + // In `--agent` mode OpenCode skips `session.created` and fires `chat.message` + // first. `bindIdentity()` is the only path that calls `opencode-start` and + // creates a `session_bindings` row. The tests below verify the DB-level + // invariants that the plugin fix relies on. + + /// Regression: calling `bind_session_to_process` without a prior + /// `session.created` (the --agent path) must still create a `session_bindings` + /// row. Before the fix, `HCOM_LAUNCHED` blocked `bindIdentity` and no row + /// was ever inserted. + #[test] + fn test_agent_mode_bind_session_creates_session_binding() { + crate::config::Config::init(); + let (db, path) = setup_test_db(); + let now = now_epoch_i64(); + + // Simulate: instance placeholder created by the launcher (no session_id yet, + // matching the state before session.created would fire in normal mode). + let mut data = serde_json::Map::new(); + data.insert("name".into(), serde_json::json!("agent-alpha")); + data.insert("status".into(), serde_json::json!("pending")); + data.insert("status_context".into(), serde_json::json!("new")); + data.insert("created_at".into(), serde_json::json!(now)); + data.insert("tool".into(), serde_json::json!("opencode")); + db.save_instance_named("agent-alpha", &data).unwrap(); + db.set_process_binding("proc-agent-1", "", "agent-alpha").unwrap(); + + // The plugin now calls this on the first chat.message (--agent path). + let result = bind_session_to_process(&db, "sess-agent-1", Some("proc-agent-1")); + assert_eq!(result, Some("agent-alpha".to_string())); + + // session_bindings row must exist (was missing before the fix). + let binding = db.get_session_binding("sess-agent-1").unwrap(); + assert_eq!(binding, Some("agent-alpha".to_string())); + + // Instance must carry the session_id. + let inst = db.get_instance_full("agent-alpha").unwrap().unwrap(); + assert_eq!(inst.session_id.as_deref(), Some("sess-agent-1")); + + cleanup(path); + } + + /// `bind_session_to_process` called twice with the same session_id must be + /// idempotent: same name returned, no duplicate session_bindings rows. + #[test] + fn test_agent_mode_bind_session_idempotent() { + crate::config::Config::init(); + let (db, path) = setup_test_db(); + let now = now_epoch_i64(); + + let mut data = serde_json::Map::new(); + data.insert("name".into(), serde_json::json!("agent-beta")); + data.insert("status".into(), serde_json::json!("pending")); + data.insert("status_context".into(), serde_json::json!("new")); + data.insert("created_at".into(), serde_json::json!(now)); + data.insert("tool".into(), serde_json::json!("opencode")); + db.save_instance_named("agent-beta", &data).unwrap(); + db.set_process_binding("proc-agent-2", "", "agent-beta").unwrap(); + + let r1 = bind_session_to_process(&db, "sess-agent-2", Some("proc-agent-2")); + let r2 = bind_session_to_process(&db, "sess-agent-2", Some("proc-agent-2")); + assert_eq!(r1, Some("agent-beta".to_string())); + assert_eq!(r2, Some("agent-beta".to_string())); + + // Exactly one session_bindings row. + let count: i64 = db + .conn() + .query_row( + "SELECT COUNT(*) FROM session_bindings WHERE session_id = ?", + rusqlite::params!["sess-agent-2"], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(count, 1, "must have exactly one session_bindings row"); + + cleanup(path); + } + + /// Full sequence: `initialize_instance_in_position_file` (tool=opencode, no + /// session_id) followed by `bind_session_to_process` — mirrors what happens + /// in the --agent path after the fix. + #[test] + fn test_agent_mode_initialize_then_bind_session() { + crate::config::Config::init(); + let (db, path) = setup_test_db(); + + // Step 1: launcher creates the placeholder row (no session_id in --agent mode). + initialize_instance_in_position_file( + &db, + "agent-gamma", + None, // no session_id yet + None, None, None, None, + Some("opencode"), + false, None, None, None, None, None, + ); + db.set_process_binding("proc-agent-3", "", "agent-gamma").unwrap(); + + // Row exists, no session yet. + let before = db.get_instance_full("agent-gamma").unwrap().unwrap(); + assert!(before.session_id.is_none(), "session_id should be absent before binding"); + + // Step 2: plugin calls bind on the first chat.message. + let result = bind_session_to_process(&db, "sess-agent-3", Some("proc-agent-3")); + assert_eq!(result, Some("agent-gamma".to_string())); + + // session_bindings row created. + assert_eq!( + db.get_session_binding("sess-agent-3").unwrap(), + Some("agent-gamma".to_string()) + ); + + // Instance carries session_id. + let after = db.get_instance_full("agent-gamma").unwrap().unwrap(); + assert_eq!(after.session_id.as_deref(), Some("sess-agent-3")); + + cleanup(path); + } + + /// When `HCOM_PROCESS_ID` is absent (no process binding), `bind_session_to_process` + /// with no process_id must return `None` and must NOT insert a phantom + /// `session_bindings` row — mirroring the daemon-absent / headless error path + /// where `opencode-start` returns `{"error": ...}`. + #[test] + fn test_agent_mode_no_process_id_no_session_binding() { + crate::config::Config::init(); + let (db, path) = setup_test_db(); + + // No instance, no process binding. + let result = bind_session_to_process(&db, "sess-agent-none", None); + assert!(result.is_none(), "must return None when no process binding exists"); + + // No phantom session_bindings row. + let binding = db.get_session_binding("sess-agent-none").unwrap(); + assert!(binding.is_none(), "must not create a phantom session_bindings row"); + + cleanup(path); + } } From 6cc9d881a36a3d8e4ba4893092c44cc239d5eaa4 Mon Sep 17 00:00:00 2001 From: Lennart Kotzur Date: Fri, 1 May 2026 13:12:37 +0200 Subject: [PATCH 3/4] fix(opencode-plugin): guard chat.message agent/model update when unbound 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. --- src/opencode_plugin/hcom.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/opencode_plugin/hcom.ts b/src/opencode_plugin/hcom.ts index 682b5caf..9e0addcb 100644 --- a/src/opencode_plugin/hcom.ts +++ b/src/opencode_plugin/hcom.ts @@ -456,7 +456,16 @@ export const HcomPlugin: Plugin = async ({ client, $ }) => { if (input.sessionID && !instanceName && !bindingPromise) { await bindIdentity(input.sessionID) } - if (isBoundSession(input.sessionID)) { + // 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, + reason: "no binding after bindIdentity attempt — hcom absent or daemon error", + }) + } else if (isBoundSession(input.sessionID)) { if (input.agent) currentAgent = input.agent const resolvedModel = normalizePromptModel(input.model) if (resolvedModel) currentModel = resolvedModel From c02743d6d46aeea14a3c4cacaba07bb477318edf Mon Sep 17 00:00:00 2001 From: Lennart Kotzur Date: Fri, 1 May 2026 14:08:28 +0200 Subject: [PATCH 4/4] fix(opencode-plugin): harden binding guards for manual launches and unbound 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. --- src/instance_binding.rs | 25 +++++++++++++------------ src/opencode_plugin/hcom.ts | 21 ++++++++++++++------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/instance_binding.rs b/src/instance_binding.rs index 7c519744..b9f8d651 100644 --- a/src/instance_binding.rs +++ b/src/instance_binding.rs @@ -1202,15 +1202,16 @@ mod tests { // ── --agent mode regression tests ──────────────────────────────────────── // - // In `--agent` mode OpenCode skips `session.created` and fires `chat.message` - // first. `bindIdentity()` is the only path that calls `opencode-start` and - // creates a `session_bindings` row. The tests below verify the DB-level - // invariants that the plugin fix relies on. + // In `--agent` startup, hcom can observe an OpenCode process before the + // OpenCode session has been bound. `bindIdentity()` is the path that calls + // `opencode-start` and creates a `session_bindings` row once a session ID is + // available. The tests below verify the DB-level invariants that the plugin + // fix relies on. /// Regression: calling `bind_session_to_process` without a prior - /// `session.created` (the --agent path) must still create a `session_bindings` - /// row. Before the fix, `HCOM_LAUNCHED` blocked `bindIdentity` and no row - /// was ever inserted. + /// `session.created` (the observed --agent startup gap) must still create a + /// `session_bindings` row. Before the fix, `HCOM_LAUNCHED` blocked + /// `bindIdentity` and no row was inserted from later session-bearing events. #[test] fn test_agent_mode_bind_session_creates_session_binding() { crate::config::Config::init(); @@ -1228,7 +1229,7 @@ mod tests { db.save_instance_named("agent-alpha", &data).unwrap(); db.set_process_binding("proc-agent-1", "", "agent-alpha").unwrap(); - // The plugin now calls this on the first chat.message (--agent path). + // The plugin now calls this from later session-bearing plugin events. let result = bind_session_to_process(&db, "sess-agent-1", Some("proc-agent-1")); assert_eq!(result, Some("agent-alpha".to_string())); @@ -1280,14 +1281,14 @@ mod tests { } /// Full sequence: `initialize_instance_in_position_file` (tool=opencode, no - /// session_id) followed by `bind_session_to_process` — mirrors what happens - /// in the --agent path after the fix. + /// session_id) followed by `bind_session_to_process` — mirrors the launcher + /// placeholder followed by a later OpenCode session binding. #[test] fn test_agent_mode_initialize_then_bind_session() { crate::config::Config::init(); let (db, path) = setup_test_db(); - // Step 1: launcher creates the placeholder row (no session_id in --agent mode). + // Step 1: launcher creates the placeholder row (no session_id yet). initialize_instance_in_position_file( &db, "agent-gamma", @@ -1302,7 +1303,7 @@ mod tests { let before = db.get_instance_full("agent-gamma").unwrap().unwrap(); assert!(before.session_id.is_none(), "session_id should be absent before binding"); - // Step 2: plugin calls bind on the first chat.message. + // Step 2: plugin calls bind once an OpenCode session ID is available. let result = bind_session_to_process(&db, "sess-agent-3", Some("proc-agent-3")); assert_eq!(result, Some("agent-gamma".to_string())); diff --git a/src/opencode_plugin/hcom.ts b/src/opencode_plugin/hcom.ts index 9e0addcb..2ad38843 100644 --- a/src/opencode_plugin/hcom.ts +++ b/src/opencode_plugin/hcom.ts @@ -292,8 +292,10 @@ export const HcomPlugin: Plugin = async ({ client, $ }) => { async function bindIdentity(sid: string): Promise { 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). + if (!process.env.HCOM_PROCESS_ID) return + + // No HCOM_LAUNCHED guard: launched OpenCode instances can start with only + // a process/PTY binding and acquire an OpenCode session later. // Safe to call repeatedly: $.nothrow() catches failures and bindIdentity's // own early-return (instanceName || bindingPromise) handles duplicates. bindingPromise = (async () => { @@ -456,11 +458,16 @@ export const HcomPlugin: Plugin = async ({ client, $ }) => { if (input.sessionID && !instanceName && !bindingPromise) { await bindIdentity(input.sessionID) } - // 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) { + // Guard: 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, reason: "no binding after bindIdentity attempt — hcom absent or daemon error",