fix(plugin): skip non-function exports in getLegacyPlugins - #1
Open
Cipher208 wants to merge 166 commits into
Open
fix(plugin): skip non-function exports in getLegacyPlugins#1Cipher208 wants to merge 166 commits into
Cipher208 wants to merge 166 commits into
Conversation
waitForWriter bounded the writer Deferred at 300s and mapped the expiry to
{status:"failure"}. That expiry does not cancel the writer, and the settle
watcher that owns the watermark advance awaits the same Deferred with no
bound — so a slow-but-successful writer still advances
last_checkpoint_message_id after the wait gave up.
The prune retry watcher therefore booked a working writer as broken:
writerFailures ticked and the session's crossed thresholds were cleared
("checkpoint writer failed — cleared thresholds for retry"). After
MAX_WRITER_FAILURES such waits it logs "gave up after max consecutive
failures" and stops checkpointing the session entirely — even though every
writer had actually succeeded. Losing checkpoint coverage also degrades
/rebuild, whose released span is what the checkpoint covers.
Observed on a live TUI run: writer spawned at 10:47:07, "failed" logged at
10:52:07 (exactly +300s), and the same writer then succeeded and advanced the
watermark. Reproduced twice in one session.
Report "timeout" instead. prune's existing `result !== "failure"` guard then
skips the counter and leaves thresholds alone, and /rebuild's `!== "success"`
check is unchanged. Retrying while the writer is still in flight was already
a no-op: isWriterRunning short-circuits new triggers.
…h writer-freshness + waiting UI
The /rebuild handler previously called rebuildFromCheckpoint() which only
inserted a boundary marker, then returned via prompt({noReply:true}) —
the runLoop was never entered, so no busy status was set (no spinner)
and no rebuild context was assembled on the spot.
Fix:
- Set session.status busy BEFORE the rebuild work so the TUI spinner
lights up immediately (wired through prompt.ts:2839 pattern → sync.tsx
→ prompt/index.tsx spinner rendering).
- Remove noReply:true on the rebuild-success path so the runLoop actually
runs — the model sees the rebuilt context boundary and produces a
response. The Runner's onIdle callback clears busy status automatically.
- Keep noReply:true only for the no-checkpoint case (no work to do),
with explicit idle status clear since the Runner won't handle it.
- The 3-case checkpoint-freshness semantics are preserved via
renderRebuildContext (checkpoint.ts:1112-1136):
1. Checkpoint exists + no writer → immediate rebuild (REBUILD_WAIT_MS not hit)
2. No checkpoint + writer running → wait FIRST_CHECKPOINT_WAIT_MS
3. Checkpoint exists + writer in-flight → wait REBUILD_WAIT_MS, fallback on timeout
Tests: 3 new tests in rebuild-on-the-spot.test.ts covering case 1
(immediate rebuild), case 2 (no checkpoint returns false), and a
source-level guard verifying busy status wiring and noReply removal.
All existing rebuild tests continue to pass.
…t exists
The previous implementation returned 'no checkpoint available' when
/rebuild fired on a cold session. Per the user's authoritative 3-case
design, case 2 requires actively spawning a checkpoint-writer, waiting
for it to finish, then rebuilding from the freshly-written checkpoint.
Changes:
- When hasCheckpoint() returns false and no writer is running, call
tryStartCheckpointWriter() to spawn one (promptOps stub since the
writer never reads it — it spawns as a subagent via spawnRef).
- Wait for the writer via waitForWriter() (5-min safety bound from
checkpoint.ts:985). On success, fall through to rebuildFromCheckpoint
which now finds the freshly-written checkpoint and inserts the boundary.
On failure/no-writer, show the no-checkpoint message as before.
- The busy status ('Rebuilding context…') is set before any work so the
TUI spinner lights up immediately; cleared by Runner's onIdle for the
rebuild path, or explicitly for the no-checkpoint fallback path.
- Updated case-2 test to assert the new behavior: source-level guard
verifying hasCheckpoint check, tryStartCheckpointWriter call,
waitForWriter call, and rebuildFromCheckpoint after writer success.
The /rebuild handler set status to busy without a message field, so the TUI showed a generic spinner indistinguishable from a normal turn. Add explicit messages so users see what phase they're in: - 'Rebuilding context…' for the initial busy status (all 3 cases) - 'Writing checkpoint…' for the case 2 writer-wait phase The busy type (SessionStatus) already supports an optional message field; the TUI renders it via component/prompt/index.tsx:1853-1859. No TUI changes needed. Updated test to assert both message strings are present in the source.
…include 'reasoning' The interleaved `field` literal was extended to include 'reasoning' and the SDK types were regenerated (upstream via XiaomiMiMo#1819), so the plugin `models` signature's ProviderV2 param (from @mimo-ai/sdk/v2) now accepts the local Provider shape. The `as any` that papered over the prior type break is no longer needed; removing it keeps `bun typecheck` green without suppressing the type (repo rule: avoid `any`).
…grepping prompt.ts source
The prior tests regex-matched the source text of prompt.ts (tryStartCheckpointWriter /
waitForWriter / busy-message strings) and called insertRebuildBoundary directly —
verifying nothing about runtime behavior and breaking on any harmless refactor
(violates AGENTS.md: 'Test actual implementation, do not duplicate logic into tests').
Rewritten to drive SessionPrompt.Service.command({ command: REBUILD }) — the same
path a user hits — against a scripted-LLM Bun.serve stub, asserting observable
outcomes:
- case 1 (checkpoint on disk + watermark): a checkpoint boundary message is
inserted and the handler enters the runLoop (model reply produced).
- case 2 (no checkpoint): a controlled spawnRef writer stub writes a fresh
checkpoint + advances the watermark, exercising the real spawn -> wait ->
rebuild path; asserts the boundary is inserted and the model replies.
- case 2 fallback (no spawnable writer): surfaces the no-checkpoint message with
noReply and inserts no boundary.
- busy status: captures the real 'Rebuilding context…' / 'Writing checkpoint…'
messages off the process-wide GlobalBus (no source-text assertions).
No mocks of the code under test; the spawnRef seam and scripted LLM stub the
system boundaries only (same pattern as checkpoint-rebuild-nonblocking.test.ts /
prompt.test.ts).
PR XiaomiMiMo#1752's core commit dropped noReply:true on the manual /rebuild success path (prompt.ts) so the runLoop would run after rebuild. That review-flagged 'deliberate behavior change' is the bug: a manual /rebuild is a user action whose intent is only to free/rebuild context — the user asked no question, so the model produces a spurious 'reply to nothing' turn (e.g. 'Ready for your next request'). Fix: restore noReply:true on the manual-/rebuild success path and clear busy status explicitly (the runLoop's onIdle no longer fires). The boundary is still inserted and the waiting UI ('Rebuilding context…' / 'Writing checkpoint…') still shows — only the spurious reply is gone. The AUTO-triggered rebuild path is untouched: it rebuilds mid-turn inside the runLoop and continues answering the pending user message, which is correct and necessary there. The distinction is structural — the auto path uses continue/return-continue inside loop() and never re-enters prompt(); only the manual handler calls prompt() with the synthetic note. Tests: rebuild-on-the-spot case 1 and case 2 now assert NO model reply after a manual /rebuild (llm.calls === 0, returned role != assistant), while still asserting the boundary insertion and busy-UI messages.
…d user turn
The manual /rebuild handler previously did the right thing (Step A:
rebuildFromCheckpoint → insertRebuildBoundary inserts the legitimate
rebuild boundary as a role:"user" message with a checkpoint part — the
SAME mechanism the auto-overflow and compaction rebuild paths use) but
then fabricated a SECOND, standalone role:"user" turn ("Context rebuilt
from the latest checkpoint…") via prompt({ noReply:true }). The auto
paths never create that: they just `continue` the runLoop to answer the
pending user message. noReply only suppressed the model REPLY; it did
NOT stop createUserMessage from persisting the fabricated user message.
So a manual /rebuild left extra role=user rows in the DB with zero
assistant replies — the earlier noReply commit was a band-aid on the
wrong layer.
Real fix: remove the fabricated prompt({ noReply:true }) call for every
/rebuild outcome. Manual /rebuild now mirrors the auto/compaction path —
insert the boundary and settle — WITHOUT a second user turn:
- success: return the freshly-inserted boundary message; surface
"Context rebuilt…" on the SessionStatus / Bus status channel; go idle.
- no usable checkpoint / degraded: return the existing last user message
(persist nothing new); surface "No checkpoint available…" on the
status channel; go idle.
Manual /rebuild is a user-initiated maintenance action with no pending
question, so after inserting the boundary it returns to idle (no model
turn, no auto-reply) — the auto path `continue`s only because it has a
pending message to answer. The noReply mechanism is preserved for other
callers (e.g. /goal clear).
Tests: rebuild-on-the-spot.test.ts now asserts that after a manual
/rebuild the message table gains EXACTLY ONE new message (the boundary,
role user + checkpoint part) — no fabricated "Context rebuilt…" user
turn and no assistant reply — and that the outcome is surfaced on the
status channel, not as a persisted user message.
Keeps XiaomiMiMo#1752's core value intact: on-the-spot rebuild, writer-freshness,
and the "Rebuilding context…"/"Writing checkpoint…" busy UI.
`/rebuild` inserts one user message whose parts are a `checkpoint` part plus `synthetic: true` text parts. The TUI's PART_MAPPING covers only text/tool/reasoning, and UserMessage renders only when a NON-synthetic text part exists — so the whole boundary message drew zero rows and the user had no way to tell a rebuild happened, or where. Render the checkpoint part as a one-line badge row, reusing the badge pattern already used for cron fires and actor notifications. Deliberately a render-only fix: rebuild's model-facing context is NOT changed. Compaction gets its visibility for free because it writes a real `summary: true` assistant message (compaction.ts) whose text renders through TextPart; rebuild instead puts its summary inline on the boundary user turn as synthetic text. Both DO reach the model — the user-part converter filters on `!part.ignored`, not `!part.synthetic` (message-v2.ts) — so rebuild's context is already at least as informative as compaction's, without a second LLM call. Copying compaction's assistant-message shape would duplicate that text in context and alter model semantics for no comprehension gain, so only the render layer moves. test/session/rebuild-boundary-model-context.test.ts pins that equivalence: the synthetic rebuild content and the compaction summary assistant turn both survive into the model messages, and filterCompacted keeps the summary message.
External plugins fail to load because getLegacyPlugins throws TypeError when encountering any export that is not a function and doesn't have a 'server' property (e.g. constants, config objects, type re-exports). This causes the entire plugin to be silently skipped — the error is caught in applyPlugin's Effect.catch and logged, but hooks are never registered. Fix: change 'throw' to 'continue' so non-plugin exports are silently skipped, matching the behavior of getServerPlugin which already returns undefined for non-plugin values. This fixes the issue where external plugins (plugin: [...]) don't execute their module code in MiMoCode 0.38.9.
…prompt Agents kept reaching for `actor run`, which blocks the whole conversation until the subagent finishes, so parallelism was lost on ordinary delegation. The cause was the prompt itself: `run` was listed first as the straightforward path and nearly every example used it. Reorder and rewrite the actor tool description so `spawn` is presented first and as THE DEFAULT (background, returns actor_id immediately, subagents run in parallel), and `run` is a narrow exception gated on a crisp test: only a tiny, fast lookup whose result you cannot make your next decision without in this turn. Examples flip to spawn (including a 3-way parallel fan-out) with one labelled run exception, and a collection section documents notifications plus wait/status. Behavior is unchanged - description/prompt text, action describe() strings and schema ordering only. Also corrects orchestrator.txt, which described `actor spawn` as blocking; both remain forbidden there for real work.
…tool calls
The empty/no-op tool-call loop guard (isEmptyStep + handleEmptyStep) treated a
tool call with `input: {}` as an "empty step" with NO PROGRESS. That is wrong:
plenty of legitimate tools take no arguments (e.g. `list_apps`), so a perfectly
valid step got soft-nudged and, after EMPTY_STEP_MAX_RECOVERY, hard-halted the
turn with a bogus "Empty tool call loop detected" terminal error.
The design cannot be narrowed into correctness — "the model called a tool with
no arguments" is indistinguishable from "the model made progress" without
per-tool schema knowledge that the guard does not reliably have. Remove it:
- delete src/session/prompt/empty-step-detection.ts and its two suites
- drop isEmptyStep / handleEmptyStep / emptyStepStreak / hardHalt wiring and
both branch call sites from src/session/prompt.ts
- drop MIMOCODE_EMPTY_STEP_MAX_RECOVERY from src/flag/flag.ts
- drop the invalid-output-continuation case that asserted the guard's halt
This is a pure revert. An earlier revision of this branch also added a
`leaked-toolcall-marker` detector (matching a text part whose whole trimmed
content is "call:", "code", or a bare invoked tool name) plus a retry ladder.
That is intentionally NOT included: the marker leak was a quirk of Claude
Opus 4.8, which is no longer in use, so the detector would be dead code for a
defect that no longer occurs. It also carried real downside — a legitimate
one-word `code` text part next to a same-named tool would discard the whole
step, and priming the model about `call:` plausibly makes the leak more likely,
not less.
Replaces separate lint.yml, test.yml, typecheck.yml with unified ci.yml: - typecheck: bun typecheck - lint: oxlint + prettier check - test: sharded unit tests (4 shards) - security: skylos scan on plugin directory - quality: repowise health on plugin directory - python: auto-detects pyproject.toml, runs ruff/mypy/pytest - rust: auto-detects Cargo.toml, runs cargo check/test - go: auto-detects go.mod, runs go vet/test Multi-language: Python, Rust, Go jobs auto-detect and skip if no files found.
Every other example in actor.txt is clean, copy-pasteable JSON. The run example carried a trailing prose annotation on the same line, and the model imitates the shape of these examples line-by-line rather than the parsed JSON — so the arrow could be copied verbatim into a real call. Move the note to its own line above, matching the prose convention the EXCEPTION example block further down already uses.
…ter-wait-timeout-not-failure fix(checkpoint): don't count a timed-out writer wait as a writer failure
…xpires Follow-up to XiaomiMiMo#1938. That PR stopped a merely-slow writer from being booked as a failure by returning a distinct "timeout" from waitForWriter, which prune's `result !== "failure"` guard skips. Correct, but it left two holes. 1. Hitting the bound became completely silent. waitForWriter returned and prune's watcher returned, neither logging — yet the +300s log line is exactly the evidence XiaomiMiMo#1938 was diagnosed from. waitForWriter now logs "checkpoint writer wait bound expired — writer still in flight". 2. The real outcome was booked nowhere. prune's watcher fiber is the only holder of the per-fire accounting and writerFailures is private to the prune layer, so once the watcher returned on "timeout" a writer that genuinely FAILED past 300s ticked no counter — making MAX_WRITER_FAILURES unreachable for exactly the slow regime XiaomiMiMo#1938 is about, so a permanently-broken-but-slow writer retried forever with no give-up warning. Symmetrically, a writer that SUCCEEDED past 300s never cleared a counter left at 1-2 by earlier fast failures, so a later fast failure could trip "gave up" for a session whose writers demonstrably work. The watcher now extends its wait across bound expiries and accounts for the settled result, capped at MAX_WRITER_WAIT_EXTENSIONS (~1h) so a writer that never settles cannot pin the fiber for the life of the process. Two microsecond-wide re-entry windows are documented rather than papered over. Tests: prune.test.ts gains the two cases that pin the prune-side consequence XiaomiMiMo#1938 is actually about (timeouts never tick the counter and a late success clears it; a late failure is still counted so the cap stays reachable) — the existing test only asserted waitForWriter's return value. The timeout test now also pins the BOUND (still pending at 4 minutes, so shrinking it to 1s fails) and asserts the writer is still running after the expiry, replacing a dead `not.toBe("failure")` implied by the line above it. Also folds the stale 5-min-padding comment into the current block and corrects checkpoint-align.ts's now-conditional claim about writerFailures.
… hostname-fallback authorship A separate worktree checkout shares the object/ref store but has its own config, so it does NOT inherit the parent repo's LOCAL git identity. When global identity is also empty, git commit autodetects user@hostname (e.g. MI <mi@host.local>), leaking the machine hostname and wrong authorship into pushed commits. setup() now resolves the parent repo's identity (git -C <ctx.worktree> config user.name/email, which walks local->global->system) and pins it into the new worktree's own local config, right after the HEAD-attach assertion and inside the existing per-repo lock. If the parent has no identity at all, it falls back to a stable mimocode identity (mimocode <mimocode@users.noreply.github.com>) so the worktree is never left without one.
Layer-1 (worktree/index.ts setup) only covers worktrees mimocode creates in code. It does NOT cover worktrees/commits an agent makes via the bash tool (git worktree add / git clone / committing in an ad-hoc dir) — those still fall back to MI <mi@hostname.local>. Layer-2 adds a floor in BashTool.shellEnv(): resolve the repo identity once per worktree (git config user.name/email at Instance.worktree via the Git service) and inject GIT_AUTHOR_NAME/EMAIL + GIT_COMMITTER_NAME/EMAIL into every bash env. Fall back to a stable mimocode-agent[bot] identity when the repo has none, and guard the non-git case (Instance.worktree === '/') so we never read git config at root. Layering / git precedence: explicit -c / repo-or-worktree LOCAL config (layer 1) > GIT_AUTHOR_*/COMMITTER_* env (layer 2 floor) > global > autodetect user@hostname. The floor is placed below process.env (an operator-set GIT_AUTHOR_* still wins) and above plugin extra.env (a plugin can still override), and only fills vars not already present in process.env. Complementary, not conflicting. Also aligns layer-1's fallback identity to mimocode-agent[bot] for consistency. Adds bash-env floor tests (inherit from repo config, bot fallback for a non-git project, operator-override wins) alongside the existing worktree identity tests.
The fallback identity email was fabricated by analogy to the real opencode-agent[bot] GitHub App address. Replace it with the intended mimo@xiaomi.com / "MiMo Code" pair in both layers (worktree setup local config + bash shellEnv floor) and their tests.
The fallback git-identity name for agent-authored commits is now "MiMo" instead of "MiMo Code"; the fallback email stays mimo@xiaomi.com.
Propagate one stable turn context through MCP tool execution and notify negotiated servers exactly once when a turn completes, is cancelled, or fails. Keep the behavior capability-gated and provider-neutral, with serialization and recovery tests for overlapping notifications.
Advertise the exact lifecycle v1 client capability and propagate turn cancellation into in-flight MCP calls so terminal notifications cannot race active tool work.
main's request-scoped MCP discovery gates MCP tools behind mcp_tool_search for the default test model, so mcp_lifecycle never executed and no turn context was captured. Pin the two tool-calling lifecycle tests to the non-GPT model that still exposes MCP tools directly.
…ends turnID/turnActorID on the tool Context were residue of the MCP-in-tool_script path that main removed in 8c29041; nothing reads them, so delete them. A lifecycle notification that never settles kept its pending-map entry forever, so every later turn for that client queued behind it, waited out the 1s timeout and was dropped -- a permanent, invisible per-turn stall. Record when a send started and release an entry that has outlived the budget so the next turn sends immediately, without ever awaiting the orphaned promise.
…or contract Review follow-up on the two-layer git-identity fix. - Extract the fallback identity into Git.FALLBACK_IDENTITY (src/git/index.ts), the module both layers already import. The literals "MiMo"/"mimo@xiaomi.com" were duplicated in src/worktree/index.ts and src/tool/bash.ts; that constant had already been renamed once across several files, so the drift risk was demonstrated rather than hypothetical. Both tests now assert against the shared constant too, so a future rename cannot pass in one layer while silently failing in the other. - Document the floor's behavioral contract on gitIdentityCache: its only job is to stop `user@hostname` authorship; it is delivered as env, and git gives GIT_AUTHOR_*/GIT_COMMITTER_* precedence OVER user.name/user.email config; the per-worktree memoization means a mid-session `git config` change is not picked up until the process restarts; operator-set vars still win per-variable. - Correct the previous shellEnv comment, which claimed the env floor sat "below repo/worktree local config, which still wins". Verified empirically that git env vars override config, so the claim was backwards. - Note why resolveGitIdentity/gitIdentityCache sit in the tool's outer setup block rather than inside shellEnv (memoization across bash invocations). - Extend the operator-override test to assert per-variable precedence: with only GIT_AUTHOR_NAME operator-set, the other three vars must still receive the floor. It previously asserted names only, leaving the email path uncovered.
A live Bedrock 400 `messages.<N>: user messages must have non-empty content` was traced to the AI SDK, not to our message array. `ai@6`'s `convertToLanguageModelMessage` strips empty text parts from user messages with no backfill: .filter((part) => part.type !== "text" || part.text !== "") That runs AFTER every ProviderTransform step, so a user message whose only text part is "" leaves our transform as a healthy length-1 array and reaches the provider as `content: []`. Emptiness therefore cannot be judged by `content.length` at this layer — it must be judged by what survives the SDK's own filter. The SDK's assistant branch has an escape for empty parts carrying providerOptions; the user branch does not, so even an empty text part holding a cache_control marker is stripped. Our only prior defense stripped empty parts in `normalizeMessages`, but gated on `@ai-sdk/anthropic`/`@ai-sdk/amazon-bedrock` — a Bedrock-backed gateway on any other npm got no protection at all. `normalizeContentArray` guarded only content shape and itself emitted `content: []`, and `ensureTrailingUserMessage` inspected only the trailing assistant, with a comment claiming an empty trailing message was "safe to send as-is". Add a provider-agnostic pre-send invariant, `ensureNonEmptyContent`, and make the two guards cooperate instead of fighting: - user -> BACKFILL a minimal non-empty text turn. Dropping it would end the request on an assistant, trading this 400 for the assistant-prefill 400 that XiaomiMiMo#1703 fixed. - assistant -> DROP; it is residue, and the trailing-user guard that runs next re-establishes the prefill invariant. - tool -> leave untouched; `tool_result` blocks cannot be synthesized and injecting text would break tool_use/tool_result pairing. `normalizeContentArray` now backfills user/tool instead of blanking, and ordering is made explicit and documented: resolve empty content FIRST, then the trailing-assistant/prefill invariant. Tests assert both invariants together — no empty content AND the request still ends with user/tool — across anthropic, bedrock, openai-compatible and openai, plus an end-to-end case run through the real AI SDK prompt conversion that reproduces the captured wire payload.
…ed results, and live sub-call trace - Restore the toolScriptMcp late-bound ref removed by 8c29041, now populated per-request by SessionPrompt with only the active MCP view — under mcp_tool_search gating exec sees exactly the search-loaded tools, so it cannot bypass the discovery gate. Dispatch reuses SessionPrompt's wrapped executes (permission ask, plugin hooks, metrics, truncation). - MCP structuredContent crosses into the guest pre-parsed as `structured` so scripts can aggregate data without re-parsing text output. - publishProgress ships a bounded trace tail (last 20 calls) and the final metadata carries it over; the TUI renders the last 5 sub-calls live and keeps them visible after completion. - New MIMOCODE_ENABLE_EXEC_TOOL flag exposes exec to all models (was GPT-toolset only).
…-call trace Revert exec to the compact collapsed-by-default view (XiaomiMiMo#1941 made the script source a always-visible BlockTool, which floods long transcripts) but keep its stripAnsi fix. Collapsed state is now a single BlockTool holding the summary title and the recent sub-call trace — one bordered, hover-highlighted click target, kept after completion. Falls back to a one-line InlineTool until the first sub-call lands.
…t-limit-double-rebuild fix(session): prevent duplicate context rebuilds
…mode feat(tui): add Vivid and Minimal visual modes
…erminology docs: add TUI rendering troubleshooting
Base agent prompts hardcoded Claude-style tool names (Glob/Grep/Read/Bash) and generate.txt told the model to use a nonexistent "Agent tool", so GPT models were instructed to call tools absent from their schema. Keep the base prompts provider-neutral and move exact GPT tool contracts into a conditional fragment instead. - explore.txt: reference the tools exposed in the current turn, with an rg/rg --files shell fallback; add parent-agent delegation semantics - generate.txt: say "actor tool" (the real tool) instead of "Agent tool" - generate-gpt.txt: new GPT-only fragment (exec/apply_patch/view_image/actor) appended by usesGPTToolset(), leaving non-GPT paths untouched - general.txt: dedicated prompt for the general subagent - add Agent.Info.completionGate so general keeps RETURN_FORMAT_INSTRUCTION now that it carries its own prompt
- allow general subagents to use the inherited runtime tool surface\n- update prompts, tests, and documentation for end-to-end delegated work
- remove the GPT-only subagent prompt fragment\n- preserve model-specific runtime tool selection and coverage
…line fix(cli): normalize command output newlines
…pt-tool-compat fix(agent): align subagent prompts with runtime tool schemas
Teaches agents how to search mimocode's memory system and raw trajectory database when the built-in memory tool alone is insufficient. Covers BM25 query optimization, scope escalation, SQLite schema documentation, 5 ready-to-use query templates, and per-goal search strategies. All queries validated against a live 2.3GB production database.
…robustness - Query 5 (repeated errors) was only finding completed bash calls with 'error' in stdout — genuinely failed tool calls (status='error') store the message in $.state.error, not $.state.output. Split into two queries: one for stdout errors, one for actual tool failures. Added explanatory note. - Query 3 (by tool name) now uses COALESCE(output, error) and shows status so both success and failure cases are visible. - Added 'cc' to the scope escalation list (Claude Code imported memories). - Review point 2 (agent_id examples) verified as correct — DB shows explore-1, general-1, etc. from allocateActorID; reviewer confused peer mode (actorID = sessionID) with normal subagent mode.
…earch-skill feat(skill): add memory-search builtin skill
* fix(mcp): share one process-wide client layer * test(mcp): assert single MCP ownership behaviorally The previous test only used toBeDefined() and reference inequality, which any two distinct objects satisfy — re-adding a self-provided MCP.defaultLayer to an appLayer kept it green. AppLayer is also a Layer.suspend, so toBeDefined() never forces the thunk, leaving the test with no marginal value over tsc. Now: - Rebuild the ownership chain exactly as app-runtime.ts does, swapping only MCP.defaultLayer for a counting stub, and assert the stub is constructed once and that provideMerge keeps MCP.Service in the output so server routes can still resolve it - Assert each appLayer fails to build standalone on a specific missing service (Command and SessionPrompt miss MCP, Actor misses SessionPrompt because it never consumes MCP directly), so re-adding a self-provided dependency turns the test red - Add one narrow source assertion that AppLayer wires MCP.defaultLayer exactly once, covering the extra-leaf regression the behavioral tests cannot see The real MCP.defaultLayer and the full AppLayer are never built, so no subprocess is spawned. * docs(mcp): correct stale Actor.defaultLayer references AppLayer now constructs Actor.appLayer rather than Actor.defaultLayer, which leaves these references inaccurate: - session/checkpoint.ts: this comment is the only explanation of how the Actor -> SessionPrompt -> SessionCheckpoint -> Actor cycle is broken by the late-bound spawnRef, so a wrong name sends readers looking for a spawnRef assignment that is not there. Also records that appLayer wraps the same Actor.layer, hence spawnRef is still populated. - tool/actor.ts, tool/session.ts, server/routes/instance/session.ts: three developer-facing diagnostics that point at an unpopulated spawnRef. Verified by grep that no test asserts these strings literally. - effect/app-runtime.ts: the same reference in the TDZ comment (comment only, mechanism untouched). * docs(mcp): correct the guard comment's account of the old layer shape The comment claimed the four-leaf shape "started one MCP subprocess set per leaf". It did not: Layer.effect memoises on the layer's own identity and every ManagedRuntime in this process shares the single memo map from src/effect/memo-map.ts, so the old graph already built exactly one MCP instance. Single-instance behaviour was therefore incidental — it rested on memo identity rather than on the composition — which is the actual reason to make the ownership chain explicit, and the actual thing the regressions in this file protect.
…uery guide Focused skill for querying the raw mimocode trajectory database directly via SQL when the built-in memory (BM25 curated markdown) and history (FTS raw messages) tools are insufficient. Covers: schema documentation, 6 validated query templates (session listing, keyword search, tool calls by name, execution chains, stdout errors, actual tool failures), per-goal strategy table, and safety constraints. All queries validated against a live 2.3GB production database.
…aomiMiMo#2035) XiaomiMiMo#1964 put the render prohibition behind the navigation gate, so opening a checkpoint-writer host is refused — but the Sessions dialog still LISTED one `↳ checkpoint-writer: …` row per checkpoint. Those are two separate paths and the gate cannot stand in for the list. The leak: sync.sync() fetches children with `visible: true`, but the `session.updated` arm in sync.tsx inserts EVERY session it sees into the store, and checkpoint.ts creates the writer host with its title already set — before the actor row is registered — so it arrives on that path with a display-ready title and `isChildOfCurrent` passed it straight through. Filter the child arm through classifySession, the same predicate the gate uses, so the list cannot disagree with what opening the entry would do. Fails open (no actor rows ⇒ listed), which is what keeps orchestrator `session create` children listed: they own a mode "peer" row and classify renderable outright.
…earch-skill-v2 feat(skill): add memory-search builtin skill — SQLite trajectory DB query guide
…s, keep reads (XiaomiMiMo#2040) * feat(config): add the memory.capture switch field Controls memory WRITES only; the read path is unaffected. The default is not written into the schema — each read site realizes it with `?? true`, matching memory.cc_index / checkpoint.* / dream.* in this repo. * feat(checkpoint): W1 do not start the checkpoint writer when memory.capture is off Short-circuit with `return "skipped"` at the very top of tryStartCheckpointWriter, which holds down all three write paths from a single place: direct template writes, the writer subagent spawn, and the validation-retry rename. The read path (renderRebuildContext / memory retrieval) is completely unaffected. * feat(memory): W2 stop demanding progress.md when capture is off W2 (subagent-progress-checker): the postStop hook returns immediately when capture is off. Otherwise "demand progress.md -> the write is hard denied -> demand it again" forms a postStop infinite loop that burns tokens. Config is read through the plugin client to avoid the app-runtime import cycle; a failed read fails open. W3 (gating the high-pressure "write to memory" nudge) is not part of this commit: that nudge was deleted wholesale upstream by dd1e20c "fix(session): remove context pressure nudge". Gating a feature that no longer exists is pointless, so prompt.ts is byte-for-byte identical to origin/main. * feat(memory): W5 hard-deny tool writes inside the memory directory when capture is off memory-path-guard stays a pure function: the switch is passed in via the optional captureEnabled parameter, and omitting it counts as enabled (backward compatible, so existing call sites need no change). The error copy states outright that memory writing is disabled and forbids the model from retrying under a different path, which avoids a loop caused by prompt drift. external-directory reads config with Effect.serviceOption — the `R` of Tool.Def.execute must be `never`, so it cannot `yield* Config.Service`; it fails open when the service is missing. Known residual: bash is not covered by this gate (already declared in the memory-path-guard.ts comment), and a model using a heredoc can still get through. Extending the gate to bash would disturb the whole permission layer, so it is out of scope for this round. * test(plugin): stub the client in the postStop test and pin that capture=false stops demanding progress.md The original test built pluginInput as `{} as never`, which crashes once the hook starts reading config. It now supplies a minimal client stub (capture unset = no memory section = on by default), plus three new cases: with capture=false the hook neither nags nor creates files, and capture=true behaves identically to having no config at all. * test(memory): integration coverage of the capture switch on the write and read paths W1: with the field absent and with capture:true it still reports started and bootstraps the templates; with capture:false it returns skipped, the spawn count is 0, and not one of checkpoint.md / notes.md / tasks is created. Read path: under capture:false an existing checkpoint still produces rebuild context and memory retrieval still hits old memories. W5: strings config -> serviceOption -> guard end to end, confirming that the caller gets an explicit "disabled" error, that writes outside the memory tree are unaffected, and that an absent field or true still lets writes through. * refactor(memory)!: rename memory.capture to memory.disable_write (negative boolean) + W6 stops dream/distill Field: memory.capture (positive) -> memory.disable_write (negative, optional boolean, no .default). Absent/false = writes proceed as usual; only true disables them. Read sites funnelled: the new isMemoryWriteEnabled(cfg) in src/memory/write-gate.ts. The double negative exists only inside that function body (disable_write !== true). Business code always calls it positively and never reads the field directly. All five call sites across W1/W2/W5/W6 go through the accessor. W6 (new this round): shouldAutoDream / shouldAutoDistill return false when writing is off, so the background does not keep auto-producing memory and skill artifacts after the switch is flipped. Filled in: the end-to-end postStop infinite-loop regression now asserts that the nag copy does not appear, because a general subagent also receives unfinished-task reminders unrelated to memory, so the turn count cannot isolate it. W3 has no corresponding change: the high-pressure "write to memory" nudge section was already deleted wholesale upstream by dd1e20c. Error copy: states outright that memory WRITING is disabled, points at memory.disable_write, and notes that reads are unaffected. * feat(memory): surface the compaction fallback when memory writing is off With memory.disable_write on, no checkpoint is ever written, so every overflow degrades to compaction for the whole session. That degradation left the user only a log line, and the one message that was surfaced blamed a failed checkpoint writer - reading like a bug to report rather than the switch they set. Name the switch instead, once per session, on the existing status channel plus a persisted display-only part so the notice survives past the status flash and reaches headless runs. * fix(memory): fall back to compaction immediately when memory writing is off A rebuild with `memory.disable_write` on could only ever end in compaction, but it walked the whole doomed path to get there: read the checkpoint file, probe hasCheckpoint plus lastBoundary, start a writer that short-circuits to "skipped", then await a writer that was never started. Every step is predetermined when the switch is on, and the wait was announced to the user as "Writing checkpoint…" — a wait for a writer we were never going to start. Guard at the top of rebuildEnsuringCheckpoint: when memory writing is off it returns the new "memory-write-off" outcome before touching disk, the DB or the writer. All three fallback sites (the token-threshold overflow, the provider-signalled overflow, and manual /rebuild) treat it as compact now and say why, so the notice is reason-attributed rather than inferred from a re-read of the config. Memory-on semantics are untouched: "insert-failed" still refuses to compact, and a genuine "writer-failed" keeps its own "the checkpoint writer failed" text, which the switch case can no longer trigger. The unit test installs a working writer stub and asserts the writer-wait announcement never fires — verified discriminating by neutralizing the early return, which makes exactly that assertion fail. The spawn count is asserted too but is documented as weaker: the memory gate inside tryStartCheckpointWriter already blocks the spawn, so it stays zero either way. * fix(memory): emit the memory-write-off messages in English only The compaction-fallback notice and the W5 memory-write refusal each packed English and Chinese into a single string. Neither is a prompt: the notice is persisted with `ignored: true`, the repo's display-only flag, so it is shown to the user and withheld from the model context. These messages land in the session record, which the TUI, headless `run --format json`, and other consuming clients all read. The engine cannot know the reader's locale; the consuming client can, and already carries its own translations. So the engine emits stable single-language English, matching its neighbours `compactedInsteadMsg` / `rebuildFailedMsg`, and localization stays with whoever renders it. No i18n mechanism is introduced here. Both texts keep their full content. The notice still names the switch AND that compaction stood in for the rebuild, keeps the `memory.disable_write` remedy and the "nothing is broken" reassurance, and does not escalate its wording. The refusal still says WRITING is off rather than memory, still tells the caller not to retry another memory path, and still notes that reading is unaffected. The tests that asserted both languages were present now anchor on the English text and pin the single-language invariant instead.
…tool calls (XiaomiMiMo#2054) * fix(mcp): use last valid JSON snapshot for tool args in openai-compatible patch * test(mcp): regression test for openai-compatible multi-arg stream patch
- compute message-specific diffs from the user message and direct assistant reply\n- preserve cached summaries while normalizing git paths
…e-diff fix(session): scope diffs to requested message
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
9 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.
Problem
External plugins () don't execute their module code in MiMoCode 0.38.9. The plugin module IS loaded (ES succeeds), but hooks are never registered.
Root Cause
in throws when it encounters ANY export that is not a function and doesn't have a property. This includes:
The throw is caught by 's handler (line 392), which silently swallows the error and returns . The plugin is never loaded, hooks are never registered.
Fix
Change to on line 178. Non-plugin exports are silently skipped, matching the behavior of which already returns for non-plugin values.
Testing
Impact