Repository navigation
Add Asimi support as a new harness - #28
LeonHibnik wants to merge 5 commits into
Conversation
zro-inference
left a comment
zro-inference
left a comment
There was a problem hiding this comment.
This PR adds Asimi as a supported harness: a new src/engine/tools/asimi.ts adapter that launches via provider env conventions (no config files on disk), plus catalog/README/CHANGELOG/tests wiring. The adapter itself, the env-only launch path, and the new tests/asimi.test.ts launch assertions look sound, and no tests were deleted.
One blocker: the CI change disables the previously-running prime live-API probes in addition to the intended asimi skip, silently removing existing coverage. Remaining notes cover the asimi live-check version command and a vacuous assertion in the new test. See inline comments.
JenClaude command reference
Reply with @MoonMathJen on this PR to trigger commands:
| Command | Example | What it does |
|---|---|---|
| Re-review | @MoonMathJen rereview |
Re-review after changes (full or scoped — see below) |
| Stop | @MoonMathJen stop |
Cancel in-progress review |
| Stop + summarize | @MoonMathJen stop and summarize |
Post partial findings so far |
| Focus specialty | @MoonMathJen only security |
Review only the named specialty |
| Skip specialty | @MoonMathJen skip test-coverage |
Review everything except the named specialty |
| Change model | @MoonMathJen pr-review-model: kimi-k2.7-coder |
Run this review with a specific model |
| Add context | @MoonMathJen focus on auth edge cases |
Free-text guidance passed to reviewers |
Specialties: correctness, security, test-coverage, adversarial
Any text in the comment beyond a recognized command is passed to the reviewers as context. Use it to point them at areas that matter, explain what changed since the last review, or call out constraints (e.g. "skip generated files", "the auth flow is the riskiest part here"). Hard review rules still win over reviewer guidance.
JenClaude metrics: cost $0.000, time 41m 32s, tokens 1,764,191.
| matrix.harness.id != 'hermes' && | ||
| matrix.harness.id != 'openclaw' && | ||
| matrix.harness.id != 'asimi' && | ||
| matrix.harness.id != 'prime' && |
There was a problem hiding this comment.
[P1] Live probes silently disabled for prime, not just asimi
The base Check live API guard had no harness exclusions (verified against main), so npm run test:live -- prime ran Prime Agent's cache + max-reasoning probes. This PR adds matrix.harness.id != 'prime' alongside the new asimi exclusion, so the entire live step is now skipped for prime and its existing live coverage disappears with no replacement. It also contradicts scripts/ci/check-live.mjs, which still defines checkPrime() and deliberately does not list prime in SKIPPED_LIVE_HARNESSES (only hermes/openclaw/asimi). If the intent was only to skip asimi, drop the prime clause; if prime must be skipped, add it to SKIPPED_LIVE_HARNESSES instead so the report records an explicit prime.live skip.
Suggested fix: remove matrix.harness.id != 'prime' && from this guard (or move the prime decision into SKIPPED_LIVE_HARNESSES).
| const reportPath = process.env.ZRO_REPORT || path.resolve("artifacts/zro-live-report.json"); | ||
| const runId = (process.env.ZRO_CI_RUN_ID || `${Date.now()}`).replace(/[^A-Za-z0-9_.-]/g, "-"); | ||
| const supportedHarnesses = ["claude", "codex", "grok", "kilo", "omp", "opencode", "pi", "hermes", "openclaw", "prime"]; | ||
| const supportedHarnesses = ["claude", "codex", "grok", "kilo", "omp", "opencode", "pi", "hermes", "openclaw", "prime", "asimi"]; |
There was a problem hiding this comment.
[P2] asimi added to live harness list without a version command
asimi was appended to supportedHarnesses here, but no asimi: ["asimi", "--version"] entry was added to the clientCommands map in collectVersions() (line 151-162). collectVersions() runs before the harness loop, so the documented manual path — ZRO_API_KEY=sk-... pnpm run test:live asimi (CONTRIBUTING.md) or plain pnpm run test:live over all — now dies with an unhandled TypeError: Cannot read properties of undefined (reading '0') at line 165 instead of a clean check, writing ok: false/checks: [] to the report. CI currently masks this because the live step is skipped for asimi, but the manual path is broken.
Suggested fix: add asimi: ["asimi", "--version"] to the clientCommands map in collectVersions().
| expect(Object.values(plan.env ?? {})).not.toContain("collector.invalid"); | ||
| }); | ||
|
|
||
| it("maps reasoning levels to asimi's low|medium|high efforts", () => { |
There was a problem hiding this comment.
[P2] Reasoning-effort mapping test can never fail
This test only asserts expect(["low", "medium", "high", undefined]).toContain(effort) for each fixture model, which is satisfied by the return type itself — it passes even if reasoningEffort returned a wrong value or a constant. TEST_MODELS only exercises xhigh (glm-5.3, defaultLevel "max") and high (kimi-k3), so the minimal → low, low → low, and medium → medium rows of EFFORT_BY_PI_LEVEL in src/engine/tools/asimi.ts are never asserted.
Suggested fix: add explicit per-level expectations (e.g. build models with each defaultLevel/piLevel pair and assert the exact mapped effort) so a mapping regression is caught.
|
@zro-inference rereview |
|
@zro-inference rereview |
zro-inference
left a comment
zro-inference
left a comment
There was a problem hiding this comment.
Adds Asimi as a supported harness: a new src/engine/tools/asimi.ts tool module that configures Asimi headlessly via its OpenAI-compatible provider env vars (OPENAI_API_KEY/OPENAI_BASE_URL + ASIMI_PROVIDER, --model/--provider flags), a preflight check that podman and just are on PATH (with an actionable install message), plus catalog/SUPPORTED_TOOLS/run.ts registration, README and CHANGELOG entries, client- and live-check wiring, and CI steps for the sandbox dependencies. Tests cover the plan shape and absence of config files, the missing-dependency failure, reasoning-effort mapping including the omit-on-unmapped case, install-and-launch, no-auth gateway mode, and the adapter matrix via a shared fakeSandboxBinDir fixture.
No blocking issues surfaced within the reviewed scope. Note for the human reviewer: the automated deep-analysis passes (correctness, security, test-coverage, adversarial) timed out on this PR, so this approval reflects a scope-level read of the diff rather than full independent specialist coverage — an extra human pass on the reasoning-effort mapping and the CI Check live API condition change would be worthwhile.
JenClaude command reference
Reply with @MoonMathJen on this PR to trigger commands:
| Command | Example | What it does |
|---|---|---|
| Re-review | @MoonMathJen rereview |
Re-review after changes (full or scoped — see below) |
| Stop | @MoonMathJen stop |
Cancel in-progress review |
| Stop + summarize | @MoonMathJen stop and summarize |
Post partial findings so far |
| Focus specialty | @MoonMathJen only security |
Review only the named specialty |
| Skip specialty | @MoonMathJen skip test-coverage |
Review everything except the named specialty |
| Change model | @MoonMathJen pr-review-model: kimi-k2.7-coder |
Run this review with a specific model |
| Add context | @MoonMathJen focus on auth edge cases |
Free-text guidance passed to reviewers |
Specialties: correctness, security, test-coverage, adversarial
Any text in the comment beyond a recognized command is passed to the reviewers as context. Use it to point them at areas that matter, explain what changed since the last review, or call out constraints (e.g. "skip generated files", "the auth flow is the riskiest part here"). Hard review rules still win over reviewer guidance.
JenClaude metrics: cost $0.000, time 41m 05s, tokens 105,341.
Introduce support for Asimi, an open-source vi-inspired coding agent, as a new harness. This implementation allows users to launch Asimi using environment conventions without writing config files. The necessary updates include modifications to the supported harnesses list, tool descriptors, and the addition of tests to ensure functionality.