Repository navigation
feat(providers): accept OpenCode 1.18.31-1.18.34 version range - #713
Yoseph-Zuskin wants to merge 1 commit into
Conversation
- Replace the exact pin with inclusive _OPENCODE_MIN_VERSION / _OPENCODE_MAX_VERSION triplets; _parse_opencode_version returns a triple and both gates (preflight + auth check) share one range predicate; prereleases and unparsable output still fail closed; the debug-config subset check is untouched - 14 new boundary cases: parse accept (.31-.34) / reject (.30, .35, prerelease, garbage, empty); probe accept / reject; RED witnessed on the exact pin before the cutover - Verified: tests/provider 55 passed, 3 skipped (pre-existing POSIX-only skips); ruff + format + diff-check clean; live matrix .31/.32/.33/.34 all green (auth + preflight + isolated live run + run --help diff) - Ponytail review: lean, no findings Signed-off-by: Yoseph Zuskin <zuskinyoseph@gmail.com> Co-Authored-By: OpenCode Muse Spark 1.3 Free (1M context) <noreply@opencode.ai>
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Hi @Yoseph-Zuskin, thank you for turning the one-release-at-a-time OpenCode pin into a verified version window, and for testing each version live!
Value and readiness: The code change is correct and still fails closed. _is_supported_opencode_version admits exactly 1.18.31, 1.18.32, 1.18.33 and 1.18.34. Both gates reject every other version before auth list or inference runs, including prereleases, unparsable or empty output, and a non-zero --version exit. The debug config subset check is unchanged. Every admitted version has evidence. Versions .31, .32 and .33 were verified in #575, #613 and #665. Since #562, the isolation and preflight code they were checked against has changed only in version strings. The upstream v1.18.33...v1.18.34 delta touches nothing the policy depends on. It is not ready yet: README and docs/DEVELOPMENT.md still tell users the provider needs exactly 1.18.33.
Material findings
- [Blocker]
README.md:245,README.md:653(lines 300 and 712 on currentmain),docs/DEVELOPMENT.md:348: these lines still sayopencode_cli"fails closed unless the installed OpenCode version is exactly1.18.33". DEVELOPMENT adds that it "fails closed on every other runtime version because the deny-all policy is verified against that exact release". After this PR, those statements describe a security gate the code no longer has. A user on 1.18.31 would upgrade for no reason. Each earlier bump (#575, #613, #665) updated these lines. Please list the accepted versions (1.18.31 through 1.18.34) and say that every other version, including prereleases, still fails closed. - [Non-blocking]
src/skillspector/providers/_agent_cli.py:463-464: the only record of which versions were verified, and how, is the table in the PR description. Comments at:457,:597-601,:620,:695and:749still describe a single 1.18.33 pin;:600says "The same pinned version uses this override". Please add a comment beside the bounds that lists each admitted version with its evidence (#575, #613, #665, this PR). Please also reword theOPENCODE_TEST_HOME/OPENCODE_TEST_MANAGED_CONFIG_DIRcomment to say these internal overrides were confirmed for every version in the window. This matters because the description understates the version gate. Thedebug configcheck sees 11 resolved keys and the selected agent's permission. It cannot tell whether OpenCode still honours those two test hooks, which keep homeAGENTS.md,.opencode,.claudeand platform managed config out of the child. Any patch release may rename them. The version gate therefore stays load-bearing, not "belt-and-suspenders", and each future widening needs the same upstream review #575, #613 and #665 received. - [Non-blocking]
tests/provider/test_opencode_cli.py:298-308:test_parse_supported_versions_compare_in_rangeandtest_parse_unsupported_versions_rejectedrepeat the range comparison inline instead of calling_is_supported_opencode_version. They would pass even if that predicate broke. The auth-check cases at:312-324do exercise the real predicate at .30, .31, .34 and .35. The preflight gate itself is tested only with 1.18.99 (:457-468). A fake-binaryrun_agent_clicase at 1.18.30 and 1.18.35 would pin that gate directly.
PIC tradeoffs:
- Window or explicit set. The review on #613 suggested exact matching against an explicit set of individually verified versions, never a range. Today the two are equivalent, because .31 to .34 all have evidence. They diverge later. If the maximum moves from .34 to .36 after only .36 is checked, .35 is admitted silently, and a window cannot exclude one bad patch. A
frozensetof verified triples makes per-version verification structural at almost no cost. Issue #712 asks for a range, so this is the PIC's call. - Re-admitting 1.18.31 and 1.18.32. This helps users who did not upgrade. Those builds print
debug configwithout the credential redaction added in 1.18.33. The preflight only parses that output in memory and never logs or reports it, so admitting them adds no exposure. - Release notes. The next release notes should state the accepted window. The 2.12.0 notes still say exactly 1.18.31.
Verification and gaps:
- Traced both gates at this head.
_preflight_opencode_policy(_agent_cli.py:649-658) and_opencode_auth_check(:860-869) share the predicate. Tuple ordering admits exactly four triples.1.19.0,1.18.3,1.18.34-1,garbageand empty output are rejected. - Ran
git diff c3bb8132 main -- src/skillspector/providers/_agent_cli.py. Only version strings differ, so .31, .32 and .33 were verified against the same isolation and preflight code. - Upstream
anomalyco/opencodev1.18.33...v1.18.34(30 commits, 135 files). The CLI source changes are two added request headers (x-opencode-session-id,x-opencode-parent-session-id), a TUI path fix and macOS signing.bun.lockonly bumps workspace versions. Nothing touches config loading, permission merging,OPENCODE_*handling,debug configorrunflags. v1.18.34 is the latest upstream release today. - I could not reproduce the live matrix in the description. Per policy, I did not run the tests or the OpenCode binary.
- CI: all six checks pass on this head, and it merges cleanly with current
main. - Overlaps: this PR conflicts with #716 in the constants block after
_OPENCODE_DENY_ALL(#716 inserts_ZEN_FREE_TIER_MARKERSright after the line this PR replaces). It also conflicts with #738 at the fake binary's--versionline (tests/provider/test_opencode_cli.py:370). Both fixes are mechanical. I suggest landing #716 first, then rebasing this PR along with the doc fix. #747 changes the TypeScript extension, not this provider, so it does not overlap.
Decision: Changes Requested (reviewed head 684922305d15b3ead51a0dd45483e80b85fabafa)
Fixes: #712
Replaces the exact OpenCode pin (1.18.33) with an inclusive
supported range, 1.18.31 through 1.18.34. Every upstream patch
release currently locks out provider users until a new pin lands;
the runtime debug-config resolved-subset check already re-verifies
the deny-all policy per invocation, so the version gate can be a
range without weakening fail-closed.
What changed:
parser returns a triple and both gates share one range
predicate. Error messages print the range.
--versionoutput are stillrejected; the debug-config subset check is untouched.
(.30, .35, prerelease, garbage, empty); probe accept / reject.
Verification matrix (all through the production path under the
isolated deny-all env; live runs on
openrouter/nvidia/nemotron-3-ultra-550b-a55b:free):
Unit: tests/provider 55 passed, 3 skipped (pre-existing
POSIX-only skips). ruff check + format + git diff --check clean.
Broader suite: 10 failures (test_cli tree rendering,
test_compare_scan_accuracy, test_create_github_release) all proven
pre-existing via pristine-tree stash checks.
Follow-up bumps widen MIN/MAX and append matrix rows.