Release: staging → main (179 commits) - #420
Draft
mindsdb-release-train[bot] wants to merge 179 commits into
Draft
Conversation
…skills The 15.3k-char BACKEND_GENERATION_PROMPT and 10.7k-char VISUALIZATIONS_HTML_OUTPUT_FORMAT_PROMPT were re-sent in the system prompt on every LLM call. They now ship as read-only built-in skills (build-fullstack-backend, build-html-dashboard) served by SkillStore and are recalled on demand; the always-sent prompt carries short mandatory recall hints instead. create_artifact/launch_backend descriptions reinforce the recall. System prompt drops from ~43.5k to ~22.6k chars. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e source of truth for HTML contract - load() falls back to the built-in when a same-label user dir exists but is unreadable (broken shadow no longer dead-ends a mandatory contract), and logs when a user skill shadows a built-in label. - recall_skill embeds a stable marker in its payload; repeat recalls return a short stub while the body is still visible in history, and re-send the full procedure if compaction evicted it. - build-fullstack-backend step 5 no longer references the deleted VISUALIZATIONS prompt section — it recalls build-html-dashboard, the single source of truth for dashboard HTML (inline defaults only as fallback). - Hygiene: revert unrelated uv.lock resync; provenance comment/docstring/ developer docs mention built-ins; list_all/list_summaries share one _iter_skill_dirs walk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The idempotence stub embedded the same marker _already_in_history matches on, so a stub surviving compaction (while the full body was evicted) suppressed re-sends forever. Detection now requires the marker AND the procedure header in the same message (only the full payload has both, ensure_ascii=False so the em-dash header actually matches), and the stub carries neither. Regression tests: surviving stub and marker-quoting summary both trigger a full re-send. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… publish_or_preview
…e scratchpad (ENG-824) On a host whose default code page isn't UTF-8 (GBK/cp936 on Chinese Windows, and likely other CJK locales) LocalScratchpadRuntime crashed before any cell ran: `_BOOT_SCRIPT_PATH.read_text()` decoded the boot script (which contains `…`/`—`) with the locale default → `'gbk' codec can't decode byte 0xa6`. Fix, at every parent↔child byte boundary + interpreter-level: - Read/write the boot script as UTF-8 (read_text/encode). - Force UTF-8 mode in the subprocess env (PYTHONUTF8=1 / PYTHONIOENCODING=utf-8, via _utf8_env, setdefault so an explicit override wins) — so the child's file I/O and stdio are UTF-8 regardless of host locale. - Decode/encode the cell payload + stdout/install output as UTF-8 (errors="replace" on display output so odd bytes never crash the reader). Non-breaking: this content is already UTF-8 on the wire, so UTF-8-default hosts (macOS/Linux/English Windows) are unchanged; it only turns the hard crash into working on non-UTF-8-locale hosts. Tests: _utf8_env forces UTF-8 / respects overrides; the boot script must be read as UTF-8 (its bytes are not GBK-decodable). 96 existing scratchpad tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…view, ENG-824) Self-review: a bare PYTHONIOENCODING=utf-8 downgrades the child's stdio error handler from surrogateescape → strict (verified), which adds nothing over PYTHONUTF8=1 (already utf-8 for open()/filesystem/stdio) and re-introduces a strict-encode crash on exotic output. Keep only PYTHONUTF8=1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…elf-review, ENG-824) Skill review: the main scratchpad subprocess got _utf8_env but the dependency install subprocess didn't, so on a non-UTF-8 host locale pip/uv output could come back as mojibake. Pass env=_utf8_env(os.environ) here too for consistency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ta (folder-aware) [ENG-844]
ENG-847: Fix scratchpad web_search() on the minds-cloud gateway
…pt read (PR #253 review, ENG-824) Address Zoran's review on #253: - _setup_parent_site_packages wrote _parent_venv.pth with a plain open() (host- locale encoded) while the child reads .pth as UTF-8 under UTF-8 mode — same class of bug as the boot script. Write it as encoding="utf-8". - Extract _read_boot_script() and add a test that spies on Path.read_text to assert the boot-script read passes encoding="utf-8" — the previous bytes-only test would still pass on UTF-8 CI if the explicit encoding were dropped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…w, ENG-824) Self-review of the review-response commit: I pinned the boot-script *read* as UTF-8 but left the sibling .pth *write* fix untested — asymmetric with the exact concern Zoran raised. Add a regression test that spies on open() and asserts the _parent_venv.pth write passes encoding="utf-8" (bypasses the heavy __init__; verified it fails if the encoding is dropped). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d-utf8 ENG-824: force UTF-8 in the scratchpad so non-UTF-8 host locales (GBK/CJK Windows) don't crash
feat(prompts): move backend + HTML-dashboard contracts into built-in skills
feat(publish): add access modes (password/restricted) to /publish and…
…ent errors (ENG-673) (#246) * fix(llm): back off + retry mid-stream provider failures; typed transient errors (ENG-673) A mid-stream overload arrives inside an HTTP-200 stream (the SDK raises APIStatusError with status_code=200, real reason in .body), so anton's status-only classifier surfaced the nonsensical "Server returned 200 — the LLM endpoint may be temporarily unavailable" and the session loop retried it instantly with zero backoff — burning all attempts within seconds of a minutes-long incident (BUG-CM-001, Anthropic incident 2026-07-08). - New `TransientProviderError` / `ProviderOverloadedError` + a shared `classify_transient` in provider.py. Classify by BODY, not status: overloaded/api_error, 5xx, plain-429, connection drops, truncated streams. - anthropic.py: refactor the two byte-identical status-only blocks into a shared `_raise_for_status_error` mirroring the ENG-598 openai mapper; openai.py: extend that mapper with the transient branch (covers all four paths). - session.py: budget-bounded backoff-and-retry (30s/turn, cancellation-aware, jittered ~2/10/18s) for the mid-stream case that had NO prior retry; on exhaustion raise ProviderOverloadedError (carries model+provider) for the cowork-server/cowork `provider_overloaded` card. Completed tool_results are never re-executed on retry (idempotency) — only dangling tool_use is sealed. - Split by prior-retry: request-time 5xx/429/connection errors (already SDK-retried) and truncated streams carry session_backoff=False — honest typed message, but fail fast instead of stacking another 30s. - Log the (scrubbed, via ENG-583 scrub_credentials) error body on every transient occurrence. Tests: tests/test_transient_retry.py (classifier, both mappers, backoff helpers, turn-level recovery / budget-exhaustion / cancel / no-replay). Updated the two ENG-598 mapper tests (429/500 now typed-transient) and the two e2e error-handling tests (honest message, fast fail). Full suite green except the 2 pre-existing environmental scratchpad-subprocess failures. Part 1 of 3 for ENG-673 (cowork-server + cowork companions to follow). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(llm): address adversarial-review findings on the transient-retry path (ENG-673) Self-review of the 3-PR stack surfaced four issues; fixing them here (anton side): - #1 (was a real regression): truncation detection raised whenever a stream ended with no finish_reason/stop_reason — but many OpenAI-compatible endpoints simply don't report one, so a COMPLETE, good answer was being discarded and turned into an error (and would fail every turn for such a provider). Now only the truly-empty case (no content AND no tool_calls) is treated as truncated; a content-bearing stream without a terminal marker is logged and passed through. - #3: user-stop DURING backoff re-raised the TransientProviderError, surfacing a provider-error card instead of a clean cancellation. Now it breaks cleanly (like a normal stop). - #4: ProviderOverloadedError always named the planning model even when the CODING model was the one that failed. TransientProviderError now carries the in-flight `model` (threaded through classify_transient + both providers' raise sites); the card names the actual failing model, falling back to planning. Tests updated for the new clean-cancel semantics + 2 new (model propagation, failing-model-not-planning). Full suite green (bar the 2 pre-existing environmental scratchpad failures). (#2 — an overstated "graceful degradation" claim — corrected in the PR #246 description, no code change.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(llm): regression guards for the truncation fix — content-without-finish_reason passes through, empty stream truncates (ENG-673) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(session): don't tell the model to "adjust your approach" on a provider blip (ENG-673 #6) A request-time TransientProviderError (5xx / rate-limit / dropped connection) reaching the count-based retry path was injected as "An error interrupted execution… adjust your approach to avoid the same error" — but that's a service hiccup, not the model's fault, so the note misattributes the failure and can degrade the next attempt mid-incident. Transient errors now get a neutral note ("a transient service issue, not a problem with your approach — continue as planned"); genuine errors keep the original recovery guidance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(llm): recover mid-stream connection/APIError failures; keep empty-truncated fail-fast (ENG-673) Review-round fixes (Sam/SailingSF, anton#246): - session_backoff now means "did the SDK already retry?" — mid-stream failures (connection drop after the 200, read timeout) back off within the budget; request-establishment failures still fail fast. Adds a stream_started flag on all three streaming paths (anthropic, openai chat.completions, Responses API). - Catch the bare openai.APIError a mid-stream SSE error raises (it is NOT an APIStatusError) so the OpenAI/MindsHub path classifies + backs off instead of leaking a generic error; classify_transient now reads both the Anthropic (nested) and OpenAI (unwrapped, top-level) body dialects. - Empty-from-start truncated stream stays fail-fast: a broken/misconfigured endpoint must not loop the 30s budget (product decision; carved out of the ticket's "truncated -> recover"). - Real-SDK mock harness (tests/test_transient_retry_e2e.py) over httpx.MockTransport; resolves the dangling test_transient_retry_e2e reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ENG-742: Count turns without tools-calls
…-independent UTF-8) (#263) * fix(scratchpad): read/write scratchpad + chat files as explicit UTF-8 (ENG-940) Completes ENG-824's Fix #2 ("suspenders") that the belt-only fix left undone. The scratchpad/chat-path reads relied entirely on PYTHONUTF8 being set by the launcher, so any path that misses it (bare CLI, provisioned/Docker cowork, OpenClaw) re-crashed on a GBK/CJK Windows host at `code = script_path.read_text()`. Add explicit encoding="utf-8" to every text read/write in that path so they're launcher-independent (belt AND suspenders, as ENG-824 specced): - chat.py: script read (the root-caused crash site), .env read + append, published/legacy/pub_file JSON read + write. - core/backends/local.py: .python_version + requirements.txt read + write. Tests (tests/test_scratchpad_utf8.py): a real non-ASCII fixture proves the explicit UTF-8 read round-trips while a host-locale (GBK) read crashes-or- corrupts (payload-independent); a regression guard fails if any `.read_text()` in anton/chat.py drops encoding=. Both independent of PYTHONUTF8. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(scratchpad): surrogatepass the cell-payload encode — encode-side sibling of ENG-824 (ENG-940) Follow-up from ENG-940's new evidence (users sabrina/eddie/janis): a non-ASCII Windows path (pt-BR "Área de Trabalho", emoji filename) is surrogate-escaped into lone surrogates (\udcXX) when decoded on a non-UTF-8 host. When that string reaches the strict UTF-8 encode of the cell payload sent to the subprocess, it raises "UnicodeEncodeError: surrogates not allowed" and kills the whole session — the encode-side sibling of ENG-824's decode crash. - local.py: extract _encode_cell_payload() using errors="surrogatepass" so the host-side encode can't crash when the host isn't in UTF-8 mode; the subprocess (always UTF-8 mode) decodes the payload fine. - The two chat.py JSON writes need NO change: json.dumps defaults to ensure_ascii=True, so surrogates become ASCII \uXXXX escapes and never reach the encoder (verified). - tests: pin the surrogate-safe encode (strict raises, surrogatepass round-trips) and a broadened accented-Latin + emoji case that must pass through unmangled. Verified the surrogate test fails if the helper reverts to strict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(scratchpad): use surrogateescape (not surrogatepass) for the cell-payload encode (ENG-940 self-review) Adversarial review of de6d6fb caught a real bug in my own fix. surrogatepass does NOT round-trip through the subprocess: the subprocess always runs in UTF-8 mode, so its stdin decodes with surrogateescape — and surrogatepass emits the 3-byte CESU form that surrogateescape then re-mangles (\udc81 -> three surrogates), so the path would not survive intact. surrogateescape is correct on both counts: it's the inverse of the os.fsdecode that created these lone surrogates (U+DC80..U+DCFF), so it restores the original path bytes, and it matches the subprocess's surrogateescape stdin decode — the path arrives verbatim. Verified end-to-end. Also fixes the test, which previously asserted a false surrogatepass/surrogatepass symmetry; it now decodes with surrogateescape (what the subprocess actually does) and genuinely discriminates — it fails if the helper reverts to surrogatepass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…1992) (#405) Adds ContentValidationError: raised when the provider permanently rejects a request because a content block in history reached it in a shape it doesn't parse (e.g. an image block built for the wrong provider) -- not a provider-availability issue. This used to fall through to the generic "Server returned 400 -- the LLM endpoint may be temporarily unavailable. Try again in a moment." ConnectionError, which is actively wrong here: retrying the identical request fails identically every time, since the same translation runs fresh from stored history on every call. Two dialects recognized in _raise_for_status_error: OpenAI Responses' "Invalid value: 'x'. Supported values are: ..." (the offending content index rides the `param` field) and Anthropic's "Input tag 'x' found using 'type' does not match any of the expected tags". cowork-server's turn-error mapping (companion PR) detects this type -- or its scrubbed class name on the remote/pod path -- and repairs the offending content in the conversation's stored history so the next turn doesn't resend the same poison.
* fix(build): the pod image reports the version it was built from (ENG-1796) `Dockerfile` hardcoded `SETUPTOOLS_SCM_PRETEND_VERSION=2.0.0`, so every turn a scratchpad pod ever served reported `anton_version=2.0.0`. Not a fallback firing occasionally — the only value cloud ever reported, and re-measured at 46% of the install population on 2026-08-26 (up from 35% six days earlier). Two consequences, both silent. A version breakdown read `2.0.0` as a legitimate cohort rather than as a null, because it is a well-formed release number. And bumping the pod image pin produced no observable change at all, so "did the deploy land?" was unanswerable for the one image that is pinned by digest and deployed by hand. The constant was there for a real reason: `.dockerignore` excludes `.git` to keep the context lean, so hatch-vcs has nothing to describe. Rather than un-ignore it, the version is resolved on the runner and passed in. Resolution lives in its own `version` job on ubuntu-latest. The build runs on the self-hosted mdb-dev runner and nothing in this repo has ever run setup-uv there, so resolving it in the build job would put an unproven dependency in front of the image; splitting it also leaves the build's checkout shallow, since only the resolver needs history. An empty arg now fails the build rather than falling back. A build that quietly substitutes something plausible is precisely what 2.0.0 was. `uv.lock` records anton-agent with no `version =` line (`dynamic = ["version"]`) and `2.0.0` appears nowhere in it, so `uv sync --frozen` is unaffected. Every consumer of `anton_version` displays it; none compare or gate on it. Guarded by six build assertions rather than convention — there is no runtime symptom to assert on, which is why this survived so long. All six mutation-verified, each mutation checked to have actually modified the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(build): reject the hatch-vcs fallback and validate the version whole-line Adversarial self-review of this PR found three defects in it. All measured, not reasoned about. 1. The fallback rebuilds the bug. With no tags reachable hatch-vcs resolves `2.0.0.dev1+g<sha>`; with no .git at all, `2.0.0-dev`. Both are well-formed and both sailed through a non-empty check -- so losing `fetch-depth: 0` would have baked the 2.0.0 family straight back in, silently. That is the entire bug, reintroduced through the front door. anton is CalVer, so a 2.0.0 version always means the tags did not arrive. Rejected in the Dockerfile as well as the workflow, so it holds however the image is built -- matching the 0.0.0 guard on the cowork-server side. 2. The shape check was a prefix match, and the value reaches a shell unquoted. The shared action expands `extra-build-args` unquoted into `docker buildx build`, and `2.26.8 --build-arg EVIL=1` passes `^[0-9]+\.[0-9]` -- becoming extra arguments to that command. This PR's body claimed the check "would have rejected it"; that claim was false. `grep -Eqx` over PEP 440 characters anchors both ends and makes it true. 3. A multi-line capture would corrupt $GITHUB_OUTPUT rather than fail, since the second line parses as another key=value pair. `tail -n 1` plus the whole-line check closes it. `pipefail` verified to be load-bearing here: without it a failing resolver is masked by the pipe. Also fixes a VACUOUS test caught by mutation, not by reading: the assertion for (3) matched `tail -n 1` anywhere in the `run` block, and the explanatory comment above the pipeline contains that string -- so deleting the pipe left the test green. It now asserts on the pipeline itself. Four new mutations, all caught. Ten total on this file now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(build): refuse a build context containing .git (ENG-1796) Review finding, and it is the wrong-quietly one this ticket is about. anton's image is SINGLE-STAGE. `COPY . /app` lands directly in the runtime image, so `.dockerignore` is the only thing keeping the repo's history out of every scratchpad pod -- and that exclusion had no guard and no test. What makes it the ticket's own failure class rather than a tidiness nit: un-ignoring `.git` produces no signal at all. Measured - `SETUPTOOLS_SCM_PRETEND_VERSION` wins over VCS discovery (`9.9.9.9` overrode a real `2.26.8.12.1rc6.dev8+g19c6e7515`), so the version stays correct, every guard added in this PR still passes, the build goes green, and the pod runs fine. The only symptom is full git history inside the image, which nothing reports. The asymmetry was backwards, too. cowork-server is multi-stage and deletes .git in the builder, and its test asserts that -- so the repo with a structural safety net was the covered one, while the repo where the exclusion is load-bearing was not. Two guards, matching the split used on the cowork-server side: - a test asserting `.dockerignore` still carries a bare `.git` line, and - a build-time `test ! -e /app/.git`, which catches .git arriving by any route (a negation pattern, a different context, a build that bypasses .dockerignore) rather than only the one spelling written in that file. Mutations 11-13, all caught: removing the line, negating it to `!.git`, and dropping the build guard. Separately: the reported `fetch-depth: 0` gap does not reproduce at 4c685f4. `test_the_version_job_fetches_tags` fails on all four ways of expressing it -- deleting the line, setting it to 1, deleting the whole `with:` block, and commenting it out. Left as is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
) * Shape the no-terminal-event turn_failed message for classification cowork-server's remote_turn_error keys on a "TypeName: message" prefix, same as the sibling TurnWorkerUnresponsive string. This one had no colon, so it read as an unrecognized type name and got discarded for the fully generic message instead of reaching the user. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Correct what this test actually proves The fallback only fires when a BaseException escapes stream_turn's finally, which today's deployed k8s path never raises from inside the pod - a real OOM-kill or dropped exec channel is detected by scratchpad-controller instead. The old docstring's "pod torn down mid-turn" framing read as production coverage this test doesn't provide. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* test connection timeout * different tool to no-console mode, allow to connect with console * Fix the non-interactive connect flow's retry prompt, timeout message, and tool schema per review feedback Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Konstantin Sivakov <konstantin.sivakov@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* feat: turn-key DataVault for cloud OAuth connectors (Google Drive + Gmail)
Adds TurnKeyDataVault, a DataVault Protocol implementation backed by a
live call to auth's POST /v1/oauth/{engine}/token endpoint with the turn
key as bearer auth. build_cloud_chat_session() now wires it in when the
turn's new `oauth` field (forwarded by scratchpad-controller) is present,
calling the same restore_namespaced_env() desktop's harness already uses
so DS_* env vars and credential scrubbing work identically in the cloud.
Part of the 6-repo cloud OAuth connectors MVP (auth, cowork-server,
cowork, Mind-Castle, anton, scratchpad-controller).
* run the blocking turn-key OAuth fetch off the event loop, send the connection name so multi-connection orgs resolve correctly, and stop gmail OAuth credentials from being misclassified against the legacy IMAP registry entry
---------
Co-authored-by: Konstantin Sivakov <konstantin.sivakov@gmail.com>
* fix(test): comments must not be able to satisfy the build guards (ENG-1796) Follow-up to #404, which merged. Same class of defect found in the cowork-server half of this ticket, and this file already had one instance of it: the single-line-capture assertion matched `tail -n 1` anywhere in the `run` block, and the comment explaining the pipeline contained that literal, so deleting the pipe left the test green. That one was patched in place; the class was not. The cowork-server half then reproduced it exactly -- a comment reading `already checks out with fetch-depth: 0` satisfied the assertion guarding that setting, and both mutations passed. Twice in one ticket is a pattern, so matching is now comment-immune by construction in both repos rather than per-assertion. Each line is truncated at its first `#` before matching. A prefix check is not enough: the likelier way to disable a guard is to keep its text as a trailing note beside a no-op, and `true # test ! -e /app/.git` passed against the merged test and fails against this one -- demonstrated on this branch before committing. A `#` inside a quoted string would truncate a real line early. That is the safe direction -- the assertion fails loudly rather than passing on prose -- and none of the asserted lines contain one. Mutations re-verified, all caught, each confirmed to have modified the file: the three build guards neutered by trailing comment, the literal 2.0.0 restored, fetch-depth dropped, `needs: version` dropped, and the build arg renamed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(test): one stripped accessor, enforced by AST (ENG-1796) Review finding: `_DOCKERFILE_TEXT()` was left behind by the previous commit -- dead code returning RAW text, sitting directly under the assertion it used to serve, with an inviting name. Nothing was broken by it, but rerouting one assertion through it and neutering the guard with a trailing comment gave `11 passed`. A loaded vector, and this class has already recurred twice. Deleting it alone would not have finished the job: the next Dockerfile assertion would reach for the raw read directly and land in the same place. So there is now exactly ONE stripped accessor -- the `dockerfile` fixture, which every other test in the file already uses -- and the guard assertion routes through it like the rest. Then a test that nothing bypasses it. Its first version scanned lines and immediately flagged its own docstring, because the prose there names the very expression it searches for: prose mistaken for code, which is exactly the defect this file has been chasing all along. Rewritten as an AST walk, which cannot see a docstring's contents, so the tool now matches the problem. Four mutations, each verified applied first: re-add the raw helper -> 1 failed assertion via a bare read -> 1 failed .git guard -> trailing comment -> 1 failed CONTROL: prose names the expression -> 12 passed (correctly ignored) The control matters as much as the failures: it is what distinguishes a guard that reads code from one that reads text. Also made a docstring raw -- it quotes a regex, and the escape was emitting a SyntaxWarning on collection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…urn_completed (#406) * feat(analytics): say WHY the completion verifier produced no verdict (ENG-1858) `turn_completed` could say THAT the verifier failed (`ended_by=handback_verifier_failure`, or `verification_skipped=true` for a latched/denied turn) but nothing said why: 343 such turns in 14 days (5.3% of real turns, ~3x the tokens of a completed one, 42% of all nemotron turns) with no groupable cause. The verdict loop already classifies each attempt for the latch — truncated / transient / hard / denied — and dropped the value at end of scope. Stamp two fields on `TurnCost` at every no-verdict exit and emit them: - `verifier_failure`: the loop's own class, plus `latched_hard` / `latched_denied` for turns that skipped the call because an earlier one latched. - `verifier_error_type`: `_safe_error_type` of the last attempt (class name only, never the message — it can quote conversation content), with `StructuredOutputError` split into `:no_call` (the model never produced the forced call — the ENG-1095 shape) vs `:unusable_call` (call present, schema rejected it). Same class name, opposite cures. `error_type` (ENG-1689) is left alone: its contract is "empty on handback" and overloading it would change meaning by `ended_by`. Tests: one per class, plus the latch/re-probe sequence and the verified-turn empty case; each stamp site watched failing under mutation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(analytics): pin latched_denied; document reached_tool_call on non-truncated errors (ENG-1858) Review follow-ups. `latched_denied` was the one emitted value with no assertion — the denied-latch test now checks both turns' (verifier_failure, verifier_error_type). Watched failing with the skip-site stamp collapsed to the bare latch reason (2 tests). `StructuredOutputError.reached_tool_call`'s docstring described the flag only for the truncated case; `_structured.py:247` computes it independently of truncation, and the verifier's `:no_call`/`:unusable_call` split relies on that. Say so, so the next reader doesn't hit the apparent contradiction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…-2017) (#409) * chore(cla): point the agreement link at the canonical repository (ENG-2017) The URL this passed was an alias that redirects to the repository actually holding the file, and it served byte-identical content, so nothing a contributor reads changes. An alias stops being harmless the moment somebody creates a repository at that name, and this is the page a contributor reads before agreeing to it. Ten library repos moved to the canonical name in the same change set; this makes all thirteen agree. Refs: ENG-2017 * chore(cla): grant actions read rather than write (ENG-2017) The shared reusable now declares `actions: read`, so write buys this caller nothing: a called workflow can never hold more than its caller grants, and the callee's own declaration caps it further. The only write the scope would buy is pullRerunRunner.ts re-running a previously failed CLA run, and that API refuses a GITHUB_TOKEN. Dropping the scope entirely is not an option, because the same file lists the repo's workflows first and main.ts turns any throw into a failed job. This is the grant the README documents for a caller, and this repo was one of three still on the old one. * chore(cla): stop passing an allowlist, the reusable owns it now (ENG-2017) The allowlist moved into mindsdb/github-actions and is bots only. This repo carried one of six hand-maintained copies naming people, and 11 of the 25 names across those copies had already left the org while still being exempt from signing. Staff sign like everybody else now. A bot cannot sign, so the two that actually open pull requests in this org stay exempt in the reusable. Dropping the line also drops two defects that rode inside it. `bot*` compiled to an unanchored `new RegExp("bot.*").test(login)`, so it exempted any login containing "bot", `robotnik` and `sabotage` included. And `Stpmax` never matched the real login `StpMax`, because non-wildcard entries are compared with a case-sensitive `===`. Needs mindsdb/github-actions#56 first, which gives the input a default.
…rom config (#410) * fix(prompt): answer "which model are you" from the served model, never from config (ENG-1638) The RUNTIME IDENTITY block was built from configuration and then told the model it "already knows" its provider and model and must NEVER ask. Three production failure modes followed: a configured-but-unapplied local model reported as running while MindsHub served the turn (conceals ENG-1634); the `mindshub_air` alias presented as a model name; and on the web pod the block rendered EMPTY with the mandate intact, so a Grok session answered "No — I'm Anton, not Grok". One block was answering two questions. Split them: - Serving model (new `anton/core/llm/identity.py`): the id the provider reported on its last planning response (`LLMResponse.model`, now captured at all six construction sites — MindsHub echoes the resolved id, verified live: mindshub_air → gpt-5.6-luna), falling back to the requested id labelled unconfirmed on turn 1. `mindshub_air` is the one opaque alias: it always reports "MindsHub Air" and the prompt says the model behind it is not disclosed — never named, guessed, or denied. That mirrors every other surface in the product (picker, billing, website), which all show the alias label and never the vendor model. - Configured LLM (`build_runtime_context`, moved here and re-exported from `anton.chat_session` for cowork-server): provider + model ids for code the agent writes, labelled as such. Workspace path and memory mode are dropped — neither served either question and the path leaked into traces. - No mandate without data: when nothing is known the prompt says to answer "cannot verify", never to guess or deny. The cloud pod now injects the configured block like desktop does. `LLMClient.last_served_model` remembers the planning role's served model; the session renders the identity lines per turn (cache prefix changes at most once per session, turn 1 → turn 2). `turn_cost.py`'s comment claiming the gateway does not echo the served model was wrong and is corrected. Tests: identity rule, sanitisation of untrusted `response.model`, section rendering, client capture (plan/stream, coding role excluded), session rendering across turns, every provider construction site, and the pod's config. Mutation-verified: 8/8 mutations of the fix fail a named test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(identity): strip the deprecated latest: pin before the Air rule (ENG-1638) Self-review finding on #410: cowork-server still resolves the deprecated `latest:` prefix and `_overlay_user_settings` copies the stored string onto AntonSettings verbatim, so a stale `latest:mindshub_air` pin reached the opaque-alias lookup, missed, and the agent would have reported the served vendor id for an Air session — the one leak this module exists to prevent. sanitize_model_name now drops the prefix, so the alias compares (and reads) the same however it was stored. Mutation-checked: disabling the strip fails both new tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(identity): state the requested model plainly; served id only reaches the prompt on the CLI (ENG-1638) Review finding on #410, confirmed by execution: cowork-server builds a fresh ChatSession and LLMClient every turn (harness.py:_build_chat_session) and the web pod is one process per turn, so `last_served_model` is None at every prompt build on both shipping hosts. The served-id line only ever appears where the client persists — the CLI. The requested-only line said "the provider has not yet confirmed it", promising a confirmation those hosts never deliver; it now says "(the model requested for this conversation)", which is true in steady state and is what the picker shows. Docstring states the reach and points at ENG-2050 (host records the served model per conversation) as the place to seed it across turns. New test pins the host shape: new client per turn, history carried, three turns, same requested line each time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…rty (ENG-1999) (#411) `scratchpad-dev-build.yml` triggers on `pull_request`, runs on `mdb-dev`, and carried no job-level condition at all. `mdb-dev` is a pod inside the newdev cluster, so once a maintainer approved a fork's run, that fork's code executed with the runner's cluster-wide Kubernetes token, its IRSA role into the build account, and the tool cache other repositories' jobs read back. It has happened once already, on a legitimate contribution. A `gate` job on `ubuntu-latest` now compares the pull request's head repository against this one and the build waits on its answer, in the shape cowork and cowork-server already carry. A promotion PR still builds, unlike in those two: this workflow is the only thing that builds the scratchpad image. The header comment claimed the build required a deploy label. No label check exists in the file; the comment now says what it does. The guard is asserted over every job rather than over `build`, so the next job pointed at a self-hosted runner cannot be added without one. Refs: ENG-1999
* refuse an out-of-list connection in TurnKeyDataVault and clear DS_* env unconditionally, not only when a turn carries an oauth block * fix stale DS_* env leaks across builds and restore vault-specific clear_ds_env dispatch
* feat: preserve artifact identity through publishing
* refactor: give an artifact one id instead of an id/stableId pair
An artifact carried two identities: `id`, eight hex characters wide and
baked into the folder slug, and `stableId`, a full UUID that keyed the
published versions, auth rules, revisions and comment threads. Two
fields meant two chances to disagree about which artifact this is.
Widen `id` to the full UUID and drop `stableId`. New artifacts get
`uuid4().hex`; the slug keeps a readable suffix by carrying `id[:8]`.
A legacy eight-character id widens deterministically:
id = old_id + uuid5(ns, f"{old_id}:{createdAt}").hex[8:]
The old characters stay the prefix, so folders already named
`<name>-<id[:8]>` keep addressing the same artifact. The 24-character
tail is derived, never random: anton widens in memory only while
cowork-server persists, so both have to reach the same value without
coordinating — a random tail would let whoever touched the artifact
first mint its identity and fork the comment threads.
Precedence, when both fields are present on disk:
* an `id` that already parses as a UUID wins outright, so a stale
`stableId` written by an older build cannot re-stamp an identity
that published versions are already keyed under;
* otherwise `stableId` decides — it already keyed those things, and
keeping them bound is worth more than the slug's readable suffix.
Anything else in `id` is widened rather than rejected: hand-written and
very old records carry names there (`"static-art"`), and refusing them
would drop the artifact from every listing, which reads as a deletion.
The one shape that raises is a value that plausibly IS a damaged
identity — hex-only and wider than a legacy id — because re-minting
that would silently detach the artifact from its published versions.
`id` is now constrained to 32 lowercase hex at the field, so a record
the widening validator could not widen is rejected at the metadata
boundary instead of surfacing later as `UUID('')` inside `artifact_key`.
`artifact_key()` moves here from cowork-server's side of the fence and
returns the canonical DASHED spelling — the same normalization the
upload lambda applies to what it stores in `_meta.json`, so both ends of
the comments API agree. The external `artifact/<uuid>` format does not
change; only which field feeds it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix: send the artifact key on the static publish path too
Review of #407. `publish()` derived `artifact_key` only for fullstack
artifacts, but a static artifact publishes its primary *file*, so the
folder holding metadata.json was never read and the payload carried no
key. The upload lambda then locked the report to the legacy
`{user_dir}/{report_id}` key and the artifact lost the identity its
draft, comment threads and access rule are grouped under. cowork-server
passes the key explicitly, so this only ever hit a direct anton publish.
Only the artifact root is consulted, never an ancestor: deriving from one
would let a nested page mint a second report under the same key, and the
auth rule html_upload upserts is per key.
Also from the review, both no-ops:
- `resolve_artifact_id`'s docstring claimed `stableId` decides only when
`id` is the short legacy form; it decides whenever `id` fails to parse,
since the damaged-id raise sits after that branch. The behaviour is
what we want (a stored `stableId` already keys published versions), so
the docstring moved, not the code.
- Dropped the line-number cross-reference from `_HOUSEKEEPING_FILES` (it
had already drifted six lines) and said instead that the set matches the
first path component, which is why `.revisions` belongs in it.
- `_zip_html` builds arcnames with `.as_posix()`, matching `_zip_fullstack`.
Not a fix: `ZipInfo.__init__` runs `_sanitize_filename()`, which already
replaces `os.sep`, so the bundle md5 was never platform-dependent.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: ianu82 <ianu82@yahoo.co.uk>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…he head repository `HEAD_REPO` is read from the pull request payload, so any trigger other than `pull_request` leaves it empty and the inequality fires — skipping the build while telling the run page a fork was responsible. The person who added the trigger would then chase a fork that does not exist. The gate now checks `IS_PR` first and emits its own message: adding a trigger stops the build until someone says how the gate should read it. The test sweep is widened to match: it walks every workflow file (both `.yml` and `.yaml`), resolves runner labels through local `uses` calls, and compares the job condition whole rather than searching it — an inverted or `always()`-prefixed condition still names the gate and still passes a substring check.
) * fix(tests): assign the analytics kill instead of setdefault (ENG-2055) `os.environ.setdefault("ANTON_ANALYTICS_ENABLED", "false")` writes only when the key is absent, so a developer with that variable exported — which is exactly what someone working on an analytics ticket sets — cancelled the guard silently, and the suite shipped real events to production PostHog. Measured against a local capture server on origin/staging: one run with the variable exported emits 259 events (tool_completed 149, ds_connect_* 89, ask_user_* 16, turn_completed 5). With the assignment: zero. All 2,709 tests pass either way, so the guard was doing nothing a test would notice. This is the source of the ~70% contamination in turn_completed. The fake rows carry `planning_model = "<AsyncMock name='mock.planning_model' id=...>"`, and counted per day the AsyncMock rows are the script-shaped rows (2026-08-27: 2,147 vs 2,147). ENG-1692's script-traffic guard does not cover this: it lives inside _emit_turn_cost alone, so three of those four families have no guard, and it only takes effect once a developer updates their installed build. Nothing loses coverage — the tests that exercise the analytics layer build their own settings objects and never read the environment, and tests/e2e/harness.py already sets the variable explicitly per subprocess. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(analytics): assert the suite's kill switch beats an exported variable Self-review finding F1 on #412. Nothing observed the guard in either direction — the same 2,709 tests passed whether it held or was silently cancelled, which is precisely why the leak survived four months with a green CI. This PR closed the hole and left that property intact. The assertion cannot be in-process: by the time any test runs, conftest has already executed in an interpreter the test did not control, so on a clean machine (which is what CI provides) an in-process check passes for the wrong reason. The child process exports the variable BEFORE the interpreter starts, which is the only shape in which the bug is visible. Mutation-verified: restoring `os.environ.setdefault(...)` fails the test with `enabled=True`; the fix passes it. Reuses test_analytics.py's existing subprocess + _CI_MARKERS scaffolding rather than adding a second pattern. The second test pins the other half, so nobody restores the escape hatch believing it was lost: `monkeypatch.setenv` still re-enables analytics inside a test, because it runs after conftest import and AntonSettings reads the environment at construction. 2,711 passed, 31 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(analytics): drop an inert CI-marker scrub and its inaccurate comment (ENG-2055) The kill-switch child imports conftest and resolves AntonSettings; it never calls send_event, so _is_ci() is never consulted on that path and clearing _CI_MARKERS changed nothing. The comment claimed the opposite. Verified in both directions with every marker set: the fix reports enabled=False, the setdefault mutant reports enabled=True. The scrub was copied from _run_child, where the child does call send_event and the pop is load-bearing. A comment asserting a guard that does nothing works against the property this test exists to establish. Also corrects the conftest note that still said this line "works on every build, immediately" — self-review F3 fixed that in the PR body but not here. It is read from the checkout, so it lands on the next run after a pull. Suite unchanged: 2,711 passed, 31 skipped, 0 events against a local capture server with ANTON_ANALYTICS_ENABLED=true exported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tests): blank the analytics sinks too, not just the flag (ENG-2055) The flag is honoured by exactly one `if` in `send_event`. This bug exists because ENG-1288 added an emitter that reached a real sink, so the next one reopens it — and the ENG-2055 test would not notice, since it asserts on a resolved setting rather than on no bytes leaving the process. Both are documented kill switches in analytics.py. Measured with the enabled-check bypassed: the flag alone still emitted 2 events, the flag plus these two emitted none. Full suite unchanged at 2711 passed / 31 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): correct the leak figures and name the caller that hides from a grep (ENG-2055) 260 events across five families, not 254 across four. Reproduced twice on a clean checkout. The fifth family matters less for its volume (1 event) than for how it was missed. Enumerating leakers by grepping the event name finds only TestPackageInstallTelemetry, whose four tests monkeypatch send_event and fire nothing. The event that actually reaches the wire comes from test_scratchpad_observer_dispatch.py, which never names the event at all and patches nothing — found by instrumenting the emitter with PYTEST_CURRENT_TEST. Same trap as the bug itself: reading the code is how this was missed for four months. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): 259 on the wire, 277 at the emitter — quote the wire (ENG-2055) The previous revision said 260 across five families and called scratchpad_package_installed a leak. It is not one. It reaches send_event and dies in _posthog_body with "Object of type MagicMock is not JSON serializable", swallowed by send_event's own except/pass. Verified three ways: absent from two full-suite wire captures, zero when the test runs alone, and instrumenting _posthog_body shows the TypeError. The 260/five figure came from counting at the emitter rather than at the wire. Counted there the totals are 277 invocations across 16 names — which is neither four families nor five, so the comment now carries both numbers and says which one to quote. Keeps the real find from that revision: grepping the event name cannot see test_scratchpad_observer_dispatch.py, which reaches the emitter without naming it. Adds the corollary it exposed — that caller passes a MagicMock, so it sails past both guards in this file and is safe only because a serialization error happens to stop it. Flagged as its own ticket rather than left reading as covered. Suite unchanged: 2711 passed, 31 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(analytics): guard the sink blanking, and stop subtracting two populations (ENG-2055) Two findings from the pre-merge review, both the same class this ticket exists to close: a claim about a guard that does not hold as written. 1. The conftest comment subtracted the emitter count from the wire count and called the difference swallowed exceptions. The two count different populations in both directions — some send_event calls never send, and some wire requests have no in-process call at all, because test_cloud_turn_process.py copies os.environ and runs the real entrypoint as a child. Measured alone: 5 wire events, 0 in-process calls. The tell was already in the numbers — the wire showed MORE tool_completed than the emitter (149 vs 148), which swallowed exceptions cannot produce. Also records the second reason the assignment matters: children inherit os.environ, so that same file goes 5 events -> 0 with this fix. 2. test_a_test_can_still_re_enable_analytics_for_itself claimed a test could still "exercise the enabled path". Since the sinks were blanked it can only flip the flag: analytics_url and posthog_key resolve empty, so send_event returns before either sink. The test passed because it asserted the setting. It now asserts at the sender — nothing handed to a sender thread — for both the direct and collector routes, and says what a test must also set to send. That second assertion closes real coverage: nothing tested the sink blanking before, so removing it from conftest was a silent no-op. Mutation-verified both ways — dropping the sink lines fails the re-enable test, restoring setdefault fails the kill-switch test. Suite unchanged: 2711 passed, 31 skipped, 0 events against a local capture server with ANTON_ANALYTICS_ENABLED=true exported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): scratchpad_package_installed does leak — name the caller that sends (ENG-2055) 260 on the wire across five families, not 259 across four. Measured three times. The previous revision was right that the observer test is safe, and right about why: MagicMock settings, TypeError in _posthog_body, swallowed. It generalised that into "it never leaked", which is wrong. Instrumenting send_event itself rather than send_package_install_event shows three callers, not one: test_analytics.py _PosthogSettings, fake host goes nowhere test_chat_scratchpad.py real AntonSettings SENDS test_scratchpad_observer_... MagicMock TypeError, swallowed The middle one is the wire event. It needs no "more realistic settings object" to become dangerous — it already builds a real AntonSettings and reads the environment. Both earlier findings survive: a name-grep cannot see the third caller, and reaching send_event is not sending. The accidental safety of the third is still worth its own ticket. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): 260 vs 259 is a cold-vs-warm workspace, not ordering (ENG-2055) The previous revision is right that scratchpad_package_installed leaks and right about which caller sends it. Its explanation for why other runs measure 259 is not: ordering within a run cannot change this. The event is gated on install_call_installed_something(result) (tool_handlers.py:719), and the `workspace` fixture is a persistent directory in the repo — <repo>/.pytest-workspace (test_chat_scratchpad.py:20), not tmp_path. So the first run on a machine really pip-installs cowsay and emits; every run after that gets "already satisfied" and emits nothing. Verified by toggling only that state, same commit, same command: workspace moved aside (cold) scratchpad_package_installed 1 workspace restored (warm) scratchpad_package_installed 0 Both counts were therefore correct on the machines that produced them. Records the cold/warm split and the `rm -rf .pytest-workspace` needed to reproduce 260 — without it the next person measures 259 and concludes the comment is wrong, which is how this question has now been reversed three times. Ran tests/test_analytics.py, tests/test_chat_scratchpad.py, tests/test_scratchpad_observer_dispatch.py, tests/test_tool_outcome_tracking.py: 99 passed. Comment-only change to conftest; CI runs the full suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* let the agent keep going instead of stoping to ask * tighten the ENG-1893 round-cap and spend-ceiling graces to be truly one-shot and bounded, add grace telemetry * fix round-cap grace re-arming on retry, deny zero tool-round configs, and handle a null close_to_done verdict
…ed (ENG-2126) (#419) Nothing had ever executed the scratchpad image's entrypoints outside a pod, so the first evidence an image could not serve a turn was prod going quiet. The build proved the image was assembled; nothing proved it ran. The Dockerfile's last layer now runs docker/image_smoke.py as UID 1000, below `USER 1000` so every check sees exactly what a pod sees. It executes both entrypoints the controller execs, with empty stdin: - `python -m anton.cloud_turn` must exit 0 having written exactly one JSON line to the protocol descriptor, and that line must be a terminal event. The controller reads the terminal off the EVENT, not the exit code, so a process that exits 0 printing nothing is a turn that hangs until the stall timer fires. - `python -m anton.core.backends.scratchpad_boot` must exit 0. It has no main guard (the module body IS the cell loop), so running it is the only way to load it; EOF ends the loop with no cell executed. Plus the invariants a pod needs and a build cannot otherwise see: the reported version matches the one the workflow resolved and is not the 2.0.0 fallback, uv is executable at the path ANTON_UV_PATH names, and the venv is writable by uid 1000 so a cell can still install a missing package mid-turn. In the build rather than in a step below it, so a failure blocks the PUSH: an image no pod can serve a turn with never reaches ECR to be pinned by mistake. Verified both ways against a real build — passes on a good image, and removing a startup-path dependency fails the build naming the missing module. tests/test_image_smoke.py covers the smoke's own failure modes. A gate that reports success no matter what is worse than no gate, so the silent-exit, polluted-stdout, non-terminal-event and raising-check paths are pinned, along with the layer's position below USER 1000.
|
I have read the CLA Document and I hereby sign the CLA 8 out of 11 committers have signed the CLA. |
Contributor
|
I have read the CLA Document and I hereby sign the CLA |
…rompt (ENG-2071) (#417) build_datasource_context() listed every `_picked_files` entry for a google_drive connection. It has no project in scope — `projects` is a cowork concept — so the list was unscoped and named files the user had granted inside *other* projects, together with "you MUST include every file above". cowork-server already renders this list correctly, scoped via ConnectionsService.picked_files_by_project() ("that's the scoping leak this closes"), and with richer prose. Both ran on desktop, so the correctly-scoped block and the unscoped one appeared in the same prompt a few hundred tokens apart. This removes the duplicate and leaves cowork-server as the single renderer. The availability paragraph stays, and its trigger is unchanged: presence of picked files is still parsed (not merely truthy-checked), so a Picker-only connection still fires it and a `_picked_files` holding only malformed entries still reads as "none". Removes ~238 tokens per LLM call for every user with a Drive connection. Security: this only removes data from the prompt. No new input is accepted, no credential handling changes, and the exposure it closes is file names/ids crossing a project boundary within one user's own account (not cross-tenant — the underlying Drive grant is connection-wide either way).
mindsdb-release-train
Bot
changed the title
Anton changes an artifact by writing a Python program that changes it. That is right for generating something. For modifying a file that already exists it is the long way round, and the program is a second thing that can be wrong: a one-line title change becomes a script, and a subtly wrong script mangles the artifact rather than leaving it alone. This adds the library for a second route — the model returns a diff and we apply it. Nothing is wired into the agent yet; that is the next commit, so this one can be reviewed on its own. The idea that makes it work, learned from the Go implementation in mindsdb/yolocoder: models get diff CONTENT right and diff ARITHMETIC wrong. They reproduce a file's lines faithfully and then miscount the @@ header, or end a hunk on its last change with no trailing context, which git rejects outright. None of that bookkeeping needs the model, because the file is right here. So every line number and count is discarded and each hunk is placed by locating its text. That one change took application from roughly half of attempts to nearly all of them. What follows from it is the rest of the design. Arithmetic is done here and judgement is left to the model: it picks which files to read, and searches when the names give nothing away; we place hunks, decide when a match is too ambiguous to risk, and check that the files it promised to create actually exist. A hunk that cannot be found and a short hunk matching in three places are both refusals, because guessing means editing the wrong part of someone's file and reporting success. Failure is a designed step. Three attempts, each shown the diagnosis and its own failed diff — told only "it failed", a model reproduces the same diff — and then the outcome carries the whole diagnosis out so the caller can hand the job to the scratchpad with it attached. Writes are all-or- nothing: a hunk that will not place in the third file leaves the first two untouched. Generated data stays out of it. A `<name>.data.js` is listed in the map but never inlined, and a diff against one is refused in code rather than merely discouraged; its `<name>.schema.json` sidecar is inlined in full instead. A hundred kilobytes of rows costs four words, its shape costs a few hundred bytes. Sidecars are derived from the bytes on disk and can be recomputed at any time, so they cannot drift from the data they describe. patch.py and workspace.py import only the standard library, which is why the engine is provable in milliseconds with no provider; a test pins it. agent.py uses anton's LLMClient directly and calls generate_object_code, so a run inherits the configured provider, the coding-model split, forced tool_choice, pydantic validation and turn tracing. 87 tests. Search is regex, guarded by a wall-clock interrupt: (a+)+b against thirty characters takes 46 seconds unguarded, and no cap on line or file size contains that — only SIGALRM does.
mindsdb-release-train
Bot
changed the title
…heck failed (#423) * make a failed completion check honest to the user and visible in a default log * keep the hand-back honest when a tool failed, and name the failing verdict model in the log Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
mindsdb-release-train
Bot
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release: staging → main
179 commit(s) queued for the next production release.
Changes
Contributors
Review checklist