Commit 448b241
[OPIK-8062] [SDK] feat: one-command MCP + skill pack setup (#7958)
* [NA] [SDK] feat: headless MCP configure, install verification, codex + opencode
Half of MCP installs never issue a tool call. The install itself is the
onboarding, and it asked permission it should assume, verified nothing, reached
three of the seven hosts users actually run, and could not be scripted at all.
- Honour `--install-mcp` without a TTY. The interactivity guard was checked
before the explicit flag, so `opik configure -y --install-mcp` silently did
nothing in exactly the environments people automate — and exited 0. `-y` alone
still skips MCP; a blanket yes should not edit another tool's config.
- Add `--host` to `opik mcp configure` (repeatable, or `all`). A terminal was
only ever needed to *ask* which host to use, so naming one lets the command run
from a coding agent, a Dockerfile, or CI. An explicit host installs whether or
not it is detected, so a fresh image can be configured before the editor is.
- Verify before claiming success. The installer wrote JSON and told the user to
go check; an unconfigured server starts happily and advertises every tool, so a
broken setup was indistinguishable from a working one until the agent hit a 401
mid-conversation. Now it makes a real call with the values it just wrote and
reports the workspace and project count, or fails with the reason.
- Add Codex and opencode host targets. Codex is the highest-volume, most reliable
client in the telemetry and was hand-writing JSON. Codex is driven through its
own CLI (its config is TOML, which we will not hand-edit) and read back via
`codex mcp get --json`; opencode gets its own block shape (`local`/`remote`,
argv as one list, `environment`).
- Refuse to guess the workspace. An unnamed workspace makes the server send
`default`, which resolves to the account default — so reads come back from the
wrong place instead of failing. On an account with several workspaces we now
stop and say so. A failed lookup is not treated as evidence of one workspace.
- Name the detected host in the consent prompt, and stop asking twice: the
configurator's prompt and the installer's picker were two questions about the
same thing. Default stays "no".
- Name the exact `uv` install command per platform when `uvx` is missing.
Analytics is deliberately not wired here — it ships on a separate branch.
`ANALYTICS:` comments mark each call site and the event it owes.
Unit tests gained an autouse stub for the new verification call, so the
configurator suite no longer reaches the network (65s -> 2s).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(mcp): document --install-mcp, --host, and the new hosts
The MCP page carried the CLI changes, but three other surfaces were left
stale or silent:
- `sdk_configuration.mdx` documented `opik configure --use_local` and
`--yes` but never mentioned `--install-mcp`, so the flag was only
discoverable by reading `--help`. It now has its own subsection,
including the deliberate `--yes` / `--install-mcp` distinction.
- `home.mdx` and `integrations/overview.mdx` advertised the MCP server as
Claude Code / Cursor / VS Code Copilot only.
No changelog entry: those are cut as weekly batches by the release owner,
and inventing a mid-cycle dated file would fake a release.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(skills): add `opik skills` and wire the pack into `opik configure`
The MCP server gives an assistant tools; the skill pack gives it the knowledge
of how to use Opik. The telemetry says the second half is the gap — 43 installs
have loaded the tool list 14,120 times between them without ever calling a
tool, and `schema` is the most widely-called tool of all. Agents are connected
and groping.
New `opik skills` group (configure / status / remove), plus
`opik configure --install-skills` and an interactive prompt after the MCP one.
The two are asked separately on purpose: MCP writes credentials into a config
file the user already trusts with them, while this writes instruction files the
assistant executes with its own permissions. Same host list, different consent.
No third-party installer is involved. Skills are `SKILL.md` directories and the
assistants have converged on a shared user-level location, so this is a tarball
fetch plus a link:
- Codex resolves `$HOME/.agents/skills` as a user-scope root
(`codex-rs/ext/skills/src/host_roots.rs`, `ConfigLayerSource::User`).
- opencode loads global skills from `~/.agents/skills`, `~/.claude/skills` and
`~/.config/opencode/skills`.
- Cursor and VS Code Copilot read the shared directory.
- Claude Code is the exception — it reads `~/.claude/skills`, so it gets a
symlink into the shared copy (a copy on Windows, where symlinks need
elevation).
So one write plus one link covers every host, with no Node, no `npx`, and no
external CLI whose flags can change under us. It also means the install is
HOME-scoped and independent of the working directory, matching the MCP install
— there is no project to be inside — and needs no Opik credentials, so it works
before `opik configure`.
Extraction is hand-rolled rather than `TarFile.extractall`: the `filter="data"`
argument that makes that safe is 3.12+, and the SDK supports 3.10. Every member
is validated instead — regular files only, no absolute paths, no `..`, size
caps on the archive and each file. A test caught `PurePosixPath(".").parts`
being empty, which made the traversal guard vacuously true and let `.` through
as a skill name.
Version tracking uses a content digest rather than a commit sha: the codeload
tarball names its root after the ref, not the commit, so a sha is not available
without a second request. `~/.agents/skills/.opik-skills.json` records it.
A pack present but unrecorded is reported as installed outside the CLI rather
than ignored, since `npx skills add comet-ml/opik-skills` writes to the same
place; the two are interchangeable. `remove` only touches what we recorded
installing, so a hand-written skill sharing a name survives.
Also flags the one known duplicate: opik-claude-code-plugin ships its own
`opik` skill whose content has drifted from the pack's.
Analytics stays on its own branch; `ANALYTICS:` comments mark the call sites.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(mcp): make the install read like a wizard, and show the plan first
`opik mcp status` renders with rich; the install it tells you to run was a wall
of `OPIK:` log lines with absolute paths wrapping over three terminal lines. The
install is the surface a first-time user actually sees, so it was the wrong one
to leave raw.
The flow now narrates through an injected view. That is not indirection for its
own sake: `configurator.mcp.install` is reachable from `opik.configure()`, which
is a library call and must not take over the caller's stdout. So
`LoggingInstallView` stays the default and preserves today's behaviour, and the
CLI passes `RichInstallView`. Tests inject a recording double, which also
decoupled them from exact log strings — eight assertions that matched on log text
now assert on what the flow *decided*.
What a user sees:
Opik MCP server setup
Deployment Opik Cloud · workspace acme-ai
Connection Local server via uvx, credentials in the host config
Will update
Cursor ~/.cursor/mcp.json
Codex via `codex mcp add`
✓ Cursor Added
✗ Codex Could not register 'opik-mcp': the `codex` CLI was not found …
✓ Verified workspace acme-ai · 7 projects visible
Restart Cursor, then ask: "list my Opik projects"
The substantive change behind the formatting is the **plan block, shown before
anything is written**. The original plan for this work called for it and the
first pass shipped only the default-on flip; consent to edit files owned by
another tool is not meaningful if you cannot see which files. That needed
`_resolve_targets` split into `_candidate_targets` (no prompting) and
`_confirm_targets`, so the paths are known before the question is asked. A test
asserts the plan precedes the write rather than trusting the call order.
Smaller things that were each a papercut:
- Spinners on the three slow steps — the hosted probe, `uv tool install`, and
verification — which previously ran in silence for up to 30s.
- `~` instead of `$HOME`, including inside failure messages, where one absolute
path wrapped over three lines and buried the instruction.
- One grid for all result rows, so the host column aligns. A grid per row aligns
each row against itself and nothing else.
- Results say "Added" rather than repeating the path the plan just showed;
failures keep the full detail, because they need it.
- "Restart Cursor and Claude Code" — and only the hosts that actually succeeded.
- Deployment and transport stated up front, so it is clear which Opik is being
connected and whether credentials are being written to disk.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(cli): pick assistants from a list, and ask which ones for skills
Two things the numbered menu got wrong. It made the user do the label-to-number
mapping themselves, and it gave no feedback until Enter. And for skills we never
asked at all — one yes/no installed into every detected assistant, which is the
wrong default for guidance the assistant then acts on: wanting it in the editor
you use for Opik work does not mean wanting it in every assistant on the machine.
Which AI assistants should the Opik MCP server be set up for?
◉ Claude Code
❯ ◉ Cursor
◯ Codex
↑↓ move · space select · a all · enter confirm
Hand-rolled on stdlib `termios`/`msvcrt` plus `rich`. The alternative was adding
`prompt_toolkit` (via `questionary` or similar) to the core SDK, which is a large
addition to every Opik install for one CLI nicety. Arrow keys, `j`/`k`, space to
toggle, `a` for all, Enter to confirm, Escape/Ctrl-C to cancel — and cancel
returns `None` rather than `[]`, because "I backed out" and "none of them,
deliberately" are different answers and only one of them should skip silently.
Not every terminal can host this: a pipe, a CI log, a platform with neither
key-reading module. `selector.is_supported()` says so and callers fall back to
the numbered menu rather than failing. A single candidate skips the list too —
arrow keys for one item is worse than a yes/no.
Selection moved onto the view (`choose_hosts`), since which-hosts is a
presentation concern. That surfaced a bug the tests caught: `RichInstallView`
extends the abstract base, so a `super().choose_hosts()` fallback silently
returned `None` instead of the menu. The menu is now a module-level
`numbered_menu()` both views call, rather than something inherited.
Scope: `opik mcp configure` and `opik skills configure` only. `opik configure`
keeps its existing prompts and plain log output for now — it still gets the
logger-backed default view, so nothing about that flow changes.
Also fixed two strings left stale by the native skills rewrite, which claimed the
skill pack needed `npx` and went through a third-party CLI. It does neither.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(mcp): show the assistant picker and per-agent skills selection
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(mcp): report the outcome per host, not how we wrote it
The result column mixed two axes. "Added"/"Updated" said whether the entry was
new or replaced; "Registered" said we drove the host's own CLI (`claude mcp add`,
`codex mcp add`) instead of writing the config file. Reading
✓ Claude Code Registered
✓ Cursor Added
there is no way to tell what the difference is, and the mechanism is already
stated in the plan block one line above ("Claude Code via `claude mcp add`").
Worse, "Registered" hid new-vs-updated for exactly the hosts that go through a
CLI, because `claude mcp add` and `codex mcp add` cannot report it — we remove
first to keep re-runs idempotent, which erases the evidence.
So read before writing. Both CLI paths now check for an existing registration
first, using the readers `opik mcp status` already relies on, and every host
reports one thing:
✓ Claude Code Added → second run: Updated
✓ Cursor Added → Updated
✓ VS Code Copilot Added → Updated
The JSON-file read was extracted out of `read_registered_block` so the installers
can reuse it rather than duplicating the parse.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(skills): add `opik skills update`, and record hosts for re-linking
`configure`, `status` and `remove` existed; there was no way to pull a newer pack
short of removing and reinstalling. `update` compares the downloaded pack against
the content hash recorded at install time and rewrites only if it actually moved,
so running it on a schedule costs one request and no disk churn.
It also removes skills the pack has dropped. That matters concretely: upstream
`opik-mcp` currently ships `evaluate` where the published mirror still ships
`instrument`, and when that sync lands an update that only added files would leave
the old skill behind for the assistant to keep reading.
$ opik skills update
Opik skill pack: already up to date (771d787d0b1e).
$ opik skills update
Opik skill pack updated to 3f9c1a02bb71.
added: evaluate
removed: instrument
Restart your AI host to pick up the change.
The manifest now records which assistants were chosen, so `update` re-links the
same set instead of guessing. Manifests written before that field fall back to
inferring from the links already on disk, so an existing install still updates.
Docs gained a warning that assistants read skills at session start: installing
into a running session does nothing visible until a new one starts, which reads
as a failed install when it isn't one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(mcp): set up the server and the skill pack in one command
`opik mcp configure` now installs both by default, and asks once. Two separate
commands with two separate consent prompts made the user assemble the offering
themselves, when the tools and the knowledge of how to use them are two halves of
the same setup.
What should Opik set up?
❯ ◉ MCP server read traces, log scores, run experiments from chat
◉ Skill pack teaches your assistant how to instrument code
↑↓ move · space select · a all · enter confirm
Which AI assistants should Opik be set up for?
❯ ◉ Claude Code
◉ Cursor
◯ Codex
A selector with both pre-selected rather than one merged yes/no. It gets the
single decision and the both-by-default the merge was after, while keeping it
visible that two different things are being written: the server puts credentials
in a config file, the pack puts instructions the assistant then acts on. Enter
accepts both; space declines either; `--no-skills` skips the question.
The assistants are chosen once and used for both. `setup_mcp_server` now returns
the hosts it actually registered, so the pack installs for exactly those rather
than asking again — and a run where every host failed installs no pack, because
there is no host to install it for. Selecting only the pack falls back to asking,
since there is no server step whose answer can be reused.
The plan block names the pack when it is included, so the confirmation covers the
whole change rather than half of it.
Also fixed the selector's layout: the title and footer were grid rows, so their
width sized the label column and pushed every hint far to the right. They are
rendered outside the grid now.
`opik skills configure` stays as the standalone path — needed for installing the
pack without a server, and for `update` and `remove`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(cli): give `opik configure` the same assistant setup, and stop the log wall
The screenshot of a real run was the argument: `OPIK:` on every line, a wrapped
one-liner listing three hosts and their paths, `claude mcp add`'s own output
("Added HTTP MCP server … / File modified: …") landing unstyled in the middle,
absolute paths throughout, two separate y/N prompts for one decision, and
"restart your assistant" printed twice.
Opik MCP server setup
Deployment Opik Cloud · workspace alexkuzmik-opik2
Connection Hosted server, browser sign-in on first connect
Will update
Claude Code via `claude mcp add`
Cursor ~/.cursor/mcp.json
Skill pack for the same assistants
✓ Claude Code Updated
✓ Cursor Updated
✓ Verified hosted server reachable; sign-in happens in the browser
Restart Claude Code and Cursor, then ask: "list my Opik projects"
`opik configure` and `opik mcp configure` now run the same step, extracted to
`cli/assistants.py` rather than assembled twice. The configurator takes it as an
injected callable, so `opik.configure()` stays a library call that does not
render — it keeps plain-text prompts, and those are now one framed question
instead of two bare ones appended to the log.
Three specific fixes behind the formatting:
- `claude mcp add` and `codex mcp add` output is captured, not streamed. We state
the outcome ourselves, so their own report was noise; on failure its first line
goes into the error detail, which is strictly more than "see output above" gave.
- The closing line is printed once for the whole step rather than by each half.
- Plan extras are labelled rows ("Skill pack for the same assistants") instead
of a bare string that rendered as a path belonging to no host.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(cli): register the server, then offer the skill pack as recommended
Asking what to install before installing anything made the user decide between
two things they had not seen yet. Running `opik mcp configure` is already the
answer for the server, so it just runs — and the pack is offered afterwards, with
the server's output in view and the assistants already known:
✓ Claude Code Updated
✓ Cursor Updated
✓ Verified hosted server reachable; sign-in happens in the browser
Recommended: also install the Opik skill pack for Claude Code, Cursor?
It teaches your assistant how to instrument code with Opik, wire up
integrations, and run test suites. [Y/n]:
Marked recommended and defaulted to yes. The server gives an assistant the tools
and the pack gives it the knowledge of how to use them, and the telemetry says
the second half is where installs stall — 43 installs have loaded the tool list
14,120 times between them without ever calling a tool.
Two consequences of asking after rather than before. The offer names the hosts the
server actually reached, not the ones requested — so a run that failed on Codex
does not offer a pack for Codex. And a run that registered nothing offers nothing,
because there is no assistant to add a pack to.
`--skills` / `--no-skills` still answer it up front for scripts.
This removes the component selector added a commit ago, along with the
`plan_extras` plumbing that existed only to announce the pack before it had been
agreed to. The plain-text path in `configurator` mirrors the same shape: the
server question, then a recommended follow-up, rather than one merged question.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): stop relisting the assistants in the skill-pack question
"also install the Opik skill pack for Claude Code, Cursor, VS Code Copilot?" put
three names between the reader and the actual question, and the results table
directly above it had just listed the same three.
✓ Claude Code Updated
✓ Cursor Updated
✓ VS Code Copilot Updated
Recommended: also install the Opik skill pack?
The hosts are still what the pack installs for — that comes from the server step's
return value, not from the prompt text. Same trim on the plain-text path in
`configurator`, where the server question named them a few lines earlier.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): separate representation from configuration logic, and stop two aborts
Three problems, one root cause: the skills installer was deciding both what to do
and how to say it.
**Layering.** `configurator/skills/install.py` was building presentation objects
and choosing wording, with `LOGGER.info` prose interleaved. It now returns an
`InstallResult` — skills written, where, what linked, what failed, whether the
Claude Code plugin overlaps — and the callers word it. `cli/assistants.py` renders
it through the rich view; `configurator/configure.py` logs a diagnostic summary,
which is what a library should do. No `LOGGER.info` and no view references remain
in the installer. `_link_for_host` returns `(linked, failure)` instead of logging
prose, and the plugin check is a predicate rather than a warning.
**Two non-interactive aborts.** Both reached `click.confirm`, which aborts on EOF:
$ opik mcp configure --host cursor < /dev/null
...
Recommended: also install the Opik skill pack? [Y/n]: Aborted!
That exited 1 *after* the server had already been registered — the worst possible
place to die. `opik skills configure` with no `--host` and `opik skills remove`
without `-y` did the same. All three now check for a terminal first and either
skip with a note naming `--skills`, or fail with an actionable message. Verified by
running every command with stdin closed; the full matrix is exit 0 or a clean
error, no `Aborted!`.
(`opik configure` with no terminal still aborts in its deployment-type prompt.
That reproduces on `main` and is left alone.)
**A "Done" block** closes the run, so it ends with a statement rather than the
last log line:
✓ Done
Set up MCP server and skill pack
For Cursor
Next Restart it, then ask "list my Opik projects"
Docs: the page now opens on what this unlocks and four copy-paste prompts to try
straight after setup, with the mechanics moved below. The first prompt exercises
the whole loop — the pack tells the assistant how to instrument, the server lets
it verify the traces landed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): honour --install-mcp without a terminal (PR review)
Reported on the PR, and correct: with `install_mcp=True` and no `--host`, nothing
had prompted, so `assume_confirmed` stayed False and `_confirm_targets` fell into
the numbered menu — where `input()` raised EOFError and failed a run that should
have installed.
EOFError: EOF when reading a line
... in numbered_menu
raw = [token.strip() for token in input(prompt).split(",") ...]
Reaching that point without a terminal can only mean the caller opted in through
a flag, because every interactive path refuses earlier. So the flag is the
consent, and the candidates are used as-is.
This is the same class as the two aborts fixed in the previous commit — a prompt
reachable from a headless path — and the third instance of it, so the regression
matrix now covers every command with stdin closed. All are exit 0 or a clean
error; none reach a prompt.
Also from the review: `opik configure --install-mcp` was documented as the
headless entry point, and it is not — `opik configure` asks its own deployment
and workspace questions first, which abort with no terminal (that reproduces on
`main` and is out of scope here). The docs now point at `opik mcp configure`
with `OPIK_API_KEY` / `OPIK_WORKSPACE` for CI, and two stale snippets showing the
old prompt wording and closing line are updated.
Test note: pytest runs with stdin detached, so an autouse fixture defaults the
suite to "has a terminal" and the headless cases opt out. Without it the new
guard silently changed what every prompt-driven test was exercising.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): make Enter register the highlighted assistant, not every one found
Reported from a real run: the user moved the cursor to Claude Code, pressed
Enter, and all three detected assistants were registered. `opik mcp status`
then listed Claude Code, Cursor and VS Code Copilot — all three config files
written in one pass.
The cause was pre-ticking every detected host and having Enter confirm the
ticked set, ignoring the cursor. A cursor sitting on a checkbox list reads as a
radio list to plenty of people, so "move to the one I want, press Enter" is a
reasonable way to mean that one — and it silently edited three other tools'
configs instead.
Nothing is pre-ticked now, and Enter on an empty set takes the highlighted row,
so both mental models land on the same result:
Enter on Claude Code -> claude-code
down, Enter -> cursor
space, down, space, Enter -> both
a, Enter -> all three
Escape -> nothing
The footer states what Enter will take rather than leaving it implied:
"enter confirm (Claude Code)" / "(2 selected)" / "(all)".
`opik configure` now also asks before any of this happens. It went straight into
the host picker, so "no, just configure Opik and leave my editors alone" was not
an answerable question. Defaults to no, on the same grounds `opik configure -y`
already refuses to reach into another tool's config — which the all-pre-ticked
default had been quietly contradicting with one keystroke. `opik mcp configure`
does not ask: running that command is the answer.
Prompts and selector titles are singular to match what now happens. Two plurals
stay because they remain true: `opik mcp status` really does report several, and
the `opik.configure()` library path has no picker, so it names every detected
host and registers all of them on one yes.
Contract change: Enter can no longer return an empty selection — Escape is now
the only way to choose nothing. Two tests encoding the old behaviour are updated
and 11 added, covering the reported interaction and each consent path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(mcp): lead with the benefit, cut the reference sprawl
The page had inflated to 694 lines and buried the setup steps under prose. Now
556, reordered so the top reads benefit -> configure -> verify -> use, with
reference material below it.
- Preamble cut from twelve lines to two. "What this unlocks" already carries the
benefit; the MCP-server-vs-skill-pack mechanics were explaining machinery
before the reader had a reason to care.
- Steps moved back up under it, stripped of three pasted terminal transcripts.
The verification guarantee survives as prose, which is the part that means
something; the ASCII of it did not.
- The starter prompts moved to their own section after the steps, each in a
titled block, so they read as four labelled actions instead of four anonymous
grey boxes. Absorbed "Using the MCP server" into the same section rather than
leaving a second usage section 400 lines down, and the tool table came with it
— reworded around what each tool lets the assistant do.
- Dropped the standalone skill-pack section and the scripts/containers block.
Both framed skills and flags as parallel workflows to learn, when the intent is
configure once and get on with it. `opik skills update` survives as two lines
under maintenance, since the pack does go stale and nothing else mentioned it.
Install is now `pip install --upgrade opik` with no floor. A pinned minimum goes
stale every time these commands gain a host or a flag, and a reader already on a
newer version gains nothing from being told one. This also retires the
bump-at-release chore noted on the ticket.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(sdk): make the unit suite pass from a clean checkout
Unrelated to the MCP work on this branch — these 22 failures reproduce on `main`
with everything else stashed. Fixing them here because the branch cannot be
verified green otherwise.
Two independent causes.
`rouge-score` was never declared. Not in `tests/test_requirements.txt`, not in
`setup.py`, not installed by any workflow — so the 18 ROUGE tests in
`test_heuristics.py` could not pass on a clean install, only on a machine that
happened to have the package. Declared as a test requirement.
The other four asserted on `caplog` while the opik logger does not propagate.
`_logging.py` sets `propagate = False`, so the bare pytest fixture captures
nothing: the warning was being emitted the whole time and visible in captured
stderr, only the assertion could not see it.
assert ('span-id' in '')
where '' = <LogCaptureFixture>.text
-- Captured stderr --
OPIK: Span 'span-id' exceeded the per-span size limit of 1.0 MB ...
`tests/unit/conftest.py` already provides `capture_log` for exactly this — it
flips propagation for the duration and yields `caplog`. These four were the only
tests reaching for the raw fixture instead.
Whole unit suite now: 4809 passed, 3 skipped, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(cli): localize the assistant step, untangle the CLI imports
Review pass against the goal that this branch's changes stay in their own
namespaces and leave the existing configure flow alone. Three findings, no
behaviour change — 988 tests in the touched areas pass untouched.
**The existing configurator had absorbed the feature.** `configure.py` carried
both consent policies, the prompt wording and a text helper: ~140 lines that are
about assistants, not about configuring Opik. Moved to a new
`configurator/assistants.py` as pure functions of their arguments — a `Decision`
enum plus `mcp_decision` / `skills_decision` / prompt text. The methods stay as
thin delegations, so all 44 existing test call sites keep working as they were.
Footprint on the pre-existing namespace:
configurator/configure.py +153 -11 -> +108 -20
cli/configure.py +105 -4 -> +99 -4
2.9% of the diff now touches pre-existing configure code, down from 4.1%.
**A duplicated helper, one copy of which crashes.** `_readable_list` existed in
both files and the copies had diverged: the CLI one indexes `names[-1]` and
raises `IndexError` on an empty list, where the other returns `""`. Unreachable
today behind a `len(detected) == 0` guard, one refactor from a crash. Now one
implementation.
**Two tangled CLI edges.** `skills.py` imported the *orchestration* module purely
to borrow a renderer, and `configure.py` reached into an MCP-named module for a
generic console. `cli/mcp_view.py` -> `cli/install_view.py` (it was never
MCP-specific — mcp, skills and configure all use it) and `render_skill_pack`
moved there, which drops both edges; `skills.py` no longer imports `assistants`
at all. Import graph verified acyclic: views are leaves, orchestration depends on
views, commands depend on orchestration.
Left alone deliberately: `mcp.py -> configure.py`, for one
`run_interactive_configure(install_mcp=False)` auto-launch. Breaking it needs a
shared bootstrap module for a single call site, which is worse than the edge.
Also removed dead code — `skills.detected_host_names` and `roots.needs_link`
(zero callers in `src/`, the latter alive only via its own test), and three test
patches left over from when the skills prompt named hosts. After this there is
not one unused symbol in 3047 lines of src. Added 23 tests for the extracted
policy as decision tables, which is what the consent rules always were.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(sdk): green up CI — mypy annotation, formatting, click 8.4 prompt
Three failures on the pushed commit, all reproduced locally first.
`ruff-format` and `mypy` failed on files this branch added but that my local
runs never checked: I had been running the hooks on each step's changed files,
while CI runs them over every file in the PR. `test_verification.py` needed
reformatting and `verification.py:40` was missing a return annotation. Both were
invisible to `git diff --name-only | xargs pre-commit run --files` because the
files were untouched by the most recent commits. Reproduced by running the hooks
over `git diff --name-only $(git merge-base HEAD origin/main) HEAD`, which is
CI's actual scope.
The unit failure was a click version difference, not a code bug.
`test_configure__no_flag__defaults_to_detected_hosts` invoked the command with
no stdin and relied on `click.confirm` returning its default at EOF:
click 8.1.8 -> empty stdin + default=True -> returns True, exit 0
click 8.4.2 -> empty stdin -> Abort, exit 1
CI resolves 8.4.2, my venv had 8.1.8, so it passed locally on every Python
version and failed on all five in CI. One detected host takes the plain
confirmation rather than the picker, so the test now answers it with
`input="y\n"` instead of depending on which click is installed. Verified by
pinning 8.4.2 locally, reproducing the exact single failure, then fixing it —
whole suite green on 8.4.2: 4809 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(cli)!: MCP and skill-pack setup only run in an interactive session
Both steps write into configuration files owned by other tools — `~/.claude.json`,
`~/.cursor/mcp.json`, `~/.agents/skills`. That is not something to do to a machine
nobody is sitting at, so it now happens only in a session the user is present for.
Flags say *which* assistant and *whether the user wants it*; they are not a licence
to write unattended.
This reverses the headless story earlier in the branch, and closes a regression it
had introduced. Measured against the merge-base with a detected host present:
main: configure(install_mcp=True), no tty -> wrote nothing
was: configure(install_mcp=True), no tty -> wrote ~/.cursor/mcp.json
now: configure(install_mcp=True), no tty -> wrote nothing
That was the only behavioural difference this PR had for non-interactive callers;
`configure()` and `configure(-y)` matched main throughout. The branch now matches
main exactly with stdin closed.
The rule lives in `configurator/assistants.py` (`mcp_decision` / `skills_decision`
return SKIP before consulting any flag), with backstops in both installers so a
library caller cannot route around it, and refusals in `opik mcp configure` /
`opik skills configure` that say why instead of aborting. `--host` no longer
implies consent; `opik mcp status`, `opik skills status` and `opik configure -y`
are unaffected.
BREAKING CHANGE: `install_mcp` is removed from the public `opik.configure()`.
It has been there since June (#6959), so callers passing it now get a TypeError.
Removed at the maintainer's request after the compatibility cost was raised: with
the step interactive-only, a flag on a programmatic entry point promises something
it cannot deliver. `install_skills` and `assistant_setup` — both added earlier in
this branch and never released — are removed from the public signature too;
`assistant_setup` was always CLI-internal wiring. All three remain on
`OpikConfigurator`, which is what the CLI now constructs directly.
Docs no longer advertise a headless path, because there isn't one: the MCP page
and `sdk_configuration` both state the terminal requirement and point at Manual
setup for images and scripts.
Tests: 12 cases that encoded "the flag beats a missing terminal" are inverted, a
`TestTerminalRequirementIsAbsolute` class asserts no flag value ever proceeds
without a session, and `skills/test_install.py` gains the autouse interactive
fixture the other suites already had, since it covers installer mechanics rather
than consent. 4817 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(cli)!: rename --host to --ai-client, and say "AI client" throughout
`--host` was the MCP specification's word, not a word users have. The spec's
architecture is Host / Client / Server, where the host is the user-facing AI
application — correct for code implementing the protocol, wrong on a flag. And
the CLI was contradicting itself: "AI host" appeared 33 times in user-visible
text against 15 for "AI assistant", with the two colliding in the same breath —
the picker asked "Which AI assistant…?" directly above a flag called `--host`.
Checked what comparable installers actually expose before picking:
MCP spec Host (architecture only, no flag)
Linear docs client "compatible clients"
install-mcp (18 targets) client --client
mcp-add / Smithery / fastmcp client --client
Neon add-mcp agents -a, but its own output column
still reads "MCP Client"
Nobody ships `--host`. `--client` is the de facto standard, so `--ai-client`
keeps that recognisability while disambiguating from an HTTP or API client.
"agent" was considered and rejected: Opik's docs use "agent" 224 times for the
application being *traced*, so "set Opik up for your agent" would read as
"instrument my agent" — the wrong meaning, and the more plausible one in an
observability product.
User-visible text is now "AI client" everywhere; `HostTarget`, `HOST_KEYS` and
`host_keys` keep the spec's vocabulary, because that layer genuinely implements
the MCP host concept. `--host` on `opik proxy` is untouched — it is a network
bind address and has nothing to do with this.
Two corrections that came out of the pass rather than the rename:
- Help text and docstrings still advertised the headless path removed in the
previous commit — "the flag to use from a script, a Dockerfile, or a coding
agent", "usable from CI … nothing is prompted". Those were false as written.
They now say naming a client skips the picker, not the terminal requirement.
- The status header briefly rendered "configured for 1 AI AI client", because
the f-string already prefixes "AI " and the substitution added another.
No deprecation shim: `--host` has zero occurrences on main, so nothing released
ever accepted it. 4817 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(sdk): address live review findings — vacuous test, 5xx verify, ESC key
Triaged all 67 bot review comments. Most were already marked addressed by earlier
commits or superseded by the interactive-only change; four were live defects that
reproduced against current code.
**A security test that could not fail.** `test_read_archive__path_traversal__is_rejected`
ended in
assert set(result.skills) <= {"opik", "escape"} - {"escape"} or True
so it passed whatever the parser returned, including an accepted `../../` entry —
in the test whose whole job is proving traversal is rejected. Replaced with an
exact-structure assertion. The parser turned out to be correct, so nothing was
hiding behind it; it just could have been. Same class as the
`PurePosixPath(".").parts` bug found earlier in this branch, which is exactly why
this one mattered.
**A broken hosted endpoint reported as healthy.** `verify_hosted_endpoint` only
failed on 404, so 400/500/502/503 all returned `succeeded=True` and the user was
told to expect a sign-in prompt. Only the 401/403 challenge proves a working
endpoint now; anything else fails with the status and next step. That undercut the
point of the verification step, which exists so a broken setup does not look
identical to a working one.
**Escape did nothing until you pressed another key.** `_read_key_posix()` followed
a bare `\x1b` with a blind `read(1)`, which blocks — so Escape appeared inert and
then swallowed the next keypress. An arrow key arrives as one burst, so a short
`select()` distinguishes them. Verified through a real pty: bare ESC now returns
`cancel` with no second key. Two unit tests cover `_has_pending_input` over a pipe
rather than relying on a pty.
**CLI tests in the configurator suite.** `TestRichInstallView` and
`TestChooseHosts` imported `opik.cli.install_view` from inside their methods —
invisible to a top-level grep — so the configurator suite depended on the CLI
layer it is meant to be independent of. Moved to `tests/unit/cli/test_install_view.py`;
`tests/unit/configurator/` no longer imports `opik.cli` at all.
Reported but deliberately not changed here, since each is a design decision rather
than a fix: the Codex API key in argv (the alternative is hand-editing TOML, which
this installer avoids on purpose), `codex mcp remove` running before `add` is
confirmed, manifest-name containment before `_remove_path` (needs attacker write
access to HOME already), and an aggregate expansion cap on the skill archive (per-
member and compressed caps already exist).
4823 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): harden the escape-key window, and keep it off Windows' path
Follow-up on the bot's review of the previous commit, which flagged the exact
risk I had left open: a 50 ms `select()` could misread a slowly-delivered arrow
sequence as a bare Escape, cancelling the picker instead of moving the cursor.
That would be worse than the bug it fixed, because it is intermittent.
What actually bounds the risk is the gap *within* one burst, not network latency:
a terminal writes `\x1b[A` in a single write, and SSH delays the whole burst
rather than spacing its bytes out, so the real gap is near zero. The window is
now a named `ESCAPE_WINDOW = 0.12` carrying that reasoning, chosen because the
costs are asymmetric — too large only delays a bare Escape by that much, while
too small turns an arrow key into a cancellation.
On Windows the question does not arise, and the answer is worth recording where
someone will find it: `import termios` fails there, so `_key_reader()` returns
the `msvcrt` reader and `_read_key_posix` — with `_has_pending_input` — is never
called. That reader has no ambiguity to resolve either, because arrows arrive
behind a `\x00`/`\xe0` prefix rather than behind Escape, so a bare Escape has
always cancelled immediately there. The POSIX-only contract is now documented on
the helper so it is not reused on a path where `select()` cannot take a pipe.
Which is also the bot's fourth finding, and a real one: the tests I added call
`_has_pending_input` over `os.pipe()`, and on Windows `select()` accepts only
sockets, so they would error rather than fail. CI is `ubuntu-latest` only, which
is precisely why it went green and I did not notice. Skipped on win32, with the
reason spelled out.
Also hoisted `select` to a module-level import, per the same review.
Not added: a pty test for the ESC-versus-arrow decision. Two attempts hung —
`sys.stdin.read(1)` on a buffered text stream blocks trying to fill its buffer,
and a fork-based harness proved flaky. `_has_pending_input` is the decision point
and has deterministic pipe-based coverage; a flaky test would be worse than none.
4823 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(cli)!: drop `opik skills`, install the pack only in the configure flow
The skill pack is worth installing for people already running `opik configure` or
`opik mcp configure` — that is the whole point, so they never have to reach for a
separate tool. Outside that flow there is already a better answer than a second
Opik command: `npx skills add comet-ml/opik-skills`, which covers 76+ clients to
our five and can install per-project, which we cannot.
So `opik skills configure|update|remove|status` is removed. The pack still installs
as the recommended follow-up inside the MCP step, which is unchanged.
Removing the command orphaned its whole lifecycle, and leaving that in place would
have been dead code: `update_skills`, `uninstall_skills`, `manifest.collect_status`
and friends, `SkillStatus`, `render_skills_status`, plus the helpers only they
reached. All gone, with their tests — 1143 lines out, 19 in. A dead-code sweep over
`configurator/skills` now reports nothing.
That leaves one honest gap, and the docs say so rather than papering over it:
`npx skills` cannot manage a pack Opik installed, because the two write different
directories — Opik the shared `~/.agents/skills` plus a Claude Code link, `npx`
each client's own directory, defaulting to project scope inside a project. So
refreshing is "re-run `opik mcp configure`", which rewrites the pack from the
latest published version, and removing is deleting it from `~/.agents/skills`.
Docs: the MCP page gains a *Clients the CLI doesn't cover* section pointing at
`npx skills` and stating the boundary — five clients natively, npx for the rest.
`sdk_configuration.mdx` no longer references the removed command.
Also verified and cited while researching this: `~/.agents/skills` really is read
by Cursor and VS Code Copilot, which `roots.py` had asserted without a source.
Both now carry one, along with the note that `.agents/skills` *without* the `~` is
a project location Opik never writes.
4779 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(mcp): stop teaching a flag without its values, and fold npx into manual setup
Two bits of the page were pulling their weight the wrong way.
The not-detected note said to "name it with `--ai-client claude-code`", which is
advice a reader cannot act on: nothing on the page says what other values exist,
so the example is either exactly right for you or useless. The flag's values are
in `opik mcp configure --help`, which is the right place for them. The note now
just points at Manual setup, which is what someone in that position actually needs.
The npx escape hatch had grown its own `###` section, which oversold it — it is
one command for a case the CLI does not cover, not a feature of the page. Folded
into Manual setup, where "your client wasn't detected" is already the framing, at
three lines instead of twenty.
541 lines, down from 694 at the start of the branch. All anchors still resolve.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): stop reading arrow keys as cancellation
Reported from real use: pressing the down arrow in the client picker ended the
flow instead of moving the cursor. My own fix two commits ago caused it.
`sys.stdin.read(1)` is buffered. An arrow key's whole `\x1b[B` burst lands in the
text stream's userspace buffer and only the `\x1b` is handed back — after which
`select()` on the *descriptor* correctly reports nothing pending, because the rest
is sitting above the kernel where select cannot see it. So every arrow key looked
like a bare Escape and cancelled.
Mixing a buffered read with a readiness check on the raw descriptor was the
mistake. The reader now uses `os.read` throughout, which keeps the descriptor the
single source of truth: a bare Escape reads as one byte with nothing pending, an
arrow reads as its whole sequence in one call, and a split sequence is completed
by a second read inside the escape window.
The decision is extracted into `_interpret(bytes) -> token`, which is the part
that was untestable before. It is a pure function now, so the regression has real
coverage: reintroducing the buffered read fails 4 of the new tests, and the fix
passes all 58. Unknown sequences (Home, End, F-keys) return "" rather than
CANCEL — an unmapped key must not close the picker either.
Worth recording why the coverage looks like this rather than an end-to-end test:
three attempts at a pty harness were flaky or hung — `rich.live` rendering into a
pty, and `pty.fork()` behaving inconsistently here — and a hanging test is worse
than none. The mocked-descriptor tests assert the shape of the bug (which call the
reader makes, and what it does with a split burst) rather than restating the
symptom, which is what actually guards it.
4799 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(cli): let a coding agent run the configure flow
The flow we want is a user asking their agent to set Opik up. Tested by being one:
in this shell `sys.stdin.isatty()` is False and `is_interactive()` returns False,
so before this change every write path refused. `opik configure` aborted on the
deployment-type prompt — `-y` does not answer it — and `opik mcp configure`
refused outright, `--ai-client` included. Exactly one thing worked,
`opik configure --use_local -y`, and it never reached MCP or skills.
The interactive-only rule from earlier today was answering the wrong question. It
asked "can I prompt?" when the thing that matters is "did the user ask?". Those
come apart precisely here: an agent has no tty but a live instruction, while a CI
runner has no tty and no instruction. Both looked identical to `is_interactive()`.
So the rule is now about intent, and the flag is how intent is expressed:
named flag / client proceed, terminal or not
terminal, nothing named ask
no terminal, nothing named skip
CI stays protected, because a job that names nothing still writes nothing — which
is what the original regression was really about. Verified both directions:
opik configure -y --install-mcp --install-skills config + mcp + skills
opik mcp configure --ai-client cursor --skills mcp + skills
opik configure -y config only
opik mcp configure refuses, names --ai-client
`opik configure` also stopped asking the deployment type when it cannot: the
environment already answers it, so `OPIK_URL_OVERRIDE` and `OPIK_API_KEY` are read
for the shape and only a bare environment errors — naming what to set.
Every refusal on this path now names the remedy, because the caller we are aiming
at reads stderr and retries. `--install-mcp` used to die on whichever prompt came
first with a bare `Aborted!` (the project-name question, as it happens); the
command now fails fast saying to add `-y`. "Run it from a shell" is a dead end for
an agent, so it is gone.
This reverses part of the interactive-only commit, and 14 tests that encoded it
are inverted back with their names and reasons updated. Added coverage for the
flow itself: deployment inference per environment shape, the flag reaching the
installer as consent, an unflagged no-terminal run reaching nothing, and the
guard naming `-y`.
Not addressed, and worth a follow-up: a rejected cloud API key surfaces as
"API key missing" rather than "invalid", which is misleading when the key was
present but wrong. Pre-existing, and out of scope here.
4809 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): make the agent-facing path discoverable, not just possible
The previous commit made the flow work but assumed the agent already knew the
invocation. Walking it as one showed the real failure was not a bad error, it was
a silent success:
$ opik configure
Error: ... Add `-y` to accept the defaults
$ opik configure -y
OPIK: Configuration completed successfully. <- and no MCP, silently
`-y` is exactly what the first error told it to add, so that is the path an agent
takes — and it configured Opik, said it had succeeded, and wrote nothing to the AI
client. An agent asked for both would report done having delivered half. A wrong
answer that looks right is worse than the abort it replaced.
So the skip now says so, and names what to add:
Skipped AI client setup: nothing named it, so nothing was written to your AI
client's config.
To include it: opik configure -y --install-mcp --install-skills
Only without a terminal: someone who typed `-y` chose this, an agent that was told
to add `-y` did not.
Both `--help` texts now carry the non-interactive recipe, since reading help is
what an agent does before guessing. The whole walk is three steps, each output
naming the next:
opik configure -> add -y
opik configure -y -> add --install-mcp
opik configure -y --install-mcp -> done
Tests cover the announcement firing without a terminal, staying quiet with one,
and `--help` carrying the flags.
4812 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(cli): assume the defaults when there is no terminal
You asked why it took so many steps. It didn't need to. `-y` existed to say "yes,
the defaults" — and with no terminal there is nobody to ask, so requiring it was a
step that existed only to be discovered. Worse, the error teaching it was the step
an agent was most likely to stop at.
Without a terminal the defaults are now assumed, which collapses the walk to one
command:
before: opik configure -> error, add -y
opik configure -y -> succeeded, no MCP
opik configure -y --install-mcp -> done
now: opik configure --install-mcp --install-skills -> done
Broader than asked, and deliberately: implying `-y` only from the assistant flags
would have left `opik configure` alone still erroring, which is the same
discovery step one command further along. The questions being defaulted are
"use the local instance we found" and "keep the project name we derived", and
neither has a second sensible answer when nobody is there to give one.
A terminal changes nothing: `automatic_approvals` is still just `-y` there, so a
person keeps every prompt they had. Asserted both directions rather than only the
new one.
What is *not* defaulted is the part that writes outside Opik: `opik configure`
with no flags still touches no AI client config, and still says so with the
remedy — now without the `-y` that is no longer needed. `opik mcp configure` with
no client named still refuses, because there the missing piece is *which* client,
which has no default.
4812 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): tell a person too when `-y` skipped their editor
Answering "what does -y do with mcp and skills now": nothing, and that was quietly
true in a terminal as well.
opik configure -y (terminal) -> Opik configured, no MCP, no message
`-y` does not install the MCP server or the skill pack and never has — it answers
Opik's own questions, and writing into another tool's config needs naming. But
`-y` reads as yes-to-everything, so someone who types it chose "stop asking me",
not "skip my editor", and got no hint that half the thing they expected did not
happen.
The skip announcement was gated to no-terminal runs on the reasoning that a person
who typed `-y` chose this. They didn't — they chose not to be asked. Same
surprise, same one line, now shown in both modes.
Also worth recording from checking this: with no terminal, `-y` is now a complete
no-op, because the defaults are already assumed. Every `-y` row in the matrix is
identical to the row without it. It stays supported so existing scripts keep
working, but it no longer buys anything there.
4812 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): match the Comet host, not a substring of the URL
CodeQL flagged this on the deployment inference added two commits ago —
`py/incomplete-url-substring-sanitization`, high severity, and correct:
url_helpers.get_base_url(url).rstrip("/").endswith("comet.com")
`evil-comet.com` ends with `comet.com`, so a self-hosted deployment on a
lookalike host was classified as Opik Cloud and configured against the wrong
place. Parsing the hostname and requiring the dot to be a real label boundary is
the check that was meant:
host == "comet.com" or host.endswith(".comet.com")
Verified against the cases the old form got wrong — `evil-comet.com`,
`comet.com.evil.net`, and `comet.com` appearing only in a path or query all now
resolve to self-hosted or local, while `comet.com`, `www.comet.com` and
`staging.comet.com` still resolve to cloud. Case is normalised too, since
hostnames are case-insensitive and the old form was not.
Worth noting what this was and was not: the misclassification pointed *at* real
Opik Cloud rather than at the attacker's host, so it was a correctness bug
before it was an exposure — but a hostname check written as a substring test is
wrong either way, and it is the kind that grows teeth when someone later reuses
it to decide what to trust.
Also in this commit: the merge of origin/main. Its only conflict was
`test_payload_truncation.py`, where main had independently made the same
`capture_log` fix and additionally asserted the field name — theirs kept, since
it is strictly stronger than mine.
4904 passed, 3 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(cli): report what the configuration flow was asked for and what it did
Adds a `configuration` analytics component covering `opik configure` and
`opik mcp configure`, which were previously invisible: every existing MCP
dashboard tile starts at the server already being installed.
Each command reports a pair of events — entry with what was asked for, exit
with what was actually written (`clients`, `skills`). The gap between the two
is the drop-off, which is the number the flow most owes us.
Both are reported from the click command rather than the installers underneath.
Analytics drops an event whose immediate caller is a different `opik` module,
so a configurator called from `opik.cli` reads as Opik calling itself; and a
reporter nested inside an already-reporting stack is dropped too, which rules
out a nested funnel. Sequential siblings from the same frame both survive,
which is what the entry/exit pair relies on. The assistant step therefore
returns its outcome instead of reporting it, and `run_interactive_configure`
recovers it through a recorder — the configurator takes the step as a callback
and discards its return value.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(cli): keep configuration event property types stable across the pair
`skills` carried a tri-state string on the entry event and a bool on the result
event. One property key holding two types breaks breakdowns and contradicts the
project's own analytics convention, so the two are named apart: entry reports
`skills_requested`, the result reports `skills_installed`.
`clients` becomes `clients_written` for the same reason it is worth a longer
name — the entry event already has `client_count`, which counts clients *named*
on the command line rather than clients actually written.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(cli): decide the assistant setup steps in one place
`opik configure --no-install-mcp` registered the MCP server anyway and installed
the skill pack unprompted. The line at fault was real, but it was a symptom: the
consent policy existed twice. The tested decision table governed only
`opik.configure()`, while `opik configure` ran its own ad-hoc ladder that never
called it — so the two could and did disagree.
Five decision sites become one `configurator.consent.resolve`, shared by both
surfaces. Its six rules are the whole policy, and it returns why alongside what,
so callers explain themselves instead of guessing.
`-y` and "no terminal" stop being conflated. The command still hands `-y` down
whenever there is no tty, but the resolver takes them separately and reports
NO_TERMINAL first, so an unattended run is no longer told that a flag it never
passed is why its editor was skipped.
The two halves of the step are now independent. `setup()` registered the server
as its first act whatever it was asked for, which is why "skills but not the
server" had to be faked by a call that forced `skills_flag=True` — overriding
`--no-install-skills` on the way. The pack's targets fall back to detected
clients, so either half can run alone.
Deleted rather than added to: the CLI ladder and its unreachable branch,
`_confirm_assistant_step`, `_wants_skill_pack`, both `*_decision` functions, a
dead `_mcp_prompt` test seam no test called, three unused loggers, and a prompt
argument threaded into an asker that ignored it.
Behaviour change worth noting: an explicit skills request with no client detected
used to skip in silence, and now reaches `setup_skills`, which names the
locations it knows.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(cli): stop the assistant installers damaging what they touch on failure
Six findings from review, each a case of a failure path doing more harm than the
operation was worth.
Writes to a client's config are atomic. `merge_server_into_json_file` truncated
before writing, and `~/.claude.json` is the whole of Claude Code's user state —
a full disk or a signal mid-write left the user's editor unparseable, while the
caller caught OSError and printed a tidy message. It now writes a sibling temp
file and renames. A file we create is 0600 because it holds an API key; one that
already existed keeps the mode its owner chose.
The client CLIs cannot raise out of configure. `shutil.which` only checks the
executable bit, and `claude`/`codex` are Node shims, so node moving out from
under them passed `which` and then raised FileNotFoundError at exec — an
unhandled traceback. All four write calls now go through one guarded runner that
also closes stdin and sets a timeout, since they inherited the terminal and a
CLI that decided to prompt would wait forever. The errno text names node rather
than the shim, so the message explains that itself.
The skill pack's archive is capped while streaming. The limit was checked after
`response.content` had already materialised the body, so it could only report
the memory it existed to prevent.
`write_skill` refuses a name that is not a direct child of the destination. It
goes on to `rmtree` what it resolves, and the name comes from the downloaded
archive: an empty name resolves to the root itself and `..` escapes it. Verified
by reverting the guard — the destination really is damaged without it.
The pack download uses a plain httpx client, not Opik's factory, whose hooks
exist to decorate calls to the Opik API and would otherwise be free to add
headers to a GitHub request.
The uv prefetch runs `uv tool run opik-mcp --help` instead of `uv tool install
opik-mcp`, which was building a persistent tool environment and putting a shim
on the user's PATH — an install nothing announced. Its output is captured, since
the caller wraps it in a rich status spinner and the two were overwriting each
other, and it has a timeout.
Also: the printed recovery hint said `--host`, which has been `--ai-client`
since 17eb5a001e, so following it failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(cli): drop the analytics markers the installers cannot act on
Thirteen `# ANALYTICS:` comments named events to emit from inside the MCP and
skills installers. They cannot be emitted there: reporting drops any event whose
immediate caller is a different `opik` module, so an installer called from
`opik.cli` reads as Opik calling itself, and a reporter nested inside an
already-reporting stack is dropped too. The flow's events are reported from the
click commands instead.
That left the markers describing work that will not happen where they sit, which
is a placeholder TODO wearing a comment's clothes.
What they asked for and the events do not yet carry: the reason a run stopped
early — declined, no client detected, uv missing, ambiguous workspace — and
per-client success. The result events say how many clients were written and
whether the pack landed, so a zero is visible but unexplained. Threading a reason
up to the command that can report it is a separate change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(cli): follow a symlinked client config instead of replacing the link
Three defects in the atomic-write change, two of them regressions it introduced.
A symlinked config was replaced rather than followed. Dotfile managers (chezmoi,
stow, yadm) routinely symlink editor configs into a tracked repo, and
`os.replace` on the link swapped it for a regular file — so the tracked file
never received the change and the link was gone. Worse than it sounds: the write
landed nowhere the user would look, and their next dotfiles sync would clobber
it. The `write_text` this replaced wrote through the link, so following it
restores the previous behaviour.
`os.chmod` was handed the `mkstemp` descriptor. That only works where
`os.chmod in os.supports_fd`, which is false on Windows — which the SDK
supports, and where this would have raised instead of preserving the mode.
Applied by path now, and after the write rather than before, so the file holding
the key is never wider than…1 parent 3652e65 commit 448b241
47 files changed
Lines changed: 7797 additions & 341 deletions
File tree
- apps/opik-documentation/documentation/fern/docs-v2
- integrations
- prompt_engineering
- tracing/advanced
- sdks/python
- src/opik
- analytics
- cli
- configurator
- mcp
- skills
- tests
- unit
- cli
- configurator
- mcp
- skills
- evaluation/suite_evaluators
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
| 25 | + | |
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
| |||
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
63 | | - | |
| 63 | + | |
64 | 64 | | |
65 | 65 | | |
66 | 66 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
18 | | - | |
19 | | - | |
| 18 | + | |
| 19 | + | |
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| |||
0 commit comments