feat(integrations): live-green baseline and headless fixes [INT-1114] - #485
Open
AlexanderZ-Band wants to merge 60 commits into
Open
feat(integrations): live-green baseline and headless fixes [INT-1114]#485AlexanderZ-Band wants to merge 60 commits into
AlexanderZ-Band wants to merge 60 commits into
Conversation
AlexanderZ-Band
changed the title
AlexanderZ-Band
marked this pull request as ready for review
July 24, 2026 13:29
AlexanderZ-Band
changed the title
Add HttpOpencodeClient transport tests over a real in-process ASGI fake
(httpx.ASGITransport), covering SSE parsing, header/query building, and
every REST call shape; add a `transport` constructor seam to make that
possible. Round out OpencodeAdapter unit tests with the turn-timeout
abort path, Emit.USAGE aggregation (reasoning-into-output folding), the
`always` permission reply, and Capability.MEMORY/CONTACTS tool gating.
Also fix the baseline registry: opencode's SUPPORTED_CAPABILITIES
already advertised {MEMORY, CONTACTS} but its matrix registration passed
supports=(), so the memory/contacts capability-scoped smokes silently
never exercised it live.
Live-testing the OpenCode adapter surfaced a real hang: on_cleanup would block indefinitely stopping the shared Band MCP SSE backend, because uvicorn's Config never set timeout_graceful_shutdown (default None = wait forever for existing connections to close on their own). An MCP client like OpenCode holds its /sse GET open for the life of its own session and may never close it after we deregister, so stop() had no way to unstick itself short of an external timeout forcing it. Set timeout_graceful_shutdown=SERVER_STOP_TIMEOUT_S so stop() force- cancels a still-open connection after a bounded grace period instead.
Live-testing against a real opencode serve surfaced a hard crash every time OpenCode asks for permission or clarification in the (default) manual approval/question mode: every notification/confirmation send_message call in the permission and question lifecycle omitted mentions, and the platform's send_message requires at least one. The crash unwound into the SSE event loop's outer handler, forcing a stream reconnect and stalling that turn until the timeout auto-reject fired minutes later. Unit tests never caught this because FakeAgentTools.send_message (used by all 30 adapter tests) accepts a call with no mentions instead of enforcing the real requirement -- a fake/production drift. Wire mentions through all six call sites (using the current turn's sender for permission/question-asked notifications mid-turn, and the control message's own sender for approve/reject/answer confirmations, since those happen outside any open turn) and add explicit mentions assertions to the existing manual-reply tests so this can't silently regress again.
Both live bugs found while validating the adapter against a real opencode serve lived in the same raw-dict SSE handling: defensive .get() chains hid the permission relay's crash path and made every handler re-derive shapes. Model the wire once instead. New src/band/integrations/opencode/events.py: Pydantic v2 models for the seven consumed event types plus a parse_opencode_event boundary the adapter's event loop calls once per payload. Unknown types and malformed payloads degrade to an ignored UnknownOpencodeEvent (the loop can no longer be crashed by a payload); malformed nested corners degrade alone (lenient wrap validators / container coercion) so the rest of the event still drives the adapter. Each event knows where its session id lives, replacing _room_state_for_event's per-type digging. OpencodeTokens.to_turn_usage and OpencodeErrorInfo.describe absorb _usage_from_info/_format_opencode_error; tool-output *presence* semantics (falsy outputs preserved) move onto OpencodeToolState. The client keeps yielding raw dicts (OpencodeClientProtocol unchanged) so transport tests and the structural fake stay as wire-level tests that now also exercise the parse boundary.
…inery Convert the ~1100-line band/adapters/opencode.py module into a package with the public import path unchanged: config.py (OpencodeAdapterConfig + the mode literals), approvals.py (new RoomApprovals), adapter.py (everything else -- turn lifecycle, session/MCP wiring and prompt building move verbatim; that code is race-hardened and test-pinned). RoomApprovals owns one room's permission/question lifecycle -- pending state, auto-reply modes, the manual room relay and reply parsing, and expiry timeouts -- behind a narrow ApprovalPorts bundle. This is where both live bugs lived; the bundle makes the two mention sources (the open turn's sender for asks, the control message's sender for confirmations) explicit instead of implicit dict plumbing. The pure parse/format helpers become module functions. The config-drift guard's module discovery now also counts package directories, so package-style adapters stay covered by conformance.
…ess with auto_accept
Root cause of the live memory-smoke failures: OpenCode blocks the
session mid-turn on a permission ask, and in a headless room nobody
answers -- the manual-mode default waits 300s then rejects, stalling
and failing the turn. Two asks matter: (a) a tightened server config
can gate the adapter's own band MCP tools, and (b) OpenCode's
doom_loop repetition heuristic (ask by default) trips on exactly the
multi-tool chains the memory smokes drive (store -> list -> get).
Fix (a) in the adapter, for every approval_mode including auto_decline:
a permission ask naming a tool the adapter itself registered (band
platform tools + additional_tools; exact or {server}_{tool}-prefixed
name) is auto-approved with "always" -- the server installs an allow
rule and stops asking. Platform plumbing never stalls on a human,
matching codex, which executes band tools with no approval gate at all.
Non-matches log at debug so OpenCode naming drift stays observable.
Fix (b) where it belongs, in the E2E lane: the baseline builder now
runs opencode with approval_mode="auto_accept" (headless rooms have no
approver; codex baseline parity via approvalPolicy="never"). doom_loop
answers "once" per trip, so the heuristic itself keeps functioning.
The SDK default stays "manual" -- right for interactive rooms, and (a)
guarantees manual never stalls on the adapter's own plumbing.
…ed tool names
Two live findings from the memory smokes, both verified by isolated
probes against a real opencode serve:
1. The band MCP tools' schemas require a room_id argument (the shared
backend dispatches tool calls by room), but the adapter never told the
model the current room id -- making every platform tool uncallable. An
isolated probe proved the model calls band_store_memory perfectly once
the schema is satisfiable. Build the system prompt per turn with the
same Room Context block the ACP client adapter injects (current
room_id + requester), instead of one static prompt at startup.
2. OpenCode registers a remote MCP server's tools as {server}_{tool}
(band_store_memory surfaces as band_band_store_memory, captured live
off the SSE stream). Reported tool_call/tool_result events then carried
that double-prefixed name, so consumers matching the canonical band
vocabulary (e.g. the baseline scorecard's call-layer assertions) never
saw the calls. Normalize reported names back to the canonical tool
name; the same helper backs the permission auto-approval match.
Live result: the store and recall memory smokes (store -> list -> get
through band MCP tools) now pass against a real server.
Address robustness regressions surfaced reviewing the typed-event refactor.
- Never strand a turn on a room-send failure. A sender-less turn (empty
sender_id, e.g. via oneshot) yields no mentions, which the platform
rejects; the manual approval relay then skipped release_turn_wait and
_deliver_fallback_text stranded on_message on its captured
release_future. Route approval posts through a best-effort _notify_room
and hoist release_turn_wait into _watch_turn_completion's finally.
- Never drop a blocking ask. A non-string permission/question scalar
degraded the whole permission.asked/question.asked event to
UnknownOpencodeEvent, deadlocking the session until the turn timeout.
Coerce those scalars (and a text delta) via a coerce_str before-validator.
- Preserve error signal. A bare-string error was swallowed to None
("no reply"); an empty error object spuriously surfaced a generic error.
OpencodeErrorInfo now accepts a scalar error and exposes is_empty, and
_apply_message_update only surfaces a non-empty error.
- SERVER_STOP_TIMEOUT_S: int -> float, matching SERVER_START_TIMEOUT_S.
Regression tests: mention-enforcing tools boundary (the standard fake
accepts mention-less sends, which hid the original crash), blocking-ask
scalar coercion, delta coercion, and error scalar/empty handling.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AlexanderZ-Band
force-pushed
the
feat/intel-opencode-mvp-integration-l0l4-validation-INT-1114
branch
from
July 24, 2026 14:02
f4f00d3 to
8665efb
Compare
A room reply is delivered to an agent only when it @mentions it, so the platform prepends the agent's @handle token to the reply content. The approval parser read tokens[0] as the command, so every real approve/ reject/answer reply parsed the mention as the command, never matched the pending ask, and was silently forwarded as a new prompt -- the manual relay stalled to the approval timeout, then auto-rejected. Verified live: even a clean "approve <id>" (mention in metadata only) arrives as "@handle approve <id>" at on_message. Strip the leading mention block once at try_handle_reply entry via a new strip_leading_mentions() beside replace_uuid_mentions(); the command/id/ answer then parse from clean text. Codex's differently-shaped scanner (finds a /command behind arbitrary prose) is left as-is -- the two do not share an algorithm. Tests across tiers: strip_leading_mentions units; an adapter guard on the real mentioned-reply shape (with mixed-case id preservation); non-matching -id fall-through; notify-room send-failure resilience; a live integration round-trip proving the strip on real platform content; and a bespoke manual-approval e2e smoke (backends lane). send_user_mention lifted into the integration conftest as the single sender. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_extract_command matched /approve|/decline|/approvals|/status only on the first token, but a delivered room reply carries the platform's leading @handle mention block (a room message reaches the agent only when it mentions it), so the command never led the content and every chat-approval reply was silently forwarded to the model instead of handled. Strip the leading mention block first, reusing strip_leading_mentions -- now shared by the opencode and claude_sdk approval paths. Same bug class as the opencode approval fix; found by sweeping the adapters for the same pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… backends lane The smoke now asserts the fix's real guarantee -- a mentioned `approve <id>` reply is recognized (the adapter posts `approval <id> handled with once`, echoing the parsed mixed-case id) -- instead of a free-model-dependent tool echo that made it flaky. Verified live 3x back to back (24/35/28s, no reruns). Two backends-lane setup fixes it depends on (both root-caused live, not flaky): - setup-opencode.sh: gate bash to `ask` so a shell tool use raises a real permission.asked. The default serve auto-allows bash, so the turn never paused and the smoke had nothing to approve. - setup-codex.sh: bump the codex model pin off the API-deprecated gpt-5-codex (every codex turn stream-errored with no tokens) to gpt-5.3-codex (verified). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Four defects found reviewing the OpenCode adapter's turn, approval, and reply logic: - Manual approval charged human-deliberation time to the compute budget: turn_timeout_s (armed right after prompt_async returns) could abort a healthy turn while it was legitimately parked on a human. The watcher now pauses while an ask awaits a human -- bounded by the ask's own expiry -- and grants a fresh budget slice for the post-approval work. - No room-posting suppression: a turn that replied via band_send_message and also emitted text double-posted. A completed room-posting band tool now suppresses the text fallback (codex/copilot_sdk/ACP parity); detection is independent of Emit.EXECUTION. - _own_tool_names was populated only at MCP registration, so a second room's first turn could race an empty set and fail to auto-approve a band tool. It is computed once at construction now. - A transient session-task-event post failure aborted the whole turn before the model ran (silently dropping the user's message). Made best-effort. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The examples defaulted to `minimax-m2.5-free`, not the free model the OpenCode server actually serves. Align the docstring, README table, and settings default with `mimo-v2.5-free` (matches setup-opencode.sh). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Validated live that the platform's /next (Chat.get_next_actionable_message) returns 'processing' messages and excludes only 'processed', so the stop->play replay (which catches up solely through /next) does not drop the stopped message. Behavior was already correct; the docs were not. Reconcile two comments that claimed the opposite and left the codebase self-contradictory on a data-loss-adjacent path: - execution.py: the stale-recovery sweep is not needed because /next skips processing; it drains stuck messages up front instead of one-per-poll. - link.py mark_processing: marking processing does NOT remove a message from /next; only mark_processed does. Add a baseline E2E (test_next_actionable_semantics) guarding the cross-system invariant ExecutionContext._abort_cycle depends on. The unit replay test mocks /next and so covers only the SDK half; this fails loud if the platform ever tightens /next to also exclude 'processing'. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The two test modules lived next to the scripts they cover, under .claude/skills/bug-hunting-via-example/scripts/. testpaths = ["tests"], so CI's bare `uv run pytest` never collected them: 374 lines of tests for subprocess lifecycle, plan validation and secret-env handling that nothing ran, plus assertions pinned to concrete example paths that could rot silently. Move them to tests/skills/bughunting/, matching how every other test for code outside src/band is placed (band-bridge -> tests/bridge, docker/band_python_kit -> tests/docker). The scripts stay where the skill runtime reads them from; a BUG_HUNTING_SCRIPTS anchor in tests/paths.py addresses them, replacing the Path(__file__).parents[4] arithmetic the tests used. A shared conftest loads each script by path once instead of duplicating the importlib fixture, and drops the sys.modules alias on teardown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Repo convention is `from band.x import y` / `from tests.x import y`, never `from .x import y` — the sibling adapter test package already does this (tests/adapters/copilot_sdk). The new OpenCode test package mixed both styles in one file. band-bridge/bridge_core is left alone: it is a separate deployable whose six modules are consistently relative. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…double FakeAgentTools.send_message accepted a mention-less send that the real AgentTools rejects, which is why 30 unit tests never saw the permission-relay crash this branch fixed. Enforce the same requirement in the fake (with the same actionable handles hint) so the whole class of defect fails in unit tests, and drop the local MentionEnforcingTools subclass it made redundant. Cost: three assertions in the fake's own tests, which sent without mentions. Approval and event fixes found by re-reading the flow: - turn_timeout_s is a *compute* budget, but it only excluded human-approval time when the ask was still parked at the instant the deadline fired. An approval that arrived earlier left the resumed work whatever the wait had not consumed — with the 300s defaults, a 4-minute deliberation aborted a healthy turn after 60s. RoomApprovals now owns a human-wait clock and the watcher recomputes the deadline from it each slice. - Pending asks were a single slot per kind, so a second permission.asked dropped the first with no reply: that tool call blocked server-side until the turn timed out, and a reply naming the dropped id fell through to OpenCode as a fresh prompt. OpenCode's own clients keep a per-session list spliced by requestID, so key them by request id; an unnamed reply resolves when exactly one is outstanding and otherwise asks which. - permission.asked / question.asked coerced `permission`/`question` but not `id`/`sessionID`, so a non-string identity degraded the whole event to UnknownOpencodeEvent — the very stall the coercion exists to prevent. Same for a delta's part id, which the reply is reassembled from. - A failure while delivering the turn result died as an unretrieved task exception whenever an approval had already released on_message; report it to the room instead (an event needs no mentions, so it lands). - Startup logging reported the deprecated config booleans rather than the effective feature set, so every features=-based agent logged execution_reporting=False while emitting execution events. Also: cover the per-agent MCP tool-visibility map, which nothing tested even though it is what keeps sibling agents on one shared serve from seeing each other's tools; correct the class docstring (one in-process MCP server serves both tool sets); explain why the SSE last-event-id commits before dispatch; and document the four OpenCode invariants that lived only in code comments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both outcomes of a resolved ask consume the message; only the confirmation differs. Say that once instead of through an if/elif/else whose three arms all return True. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The README's setup could not work: the default provider/model (`opencode`/`mimo-v2.5-free`) is OpenCode Zen-hosted and needs a provider key the instructions never mentioned, so every prompt failed. It also omitted the empty-serve-directory requirement CI already encodes — OpenCode is a coding agent with shell and grep tools, and only 02 sets a `directory`, so from a source checkout a small model explores the repo instead of replying. Alongside that: - `settings.py` reads the repository-root `.env` through an explicit anchor rather than a CWD-relative `env_file`, so the README's "loads `.env` automatically" holds from any directory, and takes `ApprovalMode` from `band.adapters.opencode` instead of re-declaring the vocabulary. - Drop the redundant `mcp>=1.25.0` from six PEP 723 headers (the `opencode` extra already pins it) and the 14 dead `pyrefly: ignore` comments (pyrefly excludes `examples/**`). - 02 now enforces the absolute `OPENCODE_DIRECTORY` its docs promised, and describes `OPENCODE_WORKSPACE` as what it is: the `x-opencode-workspace` header the client forwards. - `agent_config.yaml.example` lists the codex and opencode examples that share each agent entry, as every other framework section does. - Document `use_logged_in_user=False` on the Copilot SDK config: it opts out of GitHub identity entirely (`--no-auto-login`), which only works paired with a BYOK `provider`. - The bridge control-hook test asserted ordering only incidentally, through a `TypeError` on an unstubbed mock; assert the hook is installed instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…recondition Two barriers of `e2e_timeout * 2` under a `extra=120` backstop meant the hard pytest-timeout always fired first, so a real stall reported a phase-less `Timeout >240.0s` and then burned two `flaky_infra` reruns. The new `tests/e2e/baseline/timeouts.py` derives a test's per-barrier deadline and its `extra=` from one fact, with the deadline taken from the largest adapter turn budget (300s) rather than an unrelated multiple of E2E_TIMEOUT — which also fixes the concurrent-instances barrier, whose widened 240s still sat below opencode's own 300s and so kept tripping healthy turns. A guard test asserts the pairing holds for 1-3 barriers. Whether a `permission.asked` is ever raised is decided by the server's permission rules, not the adapter's approval_mode, so the manual-approval smoke only works against a serve that gates `bash` to `ask`. That was documented in prose, so any other serve stalled it to the deadline; it is now a declared requirement (`Dep.OPENCODE_BASH_ASKS` / `E2E_OPENCODE_BASH_ASKS`, exported by setup-opencode.sh) that skips naming the reason, following the `Dep.CODEX_CWD` precedent — a serve exposes its effective rules nowhere, and requirement predicates are env-only by contract. Also: - The smoke matches the adapter's approval narration through `APPROVAL_REQUESTED_PREFIX` / `APPROVAL_HANDLED_TEMPLATE` exported from `RoomApprovals`, instead of re-typing the wording as regexes that a reword would turn into a deadline stall, and asserts on what `wait_until` returned. - The `/next` semantics probe is adapter-agnostic and was unpinned, so it ran live in all five lanes; pin it to `core` and derive its poll window from `e2e_timeout`. - Pin the codex and opencode CLI installs, so a backend change lands as a deliberate bump rather than an unrelated-looking lane failure. - Move the yanked-release workaround for opentelemetry-resourcedetector-gcp to `[tool.uv] constraint-dependencies`: the SDK never imports it, so publishing it in the `google_adk` extra handed every consumer a direct dependency plus a pre-release opt-in. - Narrow reply_capture's best-effort teardown to `PHXClientError` so genuine misuse (an unstarted client) propagates instead of being swallowed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…on boundaries
The runner spawns LLM-driven coding agents that execute shell commands, and it
handed each one `os.environ.copy()` — which, because the runner itself imports
the baseline settings module (whose import runs `load_dotenv(.env.test)`),
included `BAND_API_KEY_USER`: the human driver's identity. An example could act
as the user and void the runner's own scoping. The child env is now built from
an explicit allowlist plus the plan's own `env`/`forward_env`, with any
`BAND_API_KEY*` stripped unconditionally; `unset_env` is gone, since a denylist
is dead once nothing is inherited, and a plan still naming it fails loudly.
Process lifecycle:
- The startup guard was dead code: `await asyncio.sleep(0)` then reading
`returncode` never observes a child that just exited (5/5 with an immediate
`sys.exit(3)`), and its only test fabricated `returncode` on a fake. It now
waits on `process.wait()` with a readiness budget, and the test drives a real
short-lived child.
- `start_new_session=True` with no SIGTERM handling meant `kill -TERM` on the
runner left orphan children holding provisioned agents and burning LLM
budget. SIGTERM/SIGHUP now cancel the run so the existing escalation reaps the
group. (SIGINT was already correct.)
- Cleanup covered only process creation, so a cancellation during the readiness
wait leaked a spawned child that nothing owned.
- `wait_or_exit` leaked both of its tasks whenever the enclosing step was
cancelled — which is exactly what the TaskGroup does to siblings on failure —
leaving a reply waiter polling a channel already left.
Typing the toolkit objects instead of `Any` exposed that the runner's central
marker assertion was a no-op: `assert_contains_any` takes ONE iterable, and
splatting a single expectation passed the marker *as* the iterable, so its
characters became the options and any reply containing any one letter passed.
Fixed at both call sites, with the test fakes replaced by the toolkit's real
observation classes so the drift cannot return.
Smaller: `validate_template` missed positional placeholders (`{0}`, `{}` raise
IndexError), so an invalid plan passed `--dry-run` and failed only after live
provisioning; the scorecard is written from a `finally` rather than lost when a
run fails; the agent key file is created 0o600 instead of chmod'ed after; the
example-command pattern matches the `python …` forms the READMEs actually use
and no longer carries a dead alternative; a non-UTF-8 file under examples/ no
longer aborts the whole inventory; each script verifies its own repository
anchor instead of trusting `parents[4]`; and the plan trust model (a plan's
`command` is arbitrary argv — `resolve_example_path` is a typo check, not a
sandbox) is stated in SKILL.md instead of implied by the code.
Tests went 11 -> 56, now covering the signal escalation with real process
groups, the plan rejection table, the config file's mode, and the failure paths
of the step assertions; each fix was mutation-checked by reverting it. pyrefly
now includes `.claude/skills/**/*.py`, which had no type gate at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OpenCode keys MCP registrations globally by name. Teardown released _state_lock before issuing its name-only disconnect, while client startup registers under that lock, so the two were not mutually exclusive: when a successor room started during the gap, the outgoing client's disconnect landed after the successor had registered and stripped it. Nothing re-registers -- that only runs on the turn that creates the client -- so the agent kept a live client with its Band tools invisible until a restart. The room is also not tracked in _rooms until its first message, so the existing "another room arrived" recheck cannot see a joined-but-silent room. Move the disconnect inside the lock, making it symmetric with registration. The remaining teardown stays outside: it touches objects already detached from self, and mcp_backend.stop() can wait out OpenCode's open SSE read. The fake client now models the serve's global name keying so the race is expressible; the regression test fails without the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ssion loss Three defects surfaced by a review pass, each with a guard test that fails without its fix. Interrupt did not interrupt. The runtime interrupts a turn by cancelling on_message, but the turn watcher runs as a detached task: it survived, posted the very reply the user stopped, and -- since it alone clears turn_future -- held the room's busy guard for up to turn_timeout_s, answering every following message with "still processing". on_message now drops the turn state on cancellation, which cancels the watcher, and aborts the OpenCode session. Extracted _abort_session so the timeout and interrupt paths share it. Abandoning an approval orphaned its expiry timer. _fail_request popped the entry without cancelling its timeout task, putting it past cancel()'s reach -- it only iterates entries still registered. The survivor kept RoomApprovals, and through the ApprovalPorts closures the whole _RoomState, alive until the wait timeout. Cancel where the pop happens, so every abandonment path is covered rather than the two that were spotted. The session gate discarded pre-upgrade sessions. mcp_server_name is None for any session persisted before the adapter recorded it, so the equality failed and every existing room silently started a new session on the first turn after deploy. Treat an unknown name as ours. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A review flagged that formatters.py and codex.py describe the platform's leading mention block differently -- bare @handle tokens versus "@handle DisplayName" pairs -- so one of them had to be wrong. Probed the live platform. It prepends one normalized @[[uuid]] per mention to the content (and does not re-prepend when the client already sent that form); replace_uuid_mentions() rewrites each to @handle. Handles are slugified and truncated to owner/agent-name, so an agent named "Support Bot Probe" gets the handle "owner/e2e-band-xxxxx-support-b" -- never whitespace, whatever the display name. So the @\S+ match is correct and codex.py's docstring was wrong. Record the slugification, since it is what makes the pattern safe, and drop the DisplayName claim. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_extract_local_command scanned the first 20 tokens for any known slash word,
so prose arguing against a command invoked it: "@team/bot Please don't
/approve req-1 yet" resolved to ("approve", "req-1 yet"), and the handler
takes the first argument token as the approval id -- approving req-1. Bare
"do not /approve" resolved to ("approve", "") which, with a single pending
request, approves it. Codex has no authorized-sender gate, so any participant
in the room could trigger this in ordinary conversation.
The bounded scan existed because the mention block would otherwise hide a real
command. strip_leading_mentions() is already the shared normalization boundary
for exactly that -- claude_sdk and opencode both use it -- so Codex now does
too and parses the leading token only, matching claude_sdk._extract_command.
Adapters keep their own command vocabularies; only the normalization is shared.
_COMMAND_TOKEN_SEARCH_LIMIT is gone.
Its comment, its regression test, and one fixture also asserted the
"@handle DisplayName" pair shape that a live probe disproved: the platform
prepends one @[[uuid]] per mention, and handles are slugified, so a display
name never reaches the content. Replaced with tests for what the platform
actually sends, plus helper-level coverage of that shape on
strip_leading_mentions itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
partition(" ") narrowed the separator to a literal space, so "/approve\treq-1"
and "/approve\nreq-1" stopped parsing -- the token scan this replaced split on
any whitespace. Use split(maxsplit=1), which keeps the prefix-only design and
restores the old separator behavior. Guarded by both shapes as test cases.
Also sharpen the comment on the typed-handle formatter test: the platform skips
its @[[uuid]] prepend only when the content already leads with that normalized
form, not merely because the client typed the handle -- both tokens are present
in that case, which is what the test asserts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AlexanderZ-Band
enabled auto-merge (squash)
July 26, 2026 08:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OpenCode integration validated live against the L0–L4 baseline (opencode 1.18.4, 11/12 green + one throttling flake). Closing the adapter's test gaps surfaced four defects, each reproduced live before it was fixed.
Adapter fixes
LocalMCPServer.stop()hung forever. uvicorn'stimeout_graceful_shutdownis unbounded and OpenCode holds its/sseGET open. Now bounded bySERVER_STOP_TIMEOUT_S.send_messagewithout the requiredmentions.FakeAgentToolsaccepted what the realAgentToolsrejects, so no unit test caught it; the fake now enforces mentions.doom_loop: askheuristic trips on multi-tool chains and themanualdefault waits 300s, then rejects. The adapter now auto-approves asks naming its own registered band tools in every mode (codex parity); the E2E builder runsapproval_mode="auto_accept".room_id, which the model was never told — now injected per turn. OpenCode surfaces MCP tools as{server}_{tool}; reportedtool_call/tool_resultnames are canonicalized back.Plus, from the review pass: pending asks are keyed by request id (a second ask used to drop the first, blocking its tool call until the turn timed out);
turn_timeout_sno longer charges manual-approval time to the compute budget;id/sessionIDare coerced likepermission/question; a failure delivering the turn result reports to the room instead of dying as an unretrieved task exception.Structure
src/band/integrations/opencode/events.py— typed Pydantic SSE models behindparse_opencode_event; malformed payloads degrade to an ignoredUnknownOpencodeEvent.src/band/adapters/opencodeis now a package (config.py/adapter.py/approvals.py). Public import path unchanged.Breaking change
OpencodeClientProtocol.deregister_mcp_server→disconnect_mcp_server, andDELETE /mcp/{name}→POST /mcp/{name}/disconnect(verified against opencode 1.18.4's route table). Breaks any custom client implementing the protocol.Tests
tests/integrations/opencode/test_client.py—HttpOpencodeClientagainst a real in-process ASGI fake (SSE parsing,Last-Event-IDresume, every REST call shape), via a newtransportinjection seam.tests/adapters/opencode/— the old single file split by concern, plus turn-timeout abort,Emit.USAGEaggregation,alwaysreply, capability gating, per-agent MCP tool visibility.supports={MEMORY, CONTACTS}(wassupports=(), so the capability smokes silently never ran against it).tests/e2e/baseline/timeouts.pyderives each barrier's deadline and its pytest-timeout backstop from one definition; the old backstop always fired first. The manual-approval smoke's precondition is now a declaredDep; the/nextprobe is pinned instead of running live in all five lanes.Also in this branch
examples/opencode/02–06and a README. The documented setup couldn't work: the default model is OpenCode Zen-hosted and needed an unmentioned provider key, and the empty-serve-directory requirement was missing.bug-hunting-via-exampleskill — new. Its script tests sat under.claude/skills/**, whichtestpaths = ["tests"]never collects, so CI ran none of them; moved totests/skills/bughunting/behind atests/paths.pyanchor. Fixed: the runner handed spawned agents the driver'sBAND_API_KEY_USER,SIGTERMorphaned children holding provisioned agents, and the central marker assertion was a no-op (assert_contains_anytakes one iterable, so splatting made the marker's characters the options). 11 → 56 tests; pyrefly now covers.claude/skills/**.google_adkextra into[tool.uv] constraint-dependencies; CLI installs pinned.Test plan
ruff check/ruff format/pyrefly checkcleanE2E_TESTS_ENABLED=true BAND_E2E_LANE=backends uv run pytest tests/e2e/baseline/ -k opencode→ 11 pass, 1 flake (test_per_adapter_replies, free-tier throttling; passed in three prior full runs and solo immediately after). All 12 have passed live post-fix, including both memory smokes, previously hard failures.🤖 Generated with Claude Code