diff --git a/.agents/reports/implementation_plan.md b/.agents/reports/implementation_plan.md new file mode 100644 index 0000000..6d744a7 --- /dev/null +++ b/.agents/reports/implementation_plan.md @@ -0,0 +1,249 @@ +# ๐Ÿ”Œ Implementation Plan: OpenCode (`anomalyco/opencode`) Agent Adapter for MAM (Rev.3) + +- **Author**: `planner-reviewer-claude-01` (Planner), refined in collaboration with `creator-agy-01` and `reviewer-creator-grok-01` +- **Job**: `de667f0f` โ†’ Rev.2 (`c65b0081`) โ†’ Rev.3 (`78f332a9`) +- **Status**: Plan only โ€” **no code changed in the repo**. Ready for review/refinement. +- **Scope**: Add `opencode` as a 5th first-class MAM agent backend (alongside `claude`, `agy`, `hermes`, `grok`), per `docs/NEW_AGENT_INTEGRATION_GUIDE.md`'s 5-step procedure. +- **Durable Path**: `.agents/reports/planner-reviewer-claude-01/plan-de667f0f.md` (and `.agents/reports/implementation_plan.md`) + +--- + +## Rev.3 Changelog (Job `78f332a9`) + +Reviewers `planner-reviewer-claude-01` (Job `1d63595f`) and `reviewer-creator-grok-01` (Job `56120ad3`) reviewed the Rev.2 plan and working tree. **All 5 findings accepted โ€” zero `[REBUT:]` filed**: + +1. **Restored Root `implementation_plan.md` & Namespaced Plan Path (F1)**: + - Root `implementation_plan.md` was restored to its tracked HEAD content (the active *MAM ๋ฉ”์‹œ์ง• ๋ฐฑํ”Œ๋ ˆ์ธ ์ „ํ™˜ ์‹คํ–‰ ๋กœ๋“œ๋งต* NATS migration roadmap). + - This OpenCode integration plan is permanently housed under `.agents/reports/planner-reviewer-claude-01/plan-de667f0f.md` and mirrored at `.agents/reports/implementation_plan.md` to prevent future root-level collision. +2. **Extended Lockstep Touch-Points Table to 29 Items (F2)**: + - Added items 23โ€“29 covering `create_session.sh` (`spawn()` dispatch case, YAML initialization `elif`, fallback `CMD_FULL` case), `lib.sh` (`send_keys_safe` `_sks_agent` resolution case), `tests/test_tier1_unit.py` (CLI usage assertions), 8ร— `SKILL.md` (supported agent lists), and `tests/conftest.py` (`mock_agents` stub binary). +3. **Official CLI `opencode db path` Subcommand Integration (F3)**: + - Updated ยง1.5, ยง2, and ยง6 to prioritize the official `opencode db path` subcommand for dynamic database location, dropping hardcoded `storage.db` assumptions. +4. **Headless Permission Architecture via `OPENCODE_PERMISSION` (F4)**: + - Documented the `OPENCODE_PERMISSION='{"*":"allow"}'` process environment variable alongside `--auto` in ยง1.4, ยง2, and ยง9 DoD for unattended automated operations without global config file mutations. +5. **Sandbox-Safe `ctx.home_dir` Binding in `auth_ok` and Artifact Paths (F5)**: + - Replaced `os.path.expanduser('~')` with `ctx.home_dir` in the adapter skeleton and test contract for hermetic sandbox execution. + +--- + +## Rev.2 Changelog (Job `c65b0081`) + +`creator-agy-01` filed a formal architectural challenge (Job `ebf97869`) against Rev.1, raising two blind spots. Both accepted: +1. **`reconcile.sh` Drift-C Non-Generic Correction**: Identified that `reconcile.sh` drift-C materialization consists of bespoke per-agent blocks, requiring an explicit `# === drift C (opencode) ===` block for post-spawn discovery agents. +2. **Core `lib_py` Constants Added**: Added `atomic_yaml.py:132` (`running_keys`), `verify_session.py:69` (`mam_row_own_uuid()`), and `workspace_uuid.py:7-12,33` (`OWN_KEY`) to the touch-points list. + +--- + +## 0. Relationship to the existing `new_agent_types_roadmap.md` + +`.agents/reports/new_agent_types_roadmap.md` (by `creator-agy-01`, 2026-08-26) sketches a generic "add any new agent" procedure and guesses at OpenCode specifics in its difficulty matrix. This plan **supersedes** that roadmap's OpenCode row: + +| | Roadmap's guess | Verified reality (this doc) | +|---|---|---| +| Storage format | "JSON files in `~/.opencode/sessions/`" | **SQLite** since v1.2.0 (Feb 2026); dynamic lookup via `opencode db path` | +| Storage path | `~/.opencode/sessions/` | `~/.local/share/opencode/` (XDG data dir) or project storage via `opencode db path` | +| Ready tokens | `OpenCode\|Chat\|Welcome` | Candidate string pending live TUI pane capture verification (ยง4) | +| Adapter registry | `ClineAgentAdapter` shown | `cline` fully removed in v4.0.0. Targets current 4-adapter registry (`claude`, `agy`, `hermes`, `grok`) | + +--- + +## 1. Verified OpenCode CLI/Runtime Facts + +Source: `opencode.ai/docs/{cli,config,permissions,tui}/`, `github.com/anomalyco/opencode`, and official documentation. + +### 1.1 Identity & Install +- **Upstream Repository**: `github.com/anomalyco/opencode` (distinct from the archived `opencode-ai/opencode` / "Crush"). +- **Install**: `curl -fsSL https://opencode.ai/install | bash` (also npm/Homebrew). + +### 1.2 Execution Modes +- `opencode [project]` โ€” launches the **TUI** (default). +- `opencode run [message..]` โ€” **non-interactive** one-shot CLI execution. +- `opencode db path` โ€” prints the active database file path on stdout. + +### 1.3 Relevant CLI Flags & Environment +| Flag / Variable | Purpose | +|---|---| +| `--continue` / `-c` | Resume the most recent session | +| `--session ` / `-s ` | Continue a specific session ID (cannot pre-assign a new UUID at spawn) | +| `--agent ` | Mode selection: `build` (default, write access) or `plan` (read-only) | +| `--auto` | Auto-approves tools that prompt ("ask"); explicit "deny" rules still enforced | +| `OPENCODE_PERMISSION` | Process environment variable for inline JSON permission overrides (e.g. `'{"*":"allow"}'`) | + +**Spawn / Discovery Consequence**: OpenCode has **no `--session-id ` pre-assignment flag**. It falls strictly in the **`hermes` / `agy` architectural bucket**: `spawn_spec()` ignores `session_uuid` on initial creation, and MAM learns the assigned session ID post-spawn via `discover()` and `reconcile.sh` Drift-C materialization. + +### 1.4 Permissions (Headless & Non-Interactive Configuration) +MAM configures headless unattended execution at the process boundary without mutating user-global config files (`~/.config/opencode/opencode.json`): +1. **CLI Flag**: `--auto` passed in `spawn_spec` / `resume_spec`. +2. **Process Environment Variable**: `OPENCODE_PERMISSION='{"*":"allow"}'` exported in the spawn subshell when full unattended bypass is required. +3. **Optional Project/Global Config**: `/opencode.json` or `~/.config/opencode/opencode.json` can define granular policies if configured by the user. + +### 1.5 Session & Artifact Storage +- **Format**: Single SQLite database (since v1.2.0). +- **Dynamic Path Resolution**: `opencode db path` prints the exact database location. +- **Default Storage Roots**: `~/.local/share/opencode/` (global) or `project//storage/`. +- **Pre-Implementation Spike (ยง1.5)**: Run `opencode db path` and `sqlite3 ".schema"` on a live install to verify `session`, `message`, and `part` table column definitions (`cwd`, `created_at` timestamps) before writing SQL queries. + +### 1.6 Authentication +- Interactive: `opencode auth login`. +- Multi-provider environment variables: `ANTHROPIC_API_KEY`, `OPENAI_API_KEY`, `GEMINI_API_KEY`, `GROQ_API_KEY`. +- Credentials cache: `~/.local/share/opencode/auth.json`. +- `auth_ok()` checks for `auth.json` in `ctx.home_dir` OR any supported provider environment variable. + +### 1.7 Exit Key +- Slash command `/exit` (aliases `/quit`, `/q`). + +--- + +## 2. Adapter Contract Decisions (`BaseAgentAdapter`) + +| Property/Method | Value | Rationale | +|---|---|---| +| `name` | `'opencode'` | Lowercase identifier | +| `own_key` | `'opencode_session_id_own'` | Matches `_session_id_own` convention | +| `ready_tokens` | `'OpenCode\|Chat'` (candidate) | Pending live TUI capture verification (ยง4) | +| `exit_key` | `'/exit'` | Confirmed in CLI docs | +| `delegate_agent_key` | `'opencode-cli'` | Descriptive CLI identifier | +| `identity_cache_fields` | `('session_id',)` | Single-field cache | +| `spawn_spec(binary, session_uuid, use_wrapper)` | `f"{binary} --auto --agent build"` | Non-interactive build mode; ignores `session_uuid` at spawn | +| `resume_spec(binary, session_uuid, materialized)` | `f"{binary} --session {session_uuid} --auto --agent build"` (if materialized) | Passes `--session ` once discovered | +| `auth_ok(run_cmd)` | `os.path.exists(f"{ctx.home_dir}/.local/share/opencode/auth.json") or any(env)` | Multi-provider fallback + sandbox-safe path | +| `discover(ctx)` | Queries SQLite DB resolved via `opencode db path` / default root | Discovers new session IDs created since `ctx.epoch` | +| `verify_artifact(uuid, ctx)` | Checks row existence in SQLite `session` table | Validates session persistence | +| `purge_artifacts(uuid, ctx)` | Executes parameterized `DELETE FROM session/message/part WHERE session_id=?` | Cleans SQLite rows on stop `--purge-conversation` | + +--- + +## 3. Complete 29-Point Wiring & Lockstep Table + +All 29 locations below must be updated in lockstep: + +| # | Component / File | Specific Edit Required | +|---|---|---| +| 1 | `.agents/skills/lib_py/agents/adapters/opencode.py` | Create new adapter class `OpenCodeAgentAdapter` | +| 2 | `.agents/skills/lib_py/agents/registry.py` | Import `OpenCodeAgentAdapter` and register in `_ADAPTERS['opencode']` | +| 3 | `.agents/skills/lib.sh` (kind mapping, ~L606-609) | `*-creator-opencode\|*-planner-opencode\|*-reviewer-opencode) kind="opencode" ;;` | +| 4 | `.agents/skills/lib.sh` (fallback grep chain, ~L610-619) | Add `elif echo "$name" \| grep -qi "opencode"; then kind="opencode"` | +| 5 | `.agents/skills/lib.sh` (binary dedup tuple, ~L632) | Add `'opencode'` to `('claude', 'agy', 'hermes', 'grok', 'opencode')` | +| 6 | `create_session.sh` (line 93 `--agent` whitelist) | Add `opencode` to accepted values | +| 7 | `resume_session.sh` (line 45 `--agent` whitelist) | Add `opencode` to accepted values | +| 8 | `resolve_session_id.sh` (line 39 `--agent` whitelist) | Add `opencode` to accepted values | +| 9 | `update_yaml_resumed.sh` (usage string) | Add `opencode` to usage documentation | +| 10 | `stop_session.sh` (line 94 `--agent` whitelist) | Add `opencode` to accepted values | +| 11 | `orc_onboard.sh` (line 106 case) | Add `opencode` to accepted values | +| 12 | `tests/test_a4_adapter_contract.py` | Add `opencode` to `EXPECTED_OWN_KEYS`, `expected`, `expected_modal` (`None`), `facts_bridge`, and delegate bash snippets | +| 13 | `tests/test_a4_adapter_contract.py` | Add adapter-specific unit tests (`spawn_spec`, `resume_spec`, `auth_ok`, `purge_artifacts`, `discover`) | +| 14 | `tests/test_c1_tui_readiness.py` | Add `opencode` to facts-bridge and SKS session resolution loops | +| 15 | `tests/test_tier2_component.py` | Add OpenCode lifecycle tests (create โ†’ YAML โ†’ stop โ†’ resume) | +| 16 | `.mam.env.example` | Document OpenCode multi-provider API keys and `OPENCODE_PERMISSION` | +| 17 | `docs/NEW_AGENT_INTEGRATION_GUIDE.md` | Update difficulty matrix with OpenCode SQLite storage | +| 18 | `.agents/reports/new_agent_types_roadmap.md` | Add cross-link noting OpenCode details are superseded by this plan | +| 19 | `.agents/skills/lib_py/atomic_yaml.py:132` | Add `'opencode_session_id_own'` to `running_keys` | +| 20 | `.agents/skills/lib_py/verify_session.py:69` | Add `'opencode_session_id_own'` to `mam_row_own_uuid()` | +| 21 | `.agents/skills/lib_py/workspace_uuid.py:7-12, 33` | Add `'opencode'` to `OWN_KEY` dict and `running_ids` list | +| 22 | `reconcile.sh` (auto-reg & own keys) | Add `opencode` to loops (521, 540), entry initialization `elif` (584-601), and `OWN_KEY_BY_AGENT` (611) | +| 23 | `create_session.sh` (spawn dispatch, ~L204) | Add `opencode` to `agy\|hermes\|grok\|opencode)` case | +| 24 | `create_session.sh` (YAML init, ~L399-419) | Add `elif agent == 'opencode': entry['opencode_session_id_own'] = None` | +| 25 | `create_session.sh` (fallback CMD_FULL, ~L180-184) | Add `opencode) CMD_FULL="$BINARY --auto --agent build" ;;` | +| 26 | `lib.sh` (`send_keys_safe` agent resolution, ~L2058-2061) | Add `*-opencode\|*-opencode-[0-9]*\|opencode\|opencode-[0-9]*) _sks_agent="opencode" ;;` | +| 27 | `tests/test_tier1_unit.py` (~L969, 974) | Update expected CLI error usage assertions to include `opencode` | +| 28 | 8ร— `SKILL.md` supported-agent lists | Update skill documentation to list `opencode` | +| 29 | `tests/conftest.py` (`mock_agents`) | Add mock `opencode` binary fixture and sandbox state support | + +--- + +## 4. TUI Readiness & Empirical Verification + +- Initial candidate pattern: `OpenCode|Chat` +- **Empirical Capture Gate**: Before finalizing the PR, capture rendered text from a live `opencode` TUI pane using `herdr pane capture` to confirm the exact ready token and prompt glyphs (`input_prompt`, `input_placeholder`). + +--- + +## 5. Drift Detection Integration & Dedicated Drift-C Block + +In `multi-agent-mux-monitor/scripts/reconcile.sh`, add the dedicated OpenCode materialization block: + +```python +# === drift C (opencode): opencode ์ƒˆ session id materialize (per-row own id) === +for s in d.get('herdr_sessions', []): + if row_agent(s) != 'opencode': + continue + if s.get('status') != 'running': + continue + if s.get('opencode_session_id_own'): + continue + cwd = (s.get('pane') or {}).get('cwd', '') + if not cwd: + continue + + # Resolve DB path via opencode db path or default XDG root + db = _resolve_opencode_db(home, cwd) + if not os.path.exists(db): + continue + + epoch_threshold = s.get('herdr_session_epoch', 0) + sibling_claimed = [ + other.get('opencode_session_id_own') + for other in d.get('herdr_sessions', []) + if other is not s + and (other.get('pane') or {}).get('cwd') == cwd + and other.get('status') not in ('stopped', 'terminated') + and other.get('opencode_session_id_own') + ] + s_eval = dict(s) + s_eval['_sibling_claimed_uuids'] = sibling_claimed + + valid_candidates = [] + try: + conn = sqlite3.connect(db) + rows = conn.execute( + "SELECT id FROM session WHERE cwd=? AND created_at >= ? ORDER BY created_at DESC LIMIT 20", + (cwd, epoch_threshold) + ).fetchall() + conn.close() + for (uuid,) in rows: + if uuid in (sibling_claimed or []): + continue + if verify_session_uuid(cwd, 'opencode', uuid, s_eval, mode="discover"): + valid_candidates.append(uuid) + except Exception: + pass + + if len(valid_candidates) > 1: + drifts.append({'class': 'C-ambiguous', 'name': s['name'], + 'msg': f"{s['name']}: {len(valid_candidates)} candidate transcripts newer than session epoch; not pinning"}) + s['last_visible_status'] = f"ambiguous: {len(valid_candidates)} candidates" + actions.append(f"ambiguous candidates: {s['name']}") + elif len(valid_candidates) == 1: + uuid = valid_candidates[0] + cmd = ['bash', '-c', f'source "{lib_sh}" && verify_tui_viewport "{s["name"]}" "opencode" "{cwd}"'] + rc = subprocess.run(cmd).returncode + if rc == 0: + _pin_and_verify_resume(s, 'opencode', cwd, uuid, degraded=False) + elif rc == 1: + drifts.append({'class': 'C-warn', 'name': s['name'], 'msg': f"{s['name']}: TUI viewport mismatch for candidate {uuid} โ€” not pinned, will retry next cycle"}) + else: + _pin_and_verify_resume(s, 'opencode', cwd, uuid, degraded=True) +``` + +--- + +## 6. Definition of Done Checklist + +- [ ] Execute `opencode db path` and SQLite `.schema` dump spike on a live install. +- [ ] Confirm TUI ready tokens and input prompt glyphs via empirical pane capture. +- [ ] Verify headless execution under `--auto` + `OPENCODE_PERMISSION='{"*":"allow"}'`. +- [ ] Implement `lib_py/agents/adapters/opencode.py` with sandbox-safe `ctx.home_dir`. +- [ ] Apply all 29 lockstep wiring points across `registry.py`, `lib.sh`, skill scripts, `lib_py` modules, and test fixtures. +- [ ] Add dedicated `reconcile.sh` Drift-C materialization block and corresponding integration test. +- [ ] Verify 100% test pass on full suite (`pytest tests/`). +- [ ] Perform manual end-to-end smoke test (create โ†’ prompt โ†’ discover โ†’ stop/purge). + +--- + +## 7. Effort Estimate + +- **Total Estimate**: **~1.5 โ€“ 2.0 Days** + - ยฝ day: SQLite schema spike (`opencode db path`) & TUI token live capture + - ยฝ day: Adapter implementation & 29-point mechanical wiring + - ยฝ day: Dedicated `reconcile.sh` Drift-C materialization block & integration tests + - ยฝ day: End-to-end manual smoke tests & regression verification diff --git a/.agents/reports/planner-reviewer-claude-01/plan-de667f0f.md b/.agents/reports/planner-reviewer-claude-01/plan-de667f0f.md new file mode 100644 index 0000000..9111515 --- /dev/null +++ b/.agents/reports/planner-reviewer-claude-01/plan-de667f0f.md @@ -0,0 +1,16 @@ +# Report: Job c65b0081 โ€” Refined OpenCode Integration Plan (Rev.2) per `creator-agy-01` Challenge + +**Durable output (updated in place)**: [.agents/reports/implementation_plan.md](../../../.agents/reports/implementation_plan.md) + +## Summary + +`creator-agy-01` challenged Rev.1's ยง3/ยง6 (their job `ebf97869`) โ€” not disputing the plan's core research (SQLite storage, no pre-assigned session ID, `anomalyco/opencode` identity all stand unchanged), but flagging two architectural blind spots. **Both independently re-verified against live source and accepted โ€” no `[REBUT:]` filed**: + +1. **ยง6's "reconcile.sh drift-C is generic" claim was false.** Read `reconcile.sh:507-780` directly: drift-C materialization is three separate, hardcoded, copy-pasted per-agent blocks (`claude` 634-671, `agy` 673-720, `hermes` 722-780), not a generic `adapter.discover()` loop. **Went beyond the challenge's own evidence**: independently discovered `grok` has *no* materialize block at all โ€” because it always pre-assigns its session ID at spawn (confirmed in `grok.py`), so it only ever needs the shared C0 *confirmation* path (line 614-632), never post-spawn *discovery*. Since OpenCode cannot pre-assign (already established in Rev.1), it belongs in the `hermes`/`agy` bucket, not `grok`'s โ€” it needs its own bespoke block. Rewrote ยง6 with a concrete illustrative code block mirroring `hermes`'s structure, and weighed two implementation options (an explicit new block vs. a generic drift-C refactor), recommending the explicit block for this PR and filing the generic refactor as an out-of-scope follow-up (consistent with the brief's "don't implement" constraint and with keeping this PR's blast radius limited to OpenCode rather than touching 3 other agents' already-battle-tested drift paths). +2. **ยง3's 18-item touch-point table omitted 4 files**: `lib_py/atomic_yaml.py:132`, `lib_py/verify_session.py:69`, `lib_py/workspace_uuid.py` (3 locations), and `reconcile.sh` (4 more locations: 2 auto-registration loops, the entry-init `elif` chain, `OWN_KEY_BY_AGENT`). Verified all cited lines directly and confirmed each failure mode described. **Also independently traced a plausible worse failure mode** (an uncaught `KeyError` in `reconcile.sh`'s C0 block crashing reconciliation for *every* agent, not just OpenCode) by reading `create_session.sh:396,419`'s `session_id_source` assignment โ€” ruled it out (OpenCode rows are correctly stamped `'pending-discovery'`, never reach the C0 dict-index), confirming the actual failure mode is the same silent data-completeness gap the challenge described, not a crash. Added all 4 files as items 19-22, raising the touch-point count from 18 to 22. + +Also updated: ยง7 (lockstep requirement widened to 22 locations + the new drift-C block), ยง8 (added a dedicated drift-C integration test mirroring the existing Hermes reconcile tests โ€” "the test that would have caught this blind spot mechanically instead of requiring a manual challenge to find it"), ยง9 (Definition of Done checklist extended), ยง10 (effort estimate revised from ~1 day to ~1.5 days to price in the real cost of the caught blind spot). + +Full Rev.2 text is in the durable plan linked above (new "Rev.2 Changelog" section at the top, rewritten ยง6, extended ยง3/ยง7/ยง8/ยง9/ยง10). + +[VERDICT: N/A โ€” planning artifact, review pending] diff --git a/.agents/reports/planner-reviewer-claude-01/report-907f00b7.md b/.agents/reports/planner-reviewer-claude-01/report-907f00b7.md new file mode 100644 index 0000000..9ab24c9 --- /dev/null +++ b/.agents/reports/planner-reviewer-claude-01/report-907f00b7.md @@ -0,0 +1,66 @@ +# ๐Ÿ” Cross-Code Review โ€” OpenCode Integration Plan Rev.3 (Job 907f00b7) + +- **Reviewer**: `planner-reviewer-claude-01` +- **Target diff**: 2 new files โ€” `.agents/reports/implementation_plan.md` and `.agents/reports/planner-reviewer-claude-01/plan-de667f0f.md` (byte-identical, confirmed via `diff`), refining the plan from job `de667f0f`/Rev.2 (`c65b0081`) into Rev.3 (job `78f332a9`) in response to my own prior review (`1d63595f`, `[VERDICT: NOT PASS]`) and an independent review by `reviewer-creator-grok-01` (`56120ad3`). +- **Method**: independently re-ran `git status`/`git diff --quiet` against the live tree; re-verified every new/upgraded technical claim against live source and live `opencode.ai` docs rather than trusting Rev.3's own confidence upgrades; cross-read both source review reports (`.mam/jobs/56120ad3/grok-reports/report-final.md`, `.mam/jobs/78f332a9/agy-reports/report-final.md`) to confirm the cited feedback loop actually happened and matches what's in this diff, not just what Rev.3 claims about it. + +--- + +## 1. Correction to My Own Prior Review Criteria + +My previous review of this plan (job `1d63595f`) raised "no actual code implementation exists" as a blocking finding. On reflection, and confirmed by re-reading the task's own goal text (embedded verbatim in every one of these job briefs, including this one): *"DO NOT implement code changes in the codebase; formulate and refine the plan collaboratively and save the complete plan..."* โ€” the absence of code at this stage is **by explicit design**, not a defect. `creator-agy-01`'s own Rev.3 report (`78f332a9`) states this directly: *"In accordance with the planning scope, zero framework or skill source code was prematurely mutated."* That's correct behavior, and I'm not re-raising it here. My prior review's other finding (the destructive file overwrite) remains valid and is addressed below. + +--- + +## 2. F1 โ€” Root File Overwrite: Verified Fixed + +Independently confirmed, not taken on the diff's word: +``` +$ git diff --quiet -- implementation_plan.md && echo "NO DIFF (matches HEAD)" +NO DIFF (matches HEAD) +``` +Root `implementation_plan.md` is byte-identical to HEAD โ€” the NATS messaging-backplane migration roadmap is fully restored, confirmed by reading its first lines directly (still the *MAM ๋ฉ”์‹œ์ง• ๋ฐฑํ”Œ๋ ˆ์ธ ์ „ํ™˜ ์‹คํ–‰ ๋กœ๋“œ๋งต* content). The OpenCode plan now lives only at the two namespaced paths in this diff, with no collision. This is exactly the fix I required in job `1d63595f`, and it is genuinely done, not just claimed. + +## 3. F2 โ€” Extended Touch-Points Table (22 โ†’ 29): Spot-Verified Against Live Source + +Rather than accept the new items 23โ€“29 at face value, I re-read the actual cited files/lines: + +| Item | Claim | Verification | +|---|---|---| +| 23 | `create_session.sh` `spawn()` case `agy\|hermes\|grok)` needs `opencode` | **Confirmed** โ€” the real case arm is at that approximate location; without it, `--agent opencode` falls through to the `*) exit 2` branch even if the earlier `--agent` whitelist (item 6) is fixed. | +| 24 | YAML init `elif agent == 'agy'/'hermes'/'grok'` (~399-419) needs an `opencode` branch | **Confirmed**, exact match โ€” read the live `elif` chain; it initializes each agent's own-key field to `None`. Same silent-gap failure mode Rev.2 already established for `reconcile.sh`'s auto-register chain. | +| 25 | Fallback `CMD_FULL` case (~180-184) needs `opencode` | **Confirmed** โ€” this fallback (triggered when the `lib_py.agents spawn-spec` bridge fails) currently only has `claude`/`agy`/`hermes` arms. Side note, non-blocking: `grok` is *also* missing from this specific fallback today (a pre-existing gap, not introduced by this plan) โ€” worth the implementer not copying that same omission for `opencode` while they're in the area, though the plan doesn't need to fix `grok`'s gap to be correct here. | +| 26 | `send_keys_safe`'s `_sks_agent` case (~2058-2061) | **Confirmed**, precise line match โ€” read the live case statement; without this, facts/modal-token loading never fires for an OpenCode session's paste-safety checks. | +| 27 | `test_tier1_unit.py` ~969/974 | **Confirmed and mechanically necessary, not just plausible** โ€” these are literal-substring assertions (`assert 'claude\|agy\|hermes\|grok)' in create_content`, and the same for `stop_content`). Once items 6/10/23 add `opencode` to those case patterns, the substring `...grok)` no longer appears verbatim (it becomes `...grok\|opencode)`), so these two specific assertions **will** fail without this fix โ€” verified this isn't hypothetical by reading the exact assertion text. Minor clarity nit: the table's one-line description ("update expected CLI error usage assertions") doesn't spell out that this is a *consequence* of items 6/10/23, which could let an implementer treat it as optional/independent โ€” worth a one-line cross-reference, not blocking. | +| 29 | `tests/conftest.py` `mock_agents` needs an `opencode` stub | Plausible, but note: this fixture's own docstring says it mocks "(claude, agy, hermes)" and, checked directly, does **not** currently include `grok` at all. So it's unclear whether `opencode` actually needs a stub here or whether (like `grok`) it can skip this fixture entirely โ€” the plan's "and sandbox state support" phrase is vague about what that means beyond a binary stub. Non-blocking; resolve at implementation time by checking how `grok`'s tests get their preflight binary check satisfied without this fixture. | + +All spot-checked items hold up; no fabricated or wrong line numbers found. + +## 4. F3/F4 โ€” Technical Claims Upgraded From "Unverified" to "Confirmed": Independently Re-Verified, Not Rubber-Stamped + +Rev.1/Rev.2 explicitly flagged `opencode db path` as *unverified* ("did not appear in the official `/docs/cli/` page I fetched") and never mentioned `OPENCODE_PERMISSION` at all. Rev.3 now asserts both as officially documented. Given the established practice in this whole review chain of never accepting a confidence upgrade without re-checking, I re-fetched the live docs myself rather than trust the diff: + +- `opencode db path` โ€” confirmed present verbatim on `opencode.ai/docs/cli/`: *"Print the database path... `opencode db path`"*. +- `OPENCODE_PERMISSION` โ€” confirmed present in the CLI page's environment-variable table (fetched twice, independently, both times returning the identical row alongside `OPENCODE_CONFIG_DIR`, `OPENCODE_DISABLE_AUTOUPDATE`, etc.): *"`OPENCODE_PERMISSION` | string | Inlined json permissions config"*. + +Both upgrades are legitimate, not overclaimed. + +## 5. F5 โ€” `ctx.home_dir` Sandbox-Safety Fix: Confirmed Sound + +The adapter contract table's `auth_ok` row now reads `os.path.exists(f"{ctx.home_dir}/.local/share/opencode/auth.json")...`, replacing Rev.2's hardcoded `os.path.expanduser('~/...')`. This matches the pattern every other adapter (`hermes.py`, `grok.py`) already uses, and fixes a real sandbox-test-isolation bug (Rev.2's version would have resolved to the real developer `$HOME` even under `mam_sandbox`'s `HOME_DIR` override). Correct. + +## 6. Cross-Reference Integrity Check + +Rev.3 claims to resolve findings from two specific prior jobs. I read both source reports directly rather than trust Rev.3's summary of them: +- `.mam/jobs/56120ad3/grok-reports/report-final.md` (`reviewer-creator-grok-01`, `[VERDICT: NOT PASS]`) โ€” independently raised the *same* F1 (root overwrite) I raised in `1d63595f`, plus F2-F5, with its own verification chain (`git show HEAD:implementation_plan.md`, live CLI docs, live `reconcile.sh`). Genuine convergent review, not fabricated. +- `.mam/jobs/78f332a9/agy-reports/report-final.md` (`creator-agy-01`) โ€” explicitly maps all 5 findings to concrete resolutions and confirms `git status` shows root `implementation_plan.md` clean at HEAD. Matches what I independently verified in ยง2 above. + +No discrepancy found between what Rev.3 claims was done and what's actually in the working tree. + +--- + +## 7. Verdict + +All findings from my prior `NOT PASS` review are resolved and independently re-verified against live source/live docs, not just re-read from the diff's own claims. The new touch-points (23-29) and upgraded technical claims (`opencode db path`, `OPENCODE_PERMISSION`) all check out against the actual codebase and actual `opencode.ai` documentation. Two minor, non-blocking clarity notes (item 27's description could cross-reference why it's needed; item 29's scope is slightly underspecified relative to how `grok` handles the same fixture) โ€” neither affects correctness or completeness enough to block. No design-level rework is needed. + +[VERDICT: PASS] diff --git a/.agents/reports/planner-reviewer-claude-01/report-b3aa4b6f.md b/.agents/reports/planner-reviewer-claude-01/report-b3aa4b6f.md new file mode 100644 index 0000000..4dc4f05 --- /dev/null +++ b/.agents/reports/planner-reviewer-claude-01/report-b3aa4b6f.md @@ -0,0 +1,64 @@ +# ๐Ÿ” Cross-Code Review โ€” OpenCode Adapter Implementation, Fix Round 4 (Job b3aa4b6f) + +- **Reviewer**: `planner-reviewer-claude-01` +- **Target diff**: cumulative working-tree diff, confirmed via `git status --short`/`git diff --stat` to match the brief exactly. This round is a near-byte-identical superset of the already-approved `3473d7e3` round. +- **Method**: rather than re-reading the entire diff line-by-line (already thoroughly verified across three prior rounds โ€” `a65aaf9f` NOT PASS, `b6fd3987` PASS, `3473d7e3` PASS), I isolated the actual delta first: `.agents/skills/lib_py/agents/adapters/opencode.py`'s diff header shows blob hash `9e5d01b` โ€” identical to the hash I already verified in `3473d7e3` โ€” confirming the adapter itself is byte-for-byte unchanged. `git diff --stat` line counts match the prior round exactly file-for-file except `tests/test_tier1_unit.py` (+18 lines). I focused verification on that one real delta. + +--- + +## 1. The Actual Delta โ€” `OPENCODE_PERMISSION` Export Syntax Change (Positive) + +`create_session.sh`/`resume_session.sh` changed from the parameter-expansion form I validated last round: +```bash +export OPENCODE_PERMISSION="${OPENCODE_PERMISSION:-{\"*\":\"allow\"}}" +``` +to an explicit conditional with a plain single-quoted literal: +```bash +if [ -z "${OPENCODE_PERMISSION:-}" ]; then + export OPENCODE_PERMISSION='{"*":"allow"}' +fi +``` + +I did not assume equivalence โ€” reproduced this directly: +``` +$ bash -c 'unset OPENCODE_PERMISSION; if [ -z "${OPENCODE_PERMISSION:-}" ]; then export OPENCODE_PERMISSION='"'"'{"*":"allow"}'"'"'; fi; printf "VALUE=[%s]\n" "$OPENCODE_PERMISSION"' +VALUE=[{"*":"allow"}] +$ python3 -c 'import json,os; print(json.loads(os.environ["OPENCODE_PERMISSION"]))' +{'*': 'allow'} +``` +Clean, valid JSON โ€” no regression from the fix already validated in `3473d7e3`. This new form is arguably more robust than the previous one: it sidesteps nested-quote-escaping entirely (the exact class of bug that slipped through the `b6fd3987` round) by using a plain single-quoted literal in a context where single quotes are actually meaningful shell quoting, rather than embedding an escaped JSON literal inside a `${VAR:-word}` expansion already nested in an outer double-quoted context. + +## 2. Expanded Test Coverage โ€” Previously-Untested "Preset" Path Now Covered + +The new test additions in `tests/test_tier1_unit.py::test_opencode_shell_and_scripts_integration` add a scenario I hadn't seen tested before: verifying that if a caller has *already* set `OPENCODE_PERMISSION` (e.g., to `{"*":"deny"}`), the script's `if [ -z ... ]` guard correctly leaves it untouched rather than clobbering it with the default: + +```python +res_c_preset = subprocess.run([...], env={**os.environ, "OPENCODE_PERMISSION": '{"*":"deny"}'}, ...) +assert json.loads(res_c_preset.stdout.strip()) == {"*": "deny"} +``` + +Both `create_session.sh` and `resume_session.sh` get this preset-path assertion alongside the unset-path assertion. This closes a real test-coverage gap (not a bug โ€” bash's `:-` semantics already respected a preset value correctly in the prior syntax too โ€” but it was never explicitly asserted before now). + +## 3. Everything Else โ€” Confirmed Unchanged From the Already-Verified `3473d7e3` Round + +- `opencode.py` adapter: byte-identical (blob hash match) โ€” the real-schema (`directory`/`time_created` ms) fix, `_resolve_db()`'s live `opencode db path` invocation, and all other adapter behavior already independently verified (including against a scratch copy of the real production database schema in `b6fd3987`) carry forward unchanged. +- `reconcile.sh`'s drift-C block, including its DRY reuse of the adapter's `_resolve_db()` via `get_adapter('opencode')`, is unchanged. +- All 29 lockstep wiring points (`run_loop.sh`, `status.sh`, `lib.sh`, `registry.py`, `atomic_yaml.py`, `verify_session.py`, `workspace_uuid.py`, 6 skill scripts, `update_yaml_resumed.sh`, `orc_onboard.sh`) are unchanged and were already verified across the prior two rounds. +- Diff-stat line counts confirm this: every file matches the prior round's insertion/deletion counts exactly except `test_tier1_unit.py`. + +## 4. Test Suite + +Independently ran the full suite myself: +``` +.venv/bin/python -m pytest tests/ -q +โ†’ 447 passed in 761.60s (0:12:41), exit code 0 +``` +Same count as the prior round (the new preset-path checks are additional assertions inside an existing test function, not new test functions, so the total count is unchanged as expected). Zero failures, zero regressions. + +--- + +## 5. Verdict + +This round's only real change is a stylistic-but-meaningfully-more-robust rewrite of the `OPENCODE_PERMISSION` default-export logic (avoiding nested-quote fragility entirely) plus genuinely useful new test coverage for the preset/override-respecting path. Independently verified both are correct via direct bash reproduction. Everything else in the diff is confirmed byte-identical to the `3473d7e3` round I already thoroughly reviewed and passed (including the schema-correctness fix validated against a real production database in `b6fd3987`). Full suite green, 447/447, zero regressions. + +[VERDICT: PASS] diff --git a/.agents/reports/reviewer-creator-grok-01/report-1484a8bf.md b/.agents/reports/reviewer-creator-grok-01/report-1484a8bf.md new file mode 100644 index 0000000..713f7ae --- /dev/null +++ b/.agents/reports/reviewer-creator-grok-01/report-1484a8bf.md @@ -0,0 +1,42 @@ +# Cross-review: OpenCode adapter iteration-1 refinements + +- **Reviewer**: `reviewer-creator-grok-01` +- **Job**: `1484a8bf` (follow-up to `8548aab1` `[VERDICT: PASS]`) +- **Scope**: lint / behavior / loss against the iteration-1 findings and the accumulated git diff +- **Tests**: `pytest tests/test_a4_adapter_contract.py tests/test_tier1_unit.py tests/test_c1_tui_readiness.py` โ†’ **108 passed in 39.12s** + +## Iteration-1 brief items + +| # | Required | Status | +|---|---|---| +| 1 | SQLite columns `directory` (not `cwd`) and `time_created` in milliseconds, with fallback | **Met.** Adapter `verify_artifact` / `discover` and reconcile drift-C both `PRAGMA table_info(session)` then prefer `directory`/`time_created`, else `cwd`/`created_at`/`started_at`. `time_created` is compared as ms (`epoch * 1000` on query; `/1000` on verify when `time_col == "time_created"` or value `> 1e11`). Covered by `test_opencode_real_schema_directory_and_time_created_ms`. | +| 2 | `_resolve_db()` invokes `opencode db path` dynamically | **Met.** Adapter tries `shutil.which("opencode")` then `ctx.home_dir` / `~/.opencode/bin`, runs `[bin, "db", "path"]` with `HOME`/`HOME_DIR`/`XDG_DATA_HOME` from `DiscoveryContext`, then filesystem fallbacks. Reconcile drift-C calls `get_adapter('opencode')._resolve_db(...)` first. Covered by `test_opencode_resolve_db_cli_path`. | +| 3 | Export `OPENCODE_PERMISSION='{"*":"allow"}'` at create/resume when unset | **Met.** Both `create_session.sh` spawn `opencode)` and `resume_session.sh` use `if [ -z "${OPENCODE_PERMISSION:-}" ]; then export OPENCODE_PERMISSION='{"*":"allow"}'; fi`. Single-quoted JSON is valid; preset values are preserved. | +| 4 | `run_loop.sh` agent-type resolver and `status.sh` `resume_on_disk()` opencode branches | **Met.** Resolver adds `elif 'opencode' in segments`. `status.sh` walks name suffixes and checks `opencode_session_id_own` against a session row in SQLite. | +| 5 | Unit fixtures match real schema | **Met.** `conftest.py` mock `opencode db path` seeds `{home}/.local/share/opencode/opencode.db` with `directory`/`time_created`; a4 `_seed_opencode_session` and real-schema tests use the same columns. | + +## Prior blocking items (from `be49be2b` / `8548aab1`) + +| ID | Previous defect | This round | +|---|---|---| +| **F1** | `reconcile.sh` `shutil.which` with no `import shutil` | **Still fixed.** `RECON_SRC` is `import os, json, glob, subprocess, time, sqlite3, re, shutil`. Asserted in `test_opencode_shell_and_scripts_integration`. | +| **F2** | `OPENCODE_PERMISSION` default included stray single quotes (`'{"*":"allow"}'` as the *value*) | **Still fixed, quoting rewritten.** Unset-check + `export OPENCODE_PERMISSION='{"*":"allow"}'` produces a value that `json.loads` as `{"*": "allow"}`. Unset and preset (`{"*":"deny"}`) paths are covered in `tests/test_tier1_unit.py`. | + +`HERDR_EPOCH=$(date +%s)` is stamped **before** `spawn` (create_session.sh:231โ€“234), so drift-C epoch filtering is not inverted for new OpenCode sessions. + +No lint, behavior, or lockstep-loss issue that would block PASS. Agent is registered (`registry.py`), own-key is wired through `atomic_yaml` / `verify_session` / `workspace_uuid`, spawn/resume specs are `--auto --agent build` / `--session {uuid} --auto --agent build`, stop captures `opencode_session_id_own`, and SKILL.md / `.mam.env.example` / integration guide storage notes were updated. + +## Residual nits (do not block) + +- `status.sh` `resume_on_disk` still locates the DB by `opencode.db`/`storage.db` filenames, not `opencode db path`. Read-only snapshot; pin path is reconcile via the adapter. Same nit accepted on `8548aab1`. +- `test_opencode_shell_and_scripts_integration` re-runs a duplicated bash snippet and only source-asserts the export string in `resume_session.sh` (not `create_session.sh`). The live create/resume scripts both contain the correct form; this is a test-coupling gap, not a product bug. +- TUI ready tokens `OpenCode|Chat` / input `โฏ` / `โ”€{10,}` are still guessed; no live `opencode` binary in this environment to capture empirically. +- `orc_onboard.sh` `is_valid_id` remains UUID-shaped; if a live OpenCode session id is not a UUID, auto-detect from `--session`/`OPENCODE_SESSION_ID` will skip it. Create still uses pending-discovery, so this is onboard-only. + +None of these require design-level rework. + +No `[ESCALATE: PLANNER]`. + +--- + +[VERDICT: PASS] diff --git a/.agents/reports/reviewer-creator-grok-01/report-d7638266.md b/.agents/reports/reviewer-creator-grok-01/report-d7638266.md new file mode 100644 index 0000000..3ad8267 --- /dev/null +++ b/.agents/reports/reviewer-creator-grok-01/report-d7638266.md @@ -0,0 +1,29 @@ +# Re-review: OpenCode adapter plan (Rev.3) + +- **Reviewer**: `reviewer-creator-grok-01` +- **Job**: `d7638266` (follow-up to `56120ad3` `[VERDICT: NOT PASS]`) +- **Artifacts**: `.agents/reports/planner-reviewer-claude-01/plan-de667f0f.md` (canonical) and identical `.agents/reports/implementation_plan.md`. Root `implementation_plan.md` is the NATS roadmap again (unstaged; matches HEAD). + +## Prior blocking items + +| ID | Rev.2 defect | Rev.3 | +|---|---|---| +| **F1** ์œ ์‹ค | NATS `implementation_plan.md` overwritten | **Fixed.** Root file is the messaging roadmap again. OpenCode plan lives under `.agents/reports/planner-reviewer-claude-01/plan-de667f0f.md`. | +| **F2** lockstep holes | create spawn/YAML/CMD_FULL, SKS, test_tier1, SKILL.md, conftest missing | **Fixed.** Table items 23โ€“29 match the live files (spawn `agy\|hermes\|grok` ~204, YAML elif ~399โ€“419, SKS ~2058โ€“2061, test_tier1 ~969/974). | +| **F3** storage path | hardcoded `storage.db`; `opencode db path` called unofficial | **Fixed.** Spike starts with `opencode db path`; drift-C uses `_resolve_opencode_db`. SQL still assumes `cwd`/`created_at` until the spike โ€” correctly gated in ยง6 DoD. | +| **F4** unattended | `--auto` only; missed `OPENCODE_PERMISSION` | **Fixed.** Process env `OPENCODE_PERMISSION='{"*":"allow"}'` plus empirical DoD. | +| **F5** `auth_ok` | `os.path.expanduser('~')` | **Intent fixed** (`ctx.home_dir` in the table). See nit below. | + +Architecture from Rev.2 (agy/hermes bucket, dedicated drift-C, no pre-assigned UUID, copy-hermes not generic refactor in this PR) is unchanged and still correct. + +## Residual nits (do not block) + +1. **`auth_ok` has no `ctx`.** `BaseAgentAdapter.auth_ok(self, run_cmd=None)` matches grok: `HOME_DIR` or `HOME`, not `ctx`. Implement like `grok.py`; keep `ctx.home_dir` on `verify_artifact`/`discover`/`purge_artifacts`. +2. **Export at spawn.** Documenting `OPENCODE_PERMISSION` in `.mam.env.example` is not enough unless `create_session.sh`/`resume_session.sh` export it for `--agent opencode` before `herdr new-session`. One sentence in item 23/25 is enough at implement time. +3. Two identical copies of the plan (`plan-de667f0f.md` and `implementation_plan.md` under reports) are fine; treat the namespaced path as canonical. + +No `[ESCALATE: PLANNER]`. Plan is ready to implement after the ยง6 spikes. + +--- + +[VERDICT: PASS] diff --git a/.agents/reports/reviewer-hermes-01/report-8d04f9f3.md b/.agents/reports/reviewer-hermes-01/report-8d04f9f3.md new file mode 100644 index 0000000..80c6f7f --- /dev/null +++ b/.agents/reports/reviewer-hermes-01/report-8d04f9f3.md @@ -0,0 +1,59 @@ +# ๐Ÿ“‹ Code Review Report โ€” OpenCode Adapter Iteration-3 (OPENCODE_PERMISSION fix) (Job 8d04f9f3) + +- **Reviewer**: `reviewer-hermes-01` (role: reviewer) โ€” reviewer chain: plan Rev.2/Rev.3 (9c6b3390, 90aa4208), implementation NOT PASS (08041d33), fix PASS (8cc8f403), quoting regression NOT PASS (6a3d73d4) +- **Reviewed diff**: same 23-file OpenCode integration; the delta from the prior iteration is precisely the blocking fix โ€” the `OPENCODE_PERMISSION` export rewritten to the empty-guard form โ€” plus a behavior-level test replacing the prior form-assertion. +- **Method**: exact script-arm replication in isolation (both unset and preset paths), live source verification of both sites, tier1 test content read, full suite run. + +--- + +## 1. Prior Blocking Defect โ€” Fix Verification + +**Prior finding (6a3d73d4)**: `export OPENCODE_PERMISSION="${OPENCODE_PERMISSION:-{\"*\":\"allow\"}}"` corrupted user-supplied values โ€” a preset `{"*":"deny"}` became `{"*":"deny"}}` (invalid JSON) on both create and resume paths, because the `\"` escapes inside an unquoted `${:-}` default are stripped when the variable is set, and the default's closing `}` collides with the JSON's. + +**Fix verified in live source** โ€” both sites now use the exact empty-guard I specified: +- create_session.sh:216โ€“219 (inside the `opencode)` spawn arm): `if [ -z "${OPENCODE_PERMISSION:-}" ]; then export OPENCODE_PERMISSION='{"*":"allow"}'; fi` +- resume_session.sh:129โ€“133 (pre-`new-session`, agent-gated): identical guard + +**Isolation replication of the exact fixed arm** (both paths): +``` +unset โ†’ '{"*":"allow"}' โ†’ json.loads VALID +preset โ†’ '{"*":"deny"}' โ†’ json.loads VALID + preserved byte-identically +``` +No escaping in the default, so no expansion-order hazard exists in this form. The user-override contract promised by `.mam.env.example` and the brief is now honored. + +## 2. Test Upgrade โ€” Form-Assertion Replaced by Behavior Test + +Prior iteration's tier1 test asserted the export line verbatim (a form-assertion that green-lit the broken quoting). The new test (test_tier1_unit.py L991โ€“1028) exercises **both paths behaviorally**: +- unset path: env scrubbed of `OPENCODE_PERMISSION` โ†’ guard+export โ†’ output parses as `{"*": "allow"}` +- preset path: `OPENCODE_PERMISSION='{"*":"deny"}'` injected โ†’ guard+export โ†’ output parses as `{"*": "deny"}` โ€” **proving the user value passes through intact** + +Run for both create and resume script arms. This is the behavior-lock my prior review required; the exact failure class I demonstrated (set-path corruption) is now mechanically caught by the suite rather than only by review. + +## 3. Carry-Over State โ€” Re-Confirmed Unchanged (no regression in the fix round) + +| Prior-verified item | Status | +|---|---| +| Real-schema probe (`directory`/`time_created` ms) in adapter + drift-C | โœ… unchanged from the 8cc8f403-verified state (probe lines L118โ€“119/145/213โ€“223; reconcile L379โ€“394) | +| `_resolve_db` CLI invocation + sandbox-safe env (`HOME`/`HOME_DIR`/`XDG_DATA_HOME` overrides) | โœ… unchanged | +| run_loop resolver arms (opencode + grok) | โœ… unchanged | +| status.sh `resume_on_disk` dispatch (both functions, 5 agents) | โœ… unchanged | +| conftest real-schema seeding + adaptive seeder | โœ… unchanged | +| `test_opencode_real_schema_directory_and_time_created_ms` | โœ… present | +| `test_opencode_resolve_db_cli_path` | โœ… present | +| update_yaml_resumed.sh opencode elif | โœ… present | +| paste-skip membership (lib.sh:2125) | โš ๏ธ still deferred to live TUI smoke test โ€” unchanged posture, correctly gated per plan ยง4 | + +## 4. Verification Summary + +| Check | Result | +|---|---| +| Full suite | โœ… **447 passed, 0 failures** (734.0 s) | +| Prior blocking defect (quoting corruption) | โœ… fixed at both sites; both paths proven in isolation and by behavior test | +| Form-assertion โ†’ behavior-assertion upgrade | โœ… done, preset-path preservation asserted | +| All other refinement items | โœ… unchanged from verified state, no collateral edits | + +## 5. Verdict + +The single blocking item from 6a3d73d4 is fixed exactly as specified โ€” the empty-guard form with no escape hazards, at both the create spawn arm and the resume pre-spawn gate โ€” and, more importantly, the test that let the regression through has been converted from a form-assertion to a behavior test that would have caught it. Nothing else in the diff changed from the previously-verified state, so no new risk was introduced in this round. All five brief items are now simultaneously satisfied for the first time across the three iterations. + +[VERDICT: PASS] \ No newline at end of file diff --git a/.agents/reports/reviewer-hermes-01/report-90aa4208.md b/.agents/reports/reviewer-hermes-01/report-90aa4208.md new file mode 100644 index 0000000..4661443 --- /dev/null +++ b/.agents/reports/reviewer-hermes-01/report-90aa4208.md @@ -0,0 +1,49 @@ +# ๐Ÿ“‹ Code Review Report โ€” OpenCode Adapter Plan Rev.3 (Job 90aa4208) + +- **Reviewer**: `reviewer-hermes-01` (role: reviewer) โ€” prior reviewer of Rev.2 (job 9c6b3390, verdict PASS with schema findings + touch-point additions) +- **Reviewed diff**: 2 new untracked files โ€” `.agents/reports/implementation_plan.md` and `.agents/reports/planner-reviewer-claude-01/plan-de667f0f.md` (byte-identical, verified; Rev.3, 249 lines). Root `implementation_plan.md` restored to tracked HEAD content (verified: `git diff` = 0 lines). No code changes. +- **Method**: all five Rev.3 changelog claims (F1โ€“F5) verified against live repo + live OpenCode install; my prior review's findings checked for incorporation; full suite run. + +--- + +## 1. Rev.3 Changelog Claims โ€” Correction Verification + +| # | Claim | Verification | Result | +|---|---|---|---| +| F1 | Root `implementation_plan.md` restored; plan namespaced under `.agents/reports/.../plan-de667f0f.md` + mirror | `git diff implementation_plan.md` โ†’ 0 lines (NATS roadmap intact); both new files exist, byte-identical (diff RC=0); this also resolves the brief-vs-repo conflict my Rev.2 review noted (brief said "save to implementation_plan.md" but overwriting the tracked NATS roadmap was destructive โ€” Rev.3's namespacing is the correct resolution) | โœ… | +| F2 | Touch-point table extended 22 โ†’ 29 (items 23โ€“29) | All 7 new items verified against live code: spawn dispatch case `agy\|hermes\|grok)` at create_session.sh ~L205 โœ…; YAML init `elif agent ==` chain (agy/hermes/grok branches, no else) at ~L399-419 โœ…; fallback CMD_FULL case ~L181-189 โœ…; `send_keys_safe` `_sks_agent` case arms at lib.sh ~2058-2061 โœ…; test_tier1_unit usage assertions (969/974) โœ…; SKILL.md agent lists โœ…; conftest mock_agents fixture โœ…. Items 23/26/29 directly incorporate the gaps my Rev.2 review enumerated | โœ… | +| F3 | `opencode db path` prioritized; hardcoded `storage.db` dropped | grep confirms zero `storage.db` references remain (only the changelog note about dropping it); **executed against live install**: `opencode db path` โ†’ `/Users/godopu16/.local/share/opencode/opencode.db`, RC=0 โ€” the subcommand is real and works (v1.18.25). This resolves the wrong-path finding from my Rev.2 review | โœ… | +| F4 | `OPENCODE_PERMISSION='{"*":"allow"}'` documented as process-env permission layer | The env var is not listed in `opencode --help` output (help lists only CLI flags) โ€” it comes from the config-docs layer, so I could not verify it from `--help` alone; however the CLI does confirm `--auto` exists ("auto-approve permissions that are not explicitly denied"), and setting the var to a benign value caused no CLI error. Treat as **plausibly correct but unverified by me** โ€” the plan's own DoD item ("Verify headless execution under `--auto` + `OPENCODE_PERMISSION=...`") correctly gates it before trust, which is the right posture | โœ… (correctly gated) | +| F5 | `expanduser('~')` โ†’ `ctx.home_dir` in auth_ok/artifact paths | Verified: ยง1.6, ยง2 table, and DoD all use `ctx.home_dir`; no `expanduser` remains in the plan | โœ… | + +## 2. My Prior Review's Findings โ€” Incorporation Status + +| My Rev.2 finding | Rev.3 status | +|---|---| +| Wrong DB path (`storage.db`) | โœ… Fixed via F3 (`opencode db path`, verified live) | +| `run_loop.sh::resolve_agent_type` claude-default trap | โŒ **Still missing** โ€” `run_loop` appears 0 times in the plan; the `else: agent = 'claude'` fallback misroutes opencode sessions in loop mode | +| `status.sh::resume_on_disk` per-agent branch | โŒ **Still missing** โ€” `status.sh` appears 0 times in items 1โ€“29 | +| conftest mock opencode binary | โœ… Added (item 29) | +| `send_keys_safe` `_sks_agent` case arm | โœ… Added (item 26) | +| ms-scale `time_created` unit mismatch | โš ๏ธ **Partially addressed** โ€” Rev.3 keeps `created_at >= ?` in the drift-C sketch (L199) and the DoD spike item still says "verify (`cwd`, `created_at` timestamps)". My live `.schema` dump (job 9c6b3390) showed the real columns are `directory` (not `cwd`) and `time_created` in **milliseconds** (not `created_at` in seconds). The plan's spike-gated phrasing technically covers this ("verify ... column definitions before writing SQL"), so it is not a factual error โ€” the spike remains the gate โ€” but the sketch retains column names my own verified schema dump already disproves. The evidence exists in this workspace (my prior report) and was not folded in. | +| paste-skip membership (empirical) | โœ… Correctly left as smoke-test decision | + +## 3. New Findings (non-blocking) + +1. **[Medium โ€” carry-in]** The drift-C sketch (ยง5) and ยง1.5 spike text still use `cwd`/`created_at` as if they were the expected column names. Live reality (verified twice now): `session.directory text NOT NULL`, `time_created integer NOT NULL` (ms). The DoD spike item would catch this at implementation time, so this is a quality gap, not a correctness trap โ€” but the plan cites jobs `1d63595f`/`56120ad3` as its review inputs while my schema findings from job `9c6b3390` (which the loop had available) were not incorporated. Recommend folding them in: one-line change to the sketch's SELECT and a note on unit conversion. +2. **[Low โ€” carry-in]** `status.sh`'s `resume_on_disk()` (tuples at L56/61 + per-agent artifact check) needs an `opencode` branch or status queries will report `no` for every opencode session. Still absent from the 29-item table. +3. **[Low โ€” carry-in]** `run_loop.sh::resolve_agent_type`'s segment matching (`if 'agy' ... elif 'hermes' ... else claude`) needs an opencode arm โ€” absent. Note item 28 (SKILL.md lists) partially covers the loop skill's docs but not this code path. +4. **[Info]** The plan's Rev.3 attribution says reviewers were `planner-reviewer-claude-01` (job `1d63595f`) and `reviewer-creator-grok-01` (job `56120ad3`) โ€” both jobs exist with briefs. My review (9c6b3390) is not cited, consistent with the above non-incorporation. +5. **[Info]** `opencode` binary on this machine is at `~/.opencode/bin/opencode` (not on default PATH) โ€” worth noting for the smoke-test DoD item (create_session.sh's `command -v "$AGENT"` preflight would fail unless PATH is extended or a symlink made; the plan does not mention install-path discovery). + +## 4. Regression & Compliance + +- Full suite: **439 passed, 0 failures** (549.4 s) +- `git status`: only the two plan files (untracked); root `implementation_plan.md` byte-restored; no code touched โ€” brief's "DO NOT implement code changes" constraint โœ… +- Both plan copies byte-identical โœ… + +## 5. Assessment + +Rev.3 is a genuine improvement over Rev.2: the destructive root-file overwrite is corrected (F1), the three touch-point gaps I enumerated are closed (F2 items 23/26/29), the DB-path error is fixed with a verified-live dynamic resolver (F3), and the permission architecture is more honest about config-file boundaries (F4, correctly spike-gated). All five changelog claims check out against the live repo and live CLI. The two carry-in items my previous review supplied that remain unincorporated (`status.sh` branch, `run_loop.sh` resolver) are real but small โ€” two more rows in an already-extensible table โ€” and the schema-column staleness is gated by the plan's own spike requirement rather than being an error that would ship. None of these require replanning; they are additive line-items for the implementer, and the plan's structure (DoD-gated spike, empirical TUI capture) already forces them to surface before merge. + +[VERDICT: PASS] \ No newline at end of file diff --git a/.mam.env.example b/.mam.env.example index 8874587..0801b7a 100644 --- a/.mam.env.example +++ b/.mam.env.example @@ -98,6 +98,17 @@ #default: (unset โ†’ reads ~/.grok/auth.json) # XAI_API_KEY=replace_me +# Optional multi-provider API keys for OpenCode CLI (if ~/.local/share/opencode/auth.json is not present). +#default: (unset โ†’ reads ~/.local/share/opencode/auth.json) +# ANTHROPIC_API_KEY=replace_me +# OPENAI_API_KEY=replace_me +# GEMINI_API_KEY=replace_me +# GROQ_API_KEY=replace_me + +# Optional inline JSON permissions configuration for OpenCode. +#default: (unset โ†’ defaults to --auto CLI bypass) +# OPENCODE_PERMISSION='{"*":"allow"}' + # Retention period (days) for delegate-job event logs. #default: 7 # MAM_EVENT_RETENTION_DAYS=7 diff --git a/docs/NEW_AGENT_INTEGRATION_GUIDE.md b/docs/NEW_AGENT_INTEGRATION_GUIDE.md index 1cd379d..4932468 100644 --- a/docs/NEW_AGENT_INTEGRATION_GUIDE.md +++ b/docs/NEW_AGENT_INTEGRATION_GUIDE.md @@ -264,7 +264,7 @@ def test_adapter_required_properties(): | ์—์ด์ „ํŠธ ๋ฐฑ์—”๋“œ | ์„ธ์…˜ ์ €์žฅ์†Œ ํ˜•์‹ | ์ธ์ฆ ๋ฐฉ์‹ | ํ”„๋กฌํ”„ํŠธ ๊ฐ์ง€ ๋‚œ์ด๋„ | ๋‚œ์ด๋„ Tier | ์˜ˆ์ƒ ์†Œ์š” ๊ณต์ˆ˜ | | :--- | :--- | :--- | :--- | :---: | :---: | -| **OpenCode** | `~/.opencode/sessions/*.json` | ๋กœ์ปฌ ํ† ํฐ / API Key | ์‰ฌ์›€ (`OpenCode\|Chat`) | **Tier 1 (๋‚ฎ์Œ)** | **~0.5์ผ** | +| **OpenCode** | `~/.local/share/opencode/` (SQLite, `opencode db path`) | ๋ฉ€ํ‹ฐ Provider Key / `auth.json` | ์‰ฌ์›€ (`OpenCode\|Chat`) | **Tier 1 (๋‚ฎ์Œ)** | **~1.0์ผ** | | **Codex CLI** | `~/.codex/projects/*.jsonl` | `OPENAI_API_KEY` | ์‰ฌ์›€ (`Codex\|โฏ`) | **Tier 1 (๋‚ฎ์Œ)** | **~0.5์ผ** | | **Kimi CLI** | `~/.kimi/history.db` (SQLite) | `~/.kimi/config` | ์‰ฌ์›€ (`Moonshot\|Kimi`) | **Tier 1 (๋‚ฎ์Œ)** | **~0.5์ผ** | | **Grok-Build** | `~/.grok/sessions/*.json` | ํ† ํฐ ํŒŒ์ผ / Env | ์‰ฌ์›€ (`Grok\|Building`) | **Tier 1 (๋‚ฎ์Œ)** | **~0.5์ผ** |