Repository navigation
fix: declare OpenCode capability strategy - #4099
Conversation
|
Thanks for this — but I do not think the declaration matches what the adapter actually does, and I would rather say so before you spend more time on it.
cmd = [
"opencode", "run",
"-m", model_config.model,
"--format", "json",
prompt,
]Against the three values in the diff:
That is also why CI is red, and it is worth being clear that the test is right and the diff is wrong — def test_text_signal_default_for_plain_adapters() -> None:
for name in ("aider", "droid", "opencode"):
assert strategy_for(name).event_channel is EventChannel.TEXT_SIGNALSPlease do not update that assertion to match the declaration — it is the thing keeping the table honest. So the decision, and it is yours: either drop this declaration and close the PR, or implement the capabilities first and declare them second — teach If it helps: |
|
You merged The merge brought the branch up to date but left the diff intact. Checked on your current head: and the adapter's spawn surface at So Nothing has changed about the decision from my earlier comment, and it is still yours to make:
If you would like a hand deciding, the check that settles it is No rush from my side, and no need to keep the branch fresh while you decide — there is an andon hold on the whole merge queue while #4102 is open, so nothing in this repo is enqueuing today regardless. |
|
Update: we picked this up and pushed the remaining items directly — details in the comment below. The two options this comment originally offered are superseded; nothing is needed from @KrzysiekSko to land this. |
…aration The opencode row declared unsupported resume and dangerous mode, and the adapter matched it: spawn was a fixed `opencode run -m <model> --format json <prompt>` with no permission configuration at all. A worker therefore inherited whatever the operator's own opencode config resolved to, so two operators running the same plan got different agent behaviour, and a permission resolving to `ask` blocks a headless run until the timeout watchdog kills it (upstream anomalyco/opencode#36762) - surfacing as a timeout rather than a blocked permission. Both axes now describe flags the adapter passes: - Every spawn carries an explicit OPENCODE_PERMISSION policy derived from the declared dangerous-mode strategy - escalated alongside `--auto` when it permits unattended action, a deny policy when it does not. It is applied after the environment allow-list so the pinned policy is what the worker sees rather than the host's. Neither direction resolves to `ask`. - `--continue` re-enters the prior session on a continuation retry, which the per-task worktree makes unambiguous, and the adapter opts into the continuation path so the warm retry `checkpoint_retry_capability` derives from the resume axis is backed by something real. Command construction moves into `_build_command` so the argv is assertable without mocking subprocess; the matrix row, the capability profile and the pinned contract move together. The event channel deliberately stays `text-signals`. The CLI does emit NDJSON under `--format json`, but nothing consumes it yet, and declaring a channel no parser reads would claim a surface that does not exist. Consuming it is the remaining half of sipyourdrink-ltd#3676. Partial implementation of sipyourdrink-ltd#3676.
|
Thank you for this, and for the shape of it — scoping the body explicitly to the declaration, and naming NDJSON consumption, permission configuration and resume wiring as separate follow-ups, is what made the remaining work briefable in a paragraph. Your three values turned out to be right on the axes I first pushed back on, and the finding underneath them was real: the row was wrong, and the adapter's permission behaviour was worse than the row. We finished the remaining items ourselves and pushed them to this branch — you shouldn't have to chase our review backlog to get a correct row landed, and your commit stays first either way. On top of it: an explicit If you'd like another in the same seam, #3671 is the same class of finding on the Letta Code adapter — prose signals, no session binding, unconditional unrestricted permissions — and most of what this PR worked out about pinning permission posture transfers directly. It's unassigned and up for grabs; happy to review whenever you pick it up. |
|
@KrzysiekSko — thank you for this one. Declaring the OpenCode capability strategy explicitly instead of letting it be inferred is exactly the kind of change that stops a class of bugs rather than one instance of it. The PR is armed for merge and moving through the queue now; nothing further needed from you. If you want the next thing in the same territory: #3887 — a spike on what an LLM provider can actually observe, and what provider-independence would have to mean given that. It is adapter-shaped and it needs someone who has just been inside a capability declaration and knows where the seams are. Reserved for you; happy to unreserve if it is not your thing. Separately, a small optional ask from my side. The project needs visibility and I am stretched thin. If you ever felt like writing a short note about it somewhere online — X, Medium, LinkedIn, your own blog, anywhere — it would help a lot. Short is fine; a paragraph about something you touched here is enough. Two examples of what people have written, just for register:
Any language is welcome — English is not required and the non-English one landed better than most. Reddit and HN I would skip. I have tried both; it mostly turned into a pile-on and is not worth the trouble. |
# Conflicts: # docs/release-notes/unreleased.md
Bisect-on-red triageThe CI workflow failed on
Auto-applied label: Posted automatically by |
github-actions
Bot
added
the
regression
Cuts v3.17.0 from main at fd0444b (queue drained, 66 check runs green, milestone 23 clear). Highlights: a failed or cancelled task now reaches dependents outside its subtree, and a DAG node whose every inbound edge skipped is materialised rather than dropped; `audit verify` is read-only and reports append, rewrite and identity separately; absence claims carry a bound coverage record; agent and skill catalogs are opt-in, lockfiled and tamper-checked; the task-scope gate sees sub-router routes again; the Hypothesis and Schemathesis smoke profiles are derandomised. The hand-written release notes ride in this branch (docs/release-notes/v3.17.0.md), not in this field. ### Changes since v3.16.0 fd0444b fix(adapters): select the openai auth type for a custom qwen base URL (sipyourdrink-ltd#4265) fcccf43 chore(ci): ratchet coverage baseline up to 84.01% (sipyourdrink-ltd#4263) 9ba950a fix(tasks): propagate a cascade cancel to dependents outside the subtree (sipyourdrink-ltd#4258) 414a8a6 fix(planning): materialise a DAG node whose every inbound edge skipped (sipyourdrink-ltd#4253) f532daf fix(agents): carry system_addendum on every resume, container, and sandbox spawn (sipyourdrink-ltd#4254) a2d595f fix(cli): diagnose a spawned-then-dead agent on the remaining run surfaces (sipyourdrink-ltd#4255) da1664b test(orchestrator): assert overlap not elapsed time in parallel-verify test (sipyourdrink-ltd#4251) f84e98f fix(security): settle subject's type in verify_result_bundle (sipyourdrink-ltd#4252) 251a541 fix(tasks): propagate failure to dependents and derive an unreachable set (sipyourdrink-ltd#4243) c028c65 feat(replay): derive a run's read-path set from its journal (sipyourdrink-ltd#4180) (sipyourdrink-ltd#4219) 257ae31 fix(cli): print overall summary panel last in audit verify (sipyourdrink-ltd#4202) (sipyourdrink-ltd#4250) f5400a3 fix(cli): thread resolved model to plan-approval panel and cost estimation (sipyourdrink-ltd#4214) (sipyourdrink-ltd#4235) 1ea79a9 fix(persistence): bound PostgresTaskStore.list_tasks() by default (sipyourdrink-ltd#4240) 914263f fix(cli): diagnose a spawned-then-dead agent instead of detaching silently (sipyourdrink-ltd#4246) 228ec8a fix(audit): make verify read-only and separate post-seal appends from a rewritten prefix (sipyourdrink-ltd#4245) 83c1ccb fix(replay): make journal verify seal-aware for identity verdicts (sipyourdrink-ltd#4244) ec97cd8 docs(cli): add unified verification exit-code reference table and tests (sipyourdrink-ltd#4206) (sipyourdrink-ltd#4241) d112b5e fix(verify): let automation gate on receipt provenance tier (sipyourdrink-ltd#4239) d04e40a fix(evolve): refuse upgrade-proposal auto-spawn with no terminal task (sipyourdrink-ltd#4229) dbd036c fix(adapters): normalize adapter name casing in admission receipt filename path (sipyourdrink-ltd#4224) (sipyourdrink-ltd#4234) 8a9ec6a fix(adapters): gate PYTHONPATH injection behind opt-in flag (sipyourdrink-ltd#4233) 3b2b317 fix(orchestration): regenerate summary.json at shutdown (sipyourdrink-ltd#4232) cffca55 fix(agent_lifecycle): re-evaluate deferred death judgments after reap (sipyourdrink-ltd#4231) 72f7757 fix(mailbox): derive since_seq cursor from consumption records (sipyourdrink-ltd#4215) ef88c9a fix(git): exclude injected scheduled_tasks.json from worktree tracking (sipyourdrink-ltd#4227) 8ab3fec feat(cli): reach the run attestation projection from the command line (sipyourdrink-ltd#4139) b13b76e chore(deps): update dependency astral-sh/uv to v0.12.4 (sipyourdrink-ltd#4216) 04e7adb chore(deps): update dependency platformdirs to v4.11.3 (sipyourdrink-ltd#4218) ebc2759 docs: regenerate adapter last-green table from canary receipts (sipyourdrink-ltd#4220) 0ff62ac docs: add 16 README translations (wave 2) (sipyourdrink-ltd#4172) 99ad660 feat(quality): record and bind coverage for absence claims (sipyourdrink-ltd#4188) 8b9df94 feat(core): add count_tasks method to TaskStore and PostgresTaskStore (sipyourdrink-ltd#4158) (sipyourdrink-ltd#4191) fc24617 ci(soc2): wire weekly evidence pack export step to sink (sipyourdrink-ltd#4165) 45c832e fix(workspace): scope workspace_merge_order task read to caller tenant (sipyourdrink-ltd#4163) 9ce948b chore(ci): ratchet coverage baseline up to 83.92% (sipyourdrink-ltd#4207) 8ccd357 feat(communication): turn MAX_PENDING_PER_TASK into a draining backlog bound (sipyourdrink-ltd#4152) (sipyourdrink-ltd#4197) b1c673d feat(tasks): enforce well-founded bounded recursive task decomposition (sipyourdrink-ltd#4179) (sipyourdrink-ltd#4196) 9313bf4 fix(mailbox): record consumption and surface silent render failures (sipyourdrink-ltd#4193) bc5c6b1 feat(tasks): add typed task-completion record schema, validation, and canonical serialization (sipyourdrink-ltd#4185) (sipyourdrink-ltd#4192) ecc2014 feat(persistence): add limit and offset to PostgresTaskStore list_tasks (sipyourdrink-ltd#4169) 8c569db chore(ci): ratchet coverage baseline up to 83.91% (sipyourdrink-ltd#4199) d8de7c1 feat(webui): record and gate colour token contrast measurements (sipyourdrink-ltd#4164) 8e3c112 docs(adapters): fix internal-scheduler LLM description drift (sipyourdrink-ltd#4198) b5edd54 fix(scripts): select test files from positional paths in run_tests (sipyourdrink-ltd#4195) 9637d8f feat(storage): export soc2 evidence pack to configured sink (sipyourdrink-ltd#4162) fabb773 test: add journal verifier to mutation gate at 72.5% kill rate (sipyourdrink-ltd#4194) c23d847 feat(suspension): suspend-side grant population + journal continuation (sipyourdrink-ltd#3649) (sipyourdrink-ltd#4131) 5248ae7 feat(memory): bind compaction results to a pre-compaction content hash (sipyourdrink-ltd#4189) 43d5851 chore(deps): update github/codeql-action digest to ff2f1c6 (sipyourdrink-ltd#4186) 3687e68 feat(knowledge): wire review corrections into file_lesson convention receipts (sipyourdrink-ltd#4170) 975099d fix(webhooks): thread tenant_id through ci-fix retry counters (sipyourdrink-ltd#4171) 62ef987 docs: add the Russian README (sipyourdrink-ltd#4094) e6d659e feat(volunteer): durable lease store for hub task claims (sipyourdrink-ltd#4036) (sipyourdrink-ltd#4190) 541835b fix(tests): resolve conflicting Hypothesis "deep" profile registrations (sipyourdrink-ltd#4187) 2410305 fix(escalation): produce terminal receipt on missing or empty run journal (sipyourdrink-ltd#4168) e070e02 feat(mcp): advertise repo and build provenance in capability card (sipyourdrink-ltd#4166) 5ae3558 fix(status): scope health check task count to caller tenant (sipyourdrink-ltd#4161) eef0525 fix: declare OpenCode capability strategy (sipyourdrink-ltd#4099) 328f8d0 feat(agents): add opt-in discovery of harness-local agent and skill resources (sipyourdrink-ltd#3975) (sipyourdrink-ltd#4126) 7e00682 feat(volunteer): claim etiquette and duplicate detection without a coordinator (sipyourdrink-ltd#3873) (sipyourdrink-ltd#4130)
…urdrink-ltd#5541) ## Summary `opencode`'s adapter contract declared its `event_channel` as `text-signals`, justified by a comment stating that `event_channel` means "what Bernstein currently parses" — but that reading contradicts the axis's own written definition in two places: - `EventChannel.STREAM_JSON`'s own docstring in `_contract.py`: *"Upstream emits newline-delimited JSON events (Claude/Cursor/Gemini)."* - `docs/adapters/capability_contract.md`: *"Newline-delimited JSON events from the upstream CLI."* Both say `event_channel` names a property of what the **upstream CLI emits**, not what Bernstein currently consumes. Under that (already-written) definition, `opencode` should be `stream-json`: `opencode.py` has passed `--format json` on every spawn since sipyourdrink-ltd#4099, and the CLI emits NDJSON under that flag. `cursor` is already declared `stream-json` under this exact reading despite having no dedicated stream parser either (checked: only `claude_stream_parser.py` and `goose_stream_parser.py` exist). ## Why this is safe to flip `event_channel` has exactly one runtime reader outside its own doc/table rendering: `acp_channel.py`, which only checks `is EventChannel.ACP` (unrelated to `STREAM_JSON` vs `TEXT_SIGNALS`). So this is a pure declaration correction — nothing about what Bernstein actually parses for `opencode` changes. NDJSON consumption for this adapter remains separate, unbuilt follow-up work, same as before. I also checked whether `docs/adapters/capability_contract.md` and `STRATEGY_MATRIX` agree everywhere else before touching anything wider than this one row — they agree for 41 of 44 comparable rows; `copilot`, `kilo`, and `letta_code` already disagree, unrelated to this issue and out of scope here. ## Changes - `src/bernstein/adapters/_contract.py`: `opencode`'s `STRATEGY_MATRIX` row now declares `event_channel=EventChannel.STREAM_JSON`; the justifying comment now states the upstream-emits reading explicitly and cites `cursor` as the adapter already declared the same way. - `docs/adapters/capability_contract.md`: the `opencode` row now reads `flag | cli-flag | stream-json`. - `tests/unit/test_adapter_opencode.py`: added `TestEventChannelDeclaration` — one test pins the declared enum value, the other ties it directly to the spawn command (`--format json` must be present), so either a future flag removal or a re-flip of the declaration alone fails immediately. - `tests/unit/adapters/test_capability_enums.py`: the two tests that had pinned the old `text-signals` value for `opencode` are updated to `stream-json`, with a docstring on the second explaining why. - `docs/release-notes/fragments/3676-opencode-event-channel.md`: user-visible note. ## Out of scope (per the issue and the maintainer's latest comment on it) - NDJSON event consumption, the permission strategy, and `--session` wiring are explicitly out of scope for this PR — the permission-pinning and `--continue` wiring were already shipped in sipyourdrink-ltd#4099, and NDJSON consumption is separate unbuilt work. - The `copilot` / `kilo` / `letta_code` doc-vs-matrix mismatches found while checking this are a separate, pre-existing issue, not touched here. ## Testing ``` uv run pytest tests/unit/test_adapter_opencode.py tests/unit/adapters/test_capability_enums.py -q uv run ruff check src/bernstein/adapters/_contract.py tests/unit/test_adapter_opencode.py tests/unit/adapters/test_capability_enums.py uv run pyright src/bernstein/adapters/_contract.py uv run lint-imports ``` All pass locally (73 tests across the two files; ruff, pyright, and the architecture-contract check clean). Fixes sipyourdrink-ltd#3676.
What
Correct the OpenCode capability strategy declaration for issue #3676.
Why
The source matrix currently uses the default
AdapterStrategy(), which renders OpenCode as:unsupported | unsupported | text-signalsThat contradicts the adapter's current OpenCode invocation and the upstream capabilities verified in version 1.18.16.
How
Declare OpenCode with the same capability axes as Claude:
flagcli-flagstream-jsonThe generated documentation remains unchanged because it is derived from this source matrix.
This PR intentionally does not implement NDJSON consumption, explicit permission configuration, or session resume wiring; those remain separate follow-up changes as requested in the issue.
Verification
The change is limited to the authoritative strategy matrix in
src/bernstein/adapters/_contract.py. The contract renderer/drift check should now resolve OpenCode to the declared values.Local test commands were not run in this environment.
Update from the maintainers
The declaration above is correct on the resume and dangerous-mode axes, and the
decomposition in the original description is what made the rest briefable. The
remaining wiring has been pushed to this branch so the two halves land together
rather than leaving the matrix describing something the adapter does not do.
Added on top of the original commit:
OPENCODE_PERMISSIONpolicy derived from the declared dangerous-modestrategy — escalated alongside
--autowhen it permits unattended action, adeny policy when it does not — applied after the environment allow-list so the
pinned policy is what the worker sees rather than the host's. Neither
direction resolves to
ask, which blocks a headless run indefinitely againstupstream
anomalyco/opencode#36762.--continuebacks the resume axis. The adapter opts into the continuationpath, so the warm retry
checkpoint_retry_capabilityalready derives fromresume=flagis backed by a flag the adapter passes. The per-task worktreemakes "the prior session here" unambiguous.
_build_command, so the argv isassertable without mocking subprocess. The matrix row, the capability profile
and the pinned contract move together.
event_channelstaystext-signals. The CLI does emit NDJSON under--format json, but nothing consumes it yet, and declaring a channel noparser reads would claim a surface that does not exist.
Tests:
tests/unit/test_adapter_opencode.pyasserts the escalated flag, therestricted policy, that a host
OPENCODE_PERMISSIONnever reaches the worker,and that the continuation flag lands in the constructed command — each failing
against the previous adapter.
tests/unit/adapters/test_capability_enums.pypins the derivation from the resume axis to the retry surface so the declaration
and the implementation cannot drift apart again.
Because NDJSON consumption is still outstanding, this is a partial
implementation and #3676 stays open after the merge.
Partial implementation of #3676.