diff --git a/ROADMAP.md b/ROADMAP.md index f7ca923..5bf8947 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -312,6 +312,23 @@ Priority order: P0 = blocks CI/green state, P1 = blocks integration wiring, P2 = 23. **`doctor --output-format json` check-level structure gap** — **done**: `claw doctor --output-format json` now keeps the human-readable `message`/`report` while also emitting structured per-check diagnostics (`name`, `status`, `summary`, `details`, plus typed fields like workspace paths and sandbox fallback data), with regression coverage in `output_format_contract.rs`. 24. **Plugin lifecycle init/shutdown test flakes under workspace-parallel execution** — dogfooding surfaced that `build_runtime_runs_plugin_lifecycle_init_and_shutdown` can fail under `cargo test --workspace` while passing in isolation because sibling tests race on tempdir-backed shell init script paths. This is test brittleness rather than a code-path regression, but it still destabilizes CI confidence and wastes diagnosis cycles. **Action:** isolate temp resources per test robustly (unique dirs + no shared cwd assumptions), audit cleanup timing, and add a regression guard so the plugin lifecycle test remains stable under parallel workspace execution. 26. **Resumed local-command JSON parity gap** — **done**: direct `claw --output-format json` already had structured renderers for `sandbox`, `mcp`, `skills`, `version`, and `init`, but resumed `claw --output-format json --resume /…` paths still fell back to prose because resumed slash dispatch only emitted JSON for `/status`. Resumed `/sandbox`, `/mcp`, `/skills`, `/version`, and `/init` now reuse the same JSON envelopes as their direct CLI counterparts, with regression coverage in `rust/crates/rusty-claude-cli/tests/resume_slash_commands.rs` and `rust/crates/rusty-claude-cli/tests/output_format_contract.rs`. + +41. **Phantom completions root cause: global session store has no per-worktree isolation** — + + **Root cause.** The session store under `~/.local/share/opencode` is global to the host. Every `opencode serve` instance — including the parallel lane workers spawned per worktree — reads and writes the same on-disk session directory. Sessions are keyed only by id and timestamp, not by the workspace they were created in, so there is no structural barrier between a session created in worktree `/tmp/b4-phantom-diag` and one created in `/tmp/b4-omc-flat`. Whichever serve instance picks up a given session id can drive it from whatever CWD that serve happens to be running in. + + **Impact.** Parallel lanes silently cross wires. A lane reports a clean run — file edits, builds, tests — and the orchestrator marks the lane green, but the writes were applied against another worktree's CWD because a sibling `opencode serve` won the session race. The originating worktree shows no diff, the *other* worktree gains unexplained edits, and downstream consumers (clawhip lane events, PR pushes, merge gates) treat the empty originator as a successful no-op. These are the "phantom completions" we keep chasing: success messaging without any landed changes in the lane that claimed them, plus stray edits in unrelated lanes whose own runs never touched those files. Because the report path is happy, retries and recovery recipes never fire, so the lane silently wedges until a human notices the diff is empty. + + **Proposed fix.** Bind every session to its workspace root + branch at creation time and refuse to drive it from any other CWD. + + - At session creation, capture the canonical workspace root (resolved git worktree path) and the active branch and persist them on the session record. + - On every load (`opencode serve`, slash-command resume, lane recovery), validate that the current process CWD matches the persisted workspace root before any tool with side effects (file_ops, bash, git) is allowed to run. Mismatches surface as a typed `WorkspaceMismatch` failure class instead of silently writing to the wrong tree. + - Namespace the on-disk session path under the workspace fingerprint (e.g. `//`) so two parallel `opencode serve` instances physically cannot collide on the same session id. + - Forks inherit the parent's workspace root by default; an explicit re-bind is required to move a session to a new worktree, and that re-bind is itself recorded as a structured event so the orchestrator can audit cross-worktree handoffs. + - Surface a `branch.workspace_mismatch` lane event so clawhip stops counting wrong-CWD writes as lane completions. + + **Status.** A `workspace_root` field has been added to `Session` in `rust/crates/runtime/src/session.rs` (with builder, accessor, JSON + JSONL round-trip, fork inheritance, and given/when/then test coverage in `persists_workspace_root_round_trip_and_forks_inherit_it`). The CWD validation, the namespaced on-disk path, and the `branch.workspace_mismatch` lane event are still outstanding and tracked under this item. + **P3 — Swarm efficiency** 13. Swarm branch-lock protocol — **done**: `branch_lock::detect_branch_lock_collisions()` now detects same-branch/same-scope and nested-module collisions before parallel lanes drift into duplicate implementation 14. Commit provenance / worktree-aware push events — **done**: lane event provenance now includes branch/worktree/superseded/canonical lineage metadata, and manifest persistence de-dupes superseded commit events before downstream consumers render them diff --git a/rust/crates/runtime/src/session.rs b/rust/crates/runtime/src/session.rs index 202db6c..61c0e75 100644 --- a/rust/crates/runtime/src/session.rs +++ b/rust/crates/runtime/src/session.rs @@ -71,6 +71,13 @@ struct SessionPersistence { } /// Persisted conversational state for the runtime and CLI session manager. +/// +/// `workspace_root` binds the session to the worktree it was created in. The +/// global session store under `~/.local/share/opencode` is shared across every +/// `opencode serve` instance, so without an explicit workspace root parallel +/// lanes can race and report success while writes land in the wrong CWD. See +/// ROADMAP.md item 41 (Phantom completions root cause) for the full +/// background. #[derive(Debug, Clone)] pub struct Session { pub version: u32, @@ -80,6 +87,7 @@ pub struct Session { pub messages: Vec, pub compaction: Option, pub fork: Option, + pub workspace_root: Option, persistence: Option, } @@ -92,6 +100,7 @@ impl PartialEq for Session { && self.messages == other.messages && self.compaction == other.compaction && self.fork == other.fork + && self.workspace_root == other.workspace_root } } @@ -141,6 +150,7 @@ impl Session { messages: Vec::new(), compaction: None, fork: None, + workspace_root: None, persistence: None, } } @@ -151,6 +161,22 @@ impl Session { self } + /// Bind this session to the workspace root it was created in. + /// + /// This is the per-worktree counterpart to the global session store and + /// lets downstream tooling reject writes that drift to the wrong CWD when + /// multiple `opencode serve` instances share `~/.local/share/opencode`. + #[must_use] + pub fn with_workspace_root(mut self, workspace_root: impl Into) -> Self { + self.workspace_root = Some(workspace_root.into()); + self + } + + #[must_use] + pub fn workspace_root(&self) -> Option<&Path> { + self.workspace_root.as_deref() + } + #[must_use] pub fn persistence_path(&self) -> Option<&Path> { self.persistence.as_ref().map(|value| value.path.as_path()) @@ -225,6 +251,7 @@ impl Session { parent_session_id: self.session_id.clone(), branch_name: normalize_optional_string(branch_name), }), + workspace_root: self.workspace_root.clone(), persistence: None, } } @@ -262,6 +289,12 @@ impl Session { if let Some(fork) = &self.fork { object.insert("fork".to_string(), fork.to_json()); } + if let Some(workspace_root) = &self.workspace_root { + object.insert( + "workspace_root".to_string(), + JsonValue::String(workspace_root_to_string(workspace_root)?), + ); + } Ok(JsonValue::Object(object)) } @@ -302,6 +335,10 @@ impl Session { .map(SessionCompaction::from_json) .transpose()?; let fork = object.get("fork").map(SessionFork::from_json).transpose()?; + let workspace_root = object + .get("workspace_root") + .and_then(JsonValue::as_str) + .map(PathBuf::from); Ok(Self { version, session_id, @@ -310,6 +347,7 @@ impl Session { messages, compaction, fork, + workspace_root, persistence: None, }) } @@ -322,6 +360,7 @@ impl Session { let mut messages = Vec::new(); let mut compaction = None; let mut fork = None; + let mut workspace_root = None; for (line_number, raw_line) in contents.lines().enumerate() { let line = raw_line.trim(); @@ -356,6 +395,10 @@ impl Session { created_at_ms = Some(required_u64(object, "created_at_ms")?); updated_at_ms = Some(required_u64(object, "updated_at_ms")?); fork = object.get("fork").map(SessionFork::from_json).transpose()?; + workspace_root = object + .get("workspace_root") + .and_then(JsonValue::as_str) + .map(PathBuf::from); } "message" => { let message_value = object.get("message").ok_or_else(|| { @@ -389,6 +432,7 @@ impl Session { messages, compaction, fork, + workspace_root, persistence: None, }) } @@ -449,6 +493,12 @@ impl Session { if let Some(fork) = &self.fork { object.insert("fork".to_string(), fork.to_json()); } + if let Some(workspace_root) = &self.workspace_root { + object.insert( + "workspace_root".to_string(), + JsonValue::String(workspace_root_to_string(workspace_root)?), + ); + } Ok(JsonValue::Object(object)) } @@ -825,6 +875,15 @@ fn i64_from_usize(value: usize, key: &str) -> Result { .map_err(|_| SessionError::Format(format!("{key} out of range for JSON number"))) } +fn workspace_root_to_string(path: &Path) -> Result { + path.to_str().map(ToOwned::to_owned).ok_or_else(|| { + SessionError::Format(format!( + "workspace_root is not valid UTF-8: {}", + path.display() + )) + }) +} + fn normalize_optional_string(value: Option) -> Option { value.and_then(|value| { let trimmed = value.trim(); @@ -1206,6 +1265,29 @@ mod tests { assert!(error.to_string().contains("unsupported block type")); } + #[test] + fn persists_workspace_root_round_trip_and_forks_inherit_it() { + // given + let path = temp_session_path("workspace-root"); + let workspace_root = PathBuf::from("/tmp/b4-phantom-diag"); + let mut session = Session::new().with_workspace_root(workspace_root.clone()); + session + .push_user_text("write to the right cwd") + .expect("user message should append"); + + // when + session + .save_to_path(&path) + .expect("workspace-bound session should save"); + let restored = Session::load_from_path(&path).expect("session should load"); + let forked = restored.fork(Some("phantom-diag".to_string())); + fs::remove_file(&path).expect("temp file should be removable"); + + // then + assert_eq!(restored.workspace_root(), Some(workspace_root.as_path())); + assert_eq!(forked.workspace_root(), Some(workspace_root.as_path())); + } + fn temp_session_path(label: &str) -> PathBuf { let nanos = SystemTime::now() .duration_since(UNIX_EPOCH)