Skip to content

Support Kimi Code CLI as a builder (PIR #1201) - #1203

Draft
mohidmakhdoomi wants to merge 69 commits into
cluesmith:mainfrom
mohidmakhdoomi:builder/pir-1201
Draft

Support Kimi Code CLI as a builder (PIR #1201)#1203
mohidmakhdoomi wants to merge 69 commits into
cluesmith:mainfrom
mohidmakhdoomi:builder/pir-1201

Conversation

@mohidmakhdoomi

@mohidmakhdoomi mohidmakhdoomi commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

PIR Review: Support Kimi Code CLI as a builder

Fixes #1201

Re-integration notice (2026-08-09). This PR was parked on two upstream blockers, both now merged (#1317/#1267, #1356/PIR #1233), and main moved 900+ commits — including Spec 1313, which made afx send mailbox-first. kimi itself moved 0.27.0 → 0.34.0. The branch has been merged up and the feature substantially redesigned against what changed underneath it. The delivery mechanism, the resume mechanism, and the pacing seam are all different from what earlier reviewers saw; the summary below describes the current design, and "What changed since your last review" near the end lists the deltas. Re-review from scratch rather than from memory.

Summary

Adds the Kimi Code CLI (kimi, ≥ 0.33.0) as a supported builder harness — shell.builder: "kimi" / builderHarness: "kimi" / --builder-cmd kimi now produce a working builder instead of the #1062 false-Claude fallthrough (which appended --append-system-prompt and a positional prompt, both rejected by kimi, and could route a stale Claude --resume <uuid> into it).

Kimi has no system-prompt flag and takes no positional prompt, so role and task travel on two different channels:

  • Role → --agent-file (kimi 0.31.0+). getWorktreeFiles writes an agent-definition file into the worktree whose body wraps the role around ${base_prompt} — the token that interpolates kimi's own default system prompt, so the role extends rather than replaces it. This is the claude --append-system-prompt analogue.
  • Task → the Spec 1313 mailbox. The generated launch script queues the task with afx send, and the render gate delivers it onto a verified-empty composer. Never a direct PTY write, so a boot screen, a busy line, or kimi's folder-trust dialog holds the message rather than corrupting or losing it.

Crash restarts resume with the documented, cwd-scoped kimi -c — no session id is ever baked into generated bash. Kimi as an architect remains out of scope (stage 2); buildRoleInjection throws and doctor warns, so misconfiguration fails loudly rather than falling through to claude flags.

The finding that shaped the design

kimi -c does not fail when there is nothing to continue. It prints No sessions to continue under "<cwd>"; starting a fresh session. and starts one anyway — and that session never saw --agent-file, so it runs silently roleless. That is the #929 hazard class, arriving through a documented flag.

So the launch loop only takes -c after an inlined node -e store probe proves a conversation exists for this cwd, and the probe fails closed: any error (no store, unreadable dir, malformed JSON) exits non-zero and the loop relaunches fresh with the role, which is always safe. Tests execute that probe against fixture stores and cross-check its verdict against the TypeScript discovery it mirrors, so the hand-written snippet cannot silently drift from findLatestKimiSessionId.

The probe asks "would kimi -c continue this?", not "does a directory exist" — a distinction review had to teach me (see below). Kimi lists a cwd's sessions before continuing one, and that listing drops archived sessions and ids it does not recognize, so a session we call resumable but kimi skips lands right back on the roleless path. Both filters now apply on both sides of the mirror.

The same probe makes the script's entry self-configuring, so afx spawn --resume and a Tower-side terminal re-create need no second script shape — and a re-run never re-queues the task into a live conversation.

⚠️ Please look here first: two edits to shared, just-merged gate logic

servers/render-gate.ts is Spec 1313 code that landed recently. This PR touches its classifier in two places. Both are opt-in per profile, and neither can change behaviour for claude, codex or agy — argued below and pinned by tests.

1. The marker exemption follows the matched span instead of column 0

-      if (row === markerRow && col === 0) continue; // the marker glyph itself
+      if (col < markerEnd) continue; // the marker glyph itself

Why: kimi draws its composer inside a rounded box, so its prompt marker sits at column 3 (│ > ), not the row start. Under the column-0 rule the > glyph counts as user text, so a genuinely empty Kimi composer classifies user-text forever — i.e. holds all of its mail, permanently.

2. A profile may declare an upper bound for the composer region

GateProfile gains an optional regionStartPatterns. Kimi sets it to the box top; every other profile leaves it unset and keeps scanning from the marker row exactly as before.

This one fixes a false CLEAN, and it was found by review, not by me. KIMI_MARKER matches │ >, findMarkerRow takes the last match, and the scan started at that row. So a draft whose final line begins with > — a pasted quote, a markdown blockquote — puts the marker on the continuation row, leaving the real draft text above the scanned region. Measured on real kimi 0.34.0:

 ╭──────────────────────────────
 │ > implement the whole feature
 │   >
 ╰──────────────────────────────

→ classified {clean: true, detail: "empty"}. A queued message would then have been typed on top of unsent user input — the exact corruption Spec 1313 removes by construction. Committed as the kimi-multiline-bare fixture, captured rather than constructed.

The bound is exclusive, mirroring the region end, and that is load-bearing rather than stylistic: the box-top row's right corner is not in the classifier's ignorable-glyph set, so an inclusive bound counted it as user text and held every idle kimi composer forever. The fixture suite caught that on the first attempt.

Why other profiles cannot reach it: they declare no regionStartPatterns, so the region starts at markerRow; and since findMarkerRow returns the last match, no row below it can match either. The set of marker-matching rows in their region is exactly {markerRow} — the previous behaviour, by construction. A test asserts all three declare no region start, and that text above the composer still classifies clean for claude and codex.

Why it is a no-op for every other app, argued and then pinned by tests:

profile marker span effect
claude ^[❯›] 1 col < 1col === 0 — literally the old rule
codex ^[❯›] 1 same
agy ^> 2 extra cell is the space, already skipped by the whitespace rule that runs before the marker check
kimi ^\s*│\s*> 4 the case this exists for

Guardrail tests (render-gate.test.ts, "marker-span exemption is a no-op…"): the exact span per shipped profile; a tightest-possible 1-char draft in the first cell the exemption could wrongly reach, per profile, all still busy (over-skipping is the only direction that could cause harm — a false CLEAN); proof that agy's span can never over-reach a typed character (>x doesn't match its marker at all); and a direct before/after demonstration that a span-2 kimi profile classifies the real 0.34.0 idle capture user-text while the shipped span-4 one classifies it clean. Every pre-existing claude/codex/agy fixture still passes unchanged.

Undocumented-surface reliance (audited against kimi 0.34.0, 2026-08-09)

Two surfaces, both dated so the reliance can be re-checked on each Kimi major:

  1. Session store ~/.kimi-code/sessions/wd_*/session_*/state.json. This has already drifted once: 0.33.0 renamed workDircwd, moved timestamps from ISO strings to epoch ms, and dropped lastPrompt. Readers accept both shapes, and codev doctor asserts the load-bearing facts explicitly and names the one that broke rather than reporting "something changed".

  2. Workspace-trust record ~/.kimi-code/workspace-trust/wd_<basename>_<sha256(root)[:12]>{root, trustedAt}. This is the maintainer veto point, so here is the full argument.

    kimi 0.33.0 added a startup "Trust this folder?" dialog. A builder worktree is always a brand-new directory; the dialog renders before any composer, and its only non-trusting option exits kimi — so an unattended builder would sit on it forever. The spawn path therefore pre-writes the trust record.

    • No sanctioned bypass exists. kimi --help has no flag; a full strings sweep of the 0.34.0 binary for KIMI_* env vars and for trust config keys found nothing (every "trust" hit was KaTeX/V8/OpenSSL noise). I looked for a supported knob first, as requested, and there isn't one.
    • What trust actually gates is narrow: whether project-level MCP servers (.mcp.json, .kimi-code/mcp.json) load from the folder. It does not gate tool execution or writes.
    • Scope: written only for a worktree Codev itself created, for a builder the human explicitly spawned, already running --yolo. It grants strictly less than launching the builder already authorized, and never touches a directory the user did not hand us.
    • Fail-soft: on any error the dialog simply appears, the gate holds the task message (no composer marker → busy), and mailbox escalation surfaces it. Never a silent misdelivery.
    • Drift is detected, not silent: codev doctor validates our derivation against kimi's own records (each record carries the root it was written for, so the expected filename is recomputable). A scheme change surfaces as a named warning instead of silently stranding every new builder on the dialog.

    My reviewers split on this one, so you should see both sides rather than just my case. Codex argued it is a real security boundary: --yolo governs approval of the agent's tool calls, whereas workspace trust governs whether repository-controlled MCP configuration is loaded and its processes started at all — so a builder spawned on a fork PR or other untrusted branch could have attacker-controlled MCP config loaded without a human ever seeing the decision. Claude reviewed the same code and reached the opposite conclusion: a --yolo builder in a Codev-created worktree already holds strictly more authority than MCP loading confers. I find Claude's reading more persuasive for the worktrees Codev creates, but Codex's fork-PR scenario is the case where the two arguments genuinely diverge, and that is a policy call I do not think is mine to settle.

    If you'd rather not ship the hash write at all, the fallback is that Kimi builders require one human keypress at first launch; say the word and I'll cut it. A narrower option, if you want the automation but not the blanket: refuse the pre-write when the worktree carries project-level MCP configuration.

Corrections to claims this PR previously made

  • "Kimi has no hook seam, so Builder worktree write-guard: prevent writes anchored at the main checkout root #1018 write-guard parity is impossible" — obsolete. Kimi documents blocking PreToolUse hooks ([[hooks]] in config.toml, exit code 2 blocks, 18 events as of 0.32.0). Parity is achievable. I have scoped it as follow-up rather than growing this PR further, but that is your call — say so and I'll add it here. Until it lands, a Kimi builder can write outside its worktree, and the docs now say exactly that.

Two decisions that are yours, not mine

A. The version floor moves 0.27.0 → 0.33.0, which drops working installs

This is a real compatibility reduction and I want it visible rather than buried in a diff. The reasoning chain:

  1. --agent-file requires ≥ 0.31.0. Below that the role does not inject at all and the builder runs silently roleless — the worst available failure mode, and not one a user would notice quickly.
  2. The folder-trust dialog appears at 0.33.0, which is what the trust pre-write exists to handle. On 0.31–0.32 there is no dialog, so that machinery is inert — but it also means those versions are a genuinely different startup path from the one I exercised.
  3. 0.33.0 is an engine boundary: it made agent-core-v2 the default. Every live measurement backing this PR — store shape, trust behaviour, the render-gate composer profile — was taken on 0.34.0, i.e. on that engine. 0.31–0.32 run the old engine and are unmeasured.
  4. Kimi ships weekly, and its store has already renamed a load-bearing field once inside this PR's lifetime. Under that cadence a narrow, evidence-backed floor is safer than a wide compatibility claim I cannot stand behind.

So the floor sits at the oldest version the evidence actually covers, not the oldest that would nominally function. If you would rather accept 0.31.0 (functional minimum, unmeasured) or hold at 0.27.0 (maximum compat, definitely broken for role injection), say which and I'll change the one constant in doctor.ts plus the docs.

B. The .builder-kimi marker is deleted — this is not a revert of your July fix

Your July REQUEST_CHANGES found a real bug: the bare launch shape (no role, no prompt) never persisted .builder-kimi-session, so an override-spawned bare Kimi builder (--builder-cmd kimi in a claude-configured workspace) fell through to claude's 80ms Enter and its mail was swallowed. That fix — touch the marker in the bare branch — was correct and shipped.

This PR removes the marker entirely, so I want to be explicit that the property your finding protected is now stronger, not weaker.

The marker was a separate artifact that every launch shape had to remember to write. That is a standing obligation, and the bare shape is precisely the one that forgot it — which is why your review caught a bug rather than a typo. Adding the missing touch fixed that instance; it did not remove the class. A future fifth launch shape could forget it again.

Pacing now reads the harness name out of the generated .builder-start.sh, which is generated from the resolved harness. There is nothing to remember: any shape that launches kimi necessarily names kimi in command position, because that is the launch. The obligation is discharged by construction rather than by discipline, and it is still override-proof for exactly your scenario — a --builder-cmd kimi spawn against a claude-configured workspace resolves kimi, because the script was generated from the override.

Tests keep your scenario pinned directly (mailbox-pacing.test.ts: "resolves kimi for the BARE launch shape too — the shape the old marker probe missed", plus an explicit override-proofness test), and spawn-worktree.test.ts pins that both generated shapes put kimi in command position, so a refactor that hid it would fail rather than silently degrade pacing.

What changed since your last review

Was Now Why
Seed bootstrap: kimi -p seed → capture session.resume_hint → pinned kimi -S <id> loop Role via --agent-file; task via the mailbox Drops 3 undocumented surfaces; role rides a system prompt instead of a user turn
seed-kick.ts: sentinel watcher + grace + BEGIN written straight to the PTY, verified via state.json.lastPrompt Deleted. Spec 1313's render gate is the readiness barrier A direct PTY write is exactly what Spec 1313 forbids; the gate already answers "is this composer empty?"
Resume: explicit -S <discovered-id> kimi -c behind a fail-closed store probe Documented flag; no undocumented id in generated bash
Pacing via a .builder-kimi marker file Harness read from the generated .builder-start.sh The marker obliged every launch shape to write one — the bare shape didn't, which was your finding last round. The launcher is generated from the resolved harness, so it cannot be forgotten or overridden away
message-pacing.ts + seedKick on createTerminal resolvePacingForSession in mailbox-wiring.ts; SeedKickRequest removed from the SDK Spec 1313 replaced the routes the old pacing hooked into — it was wired to nothing after the merge

What the 3-way review round found (and what it changed)

Before opening this for re-review I ran gemini, codex and claude over the post-merge delta, asking them to attack the shared gate edit hardest. gemini APPROVE; codex and claude both REQUEST_CHANGES — and they were right. Full dispositions are committed at codev/projects/1201-*/1201-cmap-postpivot-dispositions.md; the two blocking ones are worth stating here because they say something about where the risk in this PR actually lives:

  • The false CLEAN described above. Claude reproduced it on a constructed screen and flagged honestly that it had no live kimi to confirm the real multi-row geometry. I measured it — kimi renders exactly that shape. Claude also proposed a second input (a marker row inside a second box below the composer); measured, that one is not reachable, because kimi's / menu renders as unclosed rows with no beneath them, so anything inside it yields no-region-end → held. Both are now fixtures.
  • The store probe diverged from the TypeScript it mirrors, and the cross-check test did not catch it because it compared two implementations of the same omissions. Codex found the dangerous direction: an archived session authorized -c, which kimi then refuses to continue, producing the silently-roleless session the guard exists to prevent. Claude found the safe-but-harmful direction: readdirSync on a stray non-directory threw ENOTDIR into the single outer try, so one .DS_Store in ~/.kimi-code/sessions/ disabled resume machine-wide, permanently and silently.

Also fixed from that round: shell metacharacters in a builder id or task path could execute when the launch script printed a recovery hint (all three reviewers, from different angles); a crash loop re-queued the same task every ~2s even though the mailbox persists held rows; and both drift probes reported healthy forever after a store migration, because "any record still matches" is satisfied by the pre-migration records.

The common thread: every one of these lives in a state a happy-path run does not produce — an empty composer and a clean store both behave correctly, which is precisely why three passing live demos missed them.

One more disclosure, since it affects how you should read the demo. Two demo steps were failing when I picked this back up, and the cause was the demo, not the product: its role told the model to prefix every reply with a token, which measures whether K3 honours a persistent output-format constraint rather than whether the role was injected. The --agent-file probe, run against a production-identical agent file, passed 7/7 including role survival across kimi -c. The demo now asks for a codeword instead — the same oracle the probe uses — and carries a comment explaining why, so the weaker check does not come back.

Verification

  • pnpm build clean; full suite 4900 passed / 48 skipped / 0 failed.
  • Live demo against real kimi 0.34.0, 7/7 (codev/spikes/pir-1201-kimi-builder-demo.mjs, runs the REAL dist modules): render gate classifies the live composer; --agent-file role honored in the interactive TUI; paced multi-line delivery submits; crash → store probe → kimi -c → role survives; probe fails closed on an empty store; trust pre-write idempotent.
  • Pivot validation, 7/7 (pir-1201-kimi-agentfile-probe.mjs): --agent-file injects in both -p and the interactive TUI; TUI start mints no session, the first message mints exactly one; kimi -c resumes with the role binding intact and mints no second session.
  • Gate fixtures are real 0.34.0 captures (pir-1201-kimi-gate-measure.mjs), committed raw — they carry only throwaway /tmp paths. Idle → clean; draft, multi-line draft, the bare-> multi-line draft, / menu, @ picker and the folder-trust dialog → busy (so a blind Enter can never confirm filesystem trust).

Out of scope

Kimi as architect (stage 2); ACP / kimi server adapter; #1018 write-guard parity (now achievable — see above).

mohidmakhdoomi and others added 26 commits July 18, 2026 18:59
…ilder and architect

Seed-session bootstrap (kimi -p role seed -> capture session id from
stream-json -> TUI resume via -S) validated end-to-end; solves role
injection, initial prompt delivery, and the stored-ID session contract.
Includes reproducible POC script and full impact map / test matrix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cluesmith#1149 parity requirement

Two new observations: Kimi TUI never enters the alternate screen (no
escape-based readiness signal), and PTY input during the seed window has
no defined consumer (silently lost). Barrier design: sentinel + grace +
store-verified delivery with retry; seed carries role+task, kick is a
single BEGIN line. Architect parity correction: stored-ID resume without
an async-buildable CrashLoopFallback is cap-exhaustion outage, not cluesmith#1149
safety — ship Codex-like (stage 1) or stored-ID + async fallback (stage
2), no middle.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ipt, builder resume

- KIMI_HARNESS in harness.ts + detectHarnessFromCommand('kimi') — kills the
  cluesmith#1062 false-Claude fallthrough; buildRoleInjection throws (builder-only)
- New optional HarnessProvider.buildBuilderLaunchScript capability; Kimi
  generates the seed-session bootstrap script (idempotent seed via kimi -p
  stream-json, session.resume_hint capture, sentinel, pinned -S --yolo loop)
- kimi-session-discovery.ts: store scan / ownership verify / state reader
  (undocumented store layout, observed on kimi 0.27.0; fail-soft)
- buildResume: .builder-kimi-session precedence (ownership-verified) → store
  scan → null → fresh-with-role fallback
- spawn-worktree branches on the capability; writes .builder-seed.txt and
  passes the seedKick request through createPtySession
- Tests incl. the cluesmith#929-class regression: kimi + stale Claude jsonl never
  yields --resume <claude-uuid> or --append-system-prompt

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ss Enter pacing

- seed-kick.ts: readiness barrier — waits for the launch script's
  __CODEV_KIMI_SEED_DONE__ sentinel (writes during the seed window are
  silently lost), grace, then a store-verified BEGIN kick with an
  Enter-resend → kick-resend → loud-warn retry ladder
- createTerminal grows an optional seedKick field (core SeedKickRequest);
  handleTerminalCreate validates and arms it (malformed → ignored)
- message-write.ts: optional pacing.enterDelayMs overriding both default
  Enter delays (Kimi swallows an 80ms Enter; defaults unchanged otherwise)
- message-pacing.ts: resolves pacing per target — worktree marker probe
  first (override-proof for --builder-cmd spawns, survives Tower restarts),
  then config-resolved harness by terminal role
- Wired at all delivery paths: send direct + buffered, cron

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…store smoke probe

- Kimi in AI_DEPENDENCIES: kimi --version presence with minVersion 0.27.0
  (pins the version the undocumented surfaces were observed against)
- verifyKimi(): credential-artifact heuristic (no billed probe — Kimi
  documents no auth status command), kimi login guidance; supplementary
  'kimi doctor' config check (documented exit codes, not an auth check)
- Session-store layout smoke probe warns loudly on drift
- Architect-shell branch: kimi configured as architect → builder-only warning

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mples, skeleton mirror)

- arch.md: dedicated Kimi subsection (builder-only, seed-session bootstrap,
  sentinel-gated store-verified BEGIN, per-harness pacing, explicit-ID
  resume, undocumented-surface caveats + 0.27.0 pin, NO write-guard parity)
- agent-farm.md (instance + skeleton): builder-harness config examples

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on session type

- resolvePacingForSession wraps its whole body in try/catch: pacing is
  advisory and must never break message delivery (a missing DB in the
  tower-routes test env surfaced this as 500s on /api/send)
- CronDeps session shape carries id/cwd (the real PtySession provides both)
- tower-routes test mock gains getTerminalSessionById

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bisected on kimi 0.27.0: 80ms and 100ms swallowed; 120/250/500/1000ms
submit. Threshold ~100-120ms; shipped value stays 1000ms (~9x margin).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s pass)

codev/spikes/pir-1201-kimi-builder-demo.mjs runs the real dist modules
(script generator, armSeedKick, writeMessageToSession, buildResume)
against a real kimi PTY: seed bootstrap, sentinel-gated store-verified
BEGIN, multiline delivery at the pinned Enter delay, inner-restart
context retention, and -S resume. Executed against kimi 0.27.0 — 5/5
PASS; the ack-and-wait-with-task seed discipline held (no fallback
needed).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ier)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… consultation finding)

The delivery check used lastPrompt.includes(kickMessage) — but on a fresh
spawn lastPrompt initially holds the SEED prompt, whose ack-and-wait
wrapper itself mentions BEGIN, so the verifier reported success before the
kick ever submitted (silently defeating the swallowed-Enter recovery; the
live demo's happy path masked it). Confirmation now requires
whitespace-normalized EQUALITY (submitted messages land in lastPrompt with
newlines flattened to spaces — observed on kimi 0.27.0). Two pinning
regression tests added; live demo re-run post-fix: 5/5 PASS.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mohidmakhdoomi

Copy link
Copy Markdown
Collaborator Author

Architect Integration Review

Contributor-side review summary for maintainers (this PR was developed under the PIR protocol on our fork; we do not merge — that call is yours).

Process: Plan and dev-approval were human-gated pre-PR. The dev-approval gate included a full-path live demo through a locally installed build — a real afx spawn of a Kimi builder through Tower showing (1) seed-session bootstrap with session-id capture, (2) sentinel-gated, store-verified BEGIN delivery, (3) multiline afx send at the pinned Enter delay, and (4) inner-restart context retention via kimi -S — plus codev doctor's new kimi checks.

Consultation (CMAP, single advisory pass): gemini APPROVE, claude APPROVE, codex REQUEST_CHANGES. The codex finding was real and was accepted + fixed in 732f04b: seed-kick delivery confirmation was a substring match on lastPrompt, and the fresh-spawn seed prompt itself contains "BEGIN", so verification false-positived before the kick submitted — defeating the swallowed-Enter recovery. Fix is whitespace-normalized equality, with two pinning regression tests (both fail pre-fix), seed-kick suite 14/14, and a post-fix live demo re-run (5/5).

Architect verification of the post-CMAP fix (since PIR's single-pass consultation does not re-review fixes): I reviewed confirmed() in seed-kick.ts and the pinning tests directly — the equality predicate is correct, preserves the multiline-payload fallback (kimi flattens submitted newlines; normalization covers it), and the retry ladder (Enter re-send → one re-kick → loud warn) is intact. Known benign edge: a message raced into the session during the ~10s verify window causes one bounded re-kick, then a warning.

Scope: verified against issue #1201's builder-MVI checklist — no architect-parity changes (tower-utils/tower-instances/tower-terminals/session-manager/architect.ts untouched), no ACP adapter; kimi-as-architect fails loudly. Undocumented Kimi surfaces are labeled as observed, pinned to kimi >= 0.27.0 with a doctor smoke probe.


Architect integration review

mohidmakhdoomi and others added 2 commits July 18, 2026 20:55
… for maintainers

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cepted+fixed)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@waleedkadous waleedkadous left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outstanding work — this is one of the most disciplined external PRs we've received; the observed-vs-documented honesty around Kimi's store and the store-verified BEGIN ladder are exactly right. I'd like to see one change before merge: the bare launch path (no role, no prompt) doesn't persist .builder-kimi-session, so pacing resolution falls back to workspace config and an override-spawned bare Kimi builder gets Claude's Enter timing — the swallowed-Enter bug this PR fixes. Persisting a marker on every Kimi launch shape (plus a regression test for the override-spawn case, and softening the arch.md "exists iff Kimi-shaped" claim) closes it. I've added the area/tower label to #1201 for you.

Architect finding 4. cluesmith#1267's contract is "clean exit -> fresh rerun, no recovery",
and main's claude loop enforces it BY IDENTITY: a clean exit mints a new session id
and the superseded one is never named again. kimi cannot mint on demand and `-c` is
cwd-scoped, so identity was never pinned — the guard only asked whether ANY session
existed for this cwd. 0.33+ mints no session until the first message lands, so a
crash between a clean-exit relaunch and the first delivery found the just-ended
conversation still the newest, continued it, and delivered the re-queued task into
the conversation the human walked away from — cluesmith#1267's own motivating defect class.

The probe now answers WHICH session rather than WHETHER one exists: it prints the
newest resumable id for the cwd. The clean-exit branch records that id, and the
crash branch takes `-c` only once the newest id differs. Boolean uses derive from
"printed something", so there is still one probe and one mirror.

Measured before building, since the design assumes `-c` targets the newest session
and the existing probe only covered the zero-session case: two live sessions in one
cwd on 0.34.0, two oracles — content (codewords ALPHA/BRAVO -> BRAVO) and store
identity (only the newest session's dir was touched, nothing new minted, exit 0, no
prompt). pir-1201-kimi-continue-newest-probe.mjs.

CMAP found a defect this change INTRODUCED (claude F1, codex cluesmith#2, blocking): moving
the decision from $? onto stdout meant anything else writing to stdout counted as a
session. Measured with NODE_OPTIONS=--require preloading a module that prints — the
probe exits 1, the script read RESUME, and `kimi -c` with nothing to continue starts
a session that never saw --agent-file. Silently roleless, the cluesmith#929 class, produced
by the guard's own upgrade. Now consumes both signals.

The sketch's "empty on any error is fail-closed" was also wrong (claude F2, codex
#1): a TRANSIENT probe failure records '' and the next crash sees the ended session
as different-from-empty. Failure and empty store are now told apart by status, and
an unknown baseline blocks resume until the next clean exit re-establishes one.

Two probe/discovery divergences fixed rather than documented: `j.cwd ?? j.workDir`
short-circuited where readStateJson falls through per-field, and the probe stripped
a trailing slash before realpathSync while sameDir does not (the unsafe direction —
realpathSync already normalizes one for any directory that exists, so the strip
bought nothing).

Tests: the composition is now driven for real — the actual `while` loop with stubbed
launches, asserting resume,fresh,fresh — because injecting the superseded id from
the test left the generated assignment pinned only by a string match. Non-vacuity is
demonstrated by running the pre-fix predicate against the same store.

Full suite 4915 passed / 48 skipped / 0 failed.
…spositions

arch.md's kimi crash-resume section described an existence guard; it now describes
the identity one, the measured fact it rests on (`kimi -c` continues the newest
session for a cwd), and the two accepted residuals — the in-memory superseded id
(contract parity with claude's per-process minted id) and a store GC that evicted
newest-first.

Plus the builder thread and the full CMAP disposition record: gemini APPROVE, codex
and claude REQUEST_CHANGES, every finding accepted, including the blocking one that
this round's own change introduced.

@waleedkadous waleedkadous left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, the thing that matters most: this sat un-re-reviewed for 26 days after you reported it green on Aug 9. That was our failure, not yours — you did the work, including a drift audit most contributors would have skipped, and the evidence discipline here (live demo against the real CLI, doctor drift-probes weighted by recency, honest documentation of the trust write) is exemplary. The 3-way integration review (gemini APPROVE; codex and claude REQUEST_CHANGES, both high confidence) confirms the design is right; the changes below are mostly a consequence of main moving underneath the branch while it waited.

Findings

  1. Workspace-trust pre-write (security, verified against the branch). ensureKimiWorkspaceTrust writes ~/.kimi-code/workspace-trust/wd_… at spawn. Your arch.md note argues it grants strictly less than launching the builder already did — but the one thing the record gates (loading project-level MCP servers from .mcp.json / .kimi-code/mcp.json) is repository-controlled content, so a branch that commits an MCP config gets its processes loaded without the human ever seeing Kimi's dialog. That's the #1328 class. Refuse the pre-write when the worktree carries project-level MCP config, and/or gate it behind an explicit config opt-in.
  2. Stale against three rebuilt seams. 1,603 commits behind and CONFLICTING: the write edge is now submitMessagePaced + in-lock precheck (#1365), gate details are consolidated under MailboxGateDetail/isUnverifiableVerdict (#1482), and marker anchoring moved to cursor-row/palette (#1474). Two of this PR's edits produce real post-merge defects against them — its new gate details bypass the #1482 consolidation and re-fork isClassifierStuck locally, so a stuck Kimi hold would render as "a human at the line" with no escalation. launchLoopTail is already on main.
  3. Unmeasured under verified delivery. The 7/7 demo predates #1573/#1584 (echo-verification before delivered; zero re-writes). Kimi's echo behaviour is exactly the kind of thing that produced the #1583 loop — it needs measuring, not assuming.
  4. The approved plan describes the retired design (seed-session / PTY-kick / -S, Kimi 0.27). The shipped architecture (mailbox + --agent-file + guarded -c + trust) was never re-approved, so the human-approved artifact no longer matches the code.
  5. No PreToolUse write guard for Kimi builders (#1018 class) — documented, accepted as a follow-up by all three lanes.

What happens next — we'll do it, not you. Given the delay was ours and the conflicts are semantic (the delivery-path edits have to be re-derived against the converged code, not merged), I'm opening a PIR re-plan lane that builds on this branch: merge main, re-derive the write-edge and gate-detail integration, add the MCP-config refusal, update the plan to the shipped architecture for re-approval, and re-run the live demo under verified delivery. Your commits and authorship stay intact; the lane adds on top. If you'd rather drive it yourself, say so and I'll hand it back. Thank you for the patience — and for the audit that made this tractable.

@waleedkadous

Copy link
Copy Markdown
Contributor

Re-plan lane opened: #1620 (PIR — plan re-approval and dev-approval gates, since the approved plan is stale and the trust change is security-relevant). It builds on this branch directly; nothing here is rebased or rewritten.

waleedkadous and others added 4 commits September 4, 2026 17:03
…nverged main

Two artifacts, both for the plan-approval gate:

- codev/plans/1620-…md — this lane's plan. Measures the actual divergence
  (merge-base 4983ea8; 17 shared paths, only five needing semantic
  re-derivation), specifies the delivery-path re-derivation, the
  workspace-trust security refusals, the re-measurement, and the demo re-run.

- codev/plans/1201-…md — rewritten. The approved plan still described the
  retired seed-session/PTY-kick design, which the 2026-08-09 pivot replaced;
  it now describes the shipped architecture (mailbox task delivery,
  --agent-file role injection, guarded `kimi -c` resume, trust record,
  0.33.0 floor) as amended by this lane's security decisions.

Flagged rather than silently handled: "drop launchLoopTail changes already on
main" does not match main (still module-local in spawn-worktree.ts, and the PR
only relocates it); kimi has drifted 0.34.0 → 0.41.0; and items 3/5 are blocked
on an authenticated Kimi CLI, which is not available in this environment.

Builds on Mohid Makhdoomi's branch — merge-only, no rebase or squash.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
The issue body was the architect's distillation; auditing the plan against the
raw gemini/codex/claude KEY_ISSUES closed four gaps:

- A real defect found chasing claude's §7 (builder self-send attribution): the
  generated Kimi script queues its task with `afx send` from the worktree, but
  spawn.ts registers the builder row only AFTER the session starts, and
  detectCurrentBuilderId throws when the row is missing. Lose that race and the
  script warns once, never retries, and the builder starts with no mission —
  today only node's startup latency prevents it. Bounded retry planned;
  reordering upsertBuilder rejected (blast radius on the shared spawn path).
  Also: the sender resolves to the builder's own id, so the task arrives framed
  as a peer message from itself — `--raw` instead.

- The write-guard follow-up is bounded per both lanes that raised it: filed
  before merge, referenced from the docs where kimi is documented as supported,
  the stale "no hook seam" claim corrected, maintainer acceptance recorded
  explicitly (codex's condition).

- Echo verification's ~2.2s/send cost recorded as accepted, not discovered later.

- A KEY_ISSUES disposition table covering all three lanes — the review doc's
  skeleton, since the acceptance bar is "addressed or explicitly dispositioned".

Notes one internal conflict in the claude lane (§2 wants multi-row-draft
non-stuck; §3 argues the opposite) and takes the escalate branch, as confirmed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
@waleedkadous

Copy link
Copy Markdown
Contributor

Hi @mohidmakhdoomi — a heads-up and a request.

We are executing the re-plan described in #1620 directly on this branch: merging current main in (no rebase, no squash, your commits and authorship stay as they are), re-deriving the delivery-path integration against the write-edge, hold-verdict and marker-anchoring changes that landed while this sat, adding the workspace-trust refusals, and rewriting the 1201 plan to describe the shipped architecture. The 26-day gap that made this necessary is on us, not you.

One thing we cannot do on our side: we have no authenticated Kimi CLI here, and the CLI is now at 0.41.0, seven minors past the 0.34.0 you measured. So when our commits land, could you review the delta and re-run the live demo (codev/spikes/pir-1201-kimi-builder-demo.mjs) plus the render-gate captures on the current CLI? We will post a short, exact checklist here at that point so it is one pass for you rather than an open-ended ask.

Thank you for the original work — the design held up to a full 3-way review; only the branch had drifted.

waleedkadous and others added 18 commits September 5, 2026 11:09
…d to @mohidmakhdoomi

Human decision 2026-09-05 — no authenticated Kimi maintainer-side and no
credentials available, so this lane does not run items 5 and 6. They go to the
original contributor, who has an authenticated Kimi; his evidence attaches to
PR cluesmith#1203.

- Items 5-6 rewritten as a handoff, with the consequences stated rather than
  implied: KIMI_PROFILE ships on 0.34.0-era measurement, markerRequiresCursorRow
  is not adopted (it was conditional on captures we can no longer take), and
  Kimi's echo behaviour stays unmeasured by us.
- dev-approval re-scoped to non-Kimi regression proof. With no Kimi to exercise,
  what our gate can prove is that a change for Kimi moved nothing else — and
  render-gate.ts, message-write.ts and hold-verdict.ts carry claude, codex and
  agy delivery for every user.
- A seven-step handoff checklist for @mohidmakhdoomi in item 7, to be posted on
  cluesmith#1203 when the implementation commits land: exact commands, the eight fixture
  names, the one question we cannot answer, and where evidence goes.
- New risk recorded: we ship a Kimi feature none of us ran, on measurements
  seven minors old. Mitigation is procedural and partial, and says so.

The 1201 plan's test section now attributes the live demo to the contributor and
records which facts were measured on which version.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
Architect standing rule: this lane does not post to PR cluesmith#1203 or any contributor
thread; outward artefacts are drafted to /tmp for human approval.

Folded into the plan as its own section rather than left in the thread log,
because item 7 previously read "posted as a PR comment the moment the
implementation commits are pushed" — which a later reader could have taken as an
instruction to this lane. The section also names what the rule does NOT cover,
so it is unambiguous later: commits to builder/pir-1201 continue (they are the
deliverable, not a message), and reading the PR continues.

Verified nothing had already been posted: the PR's last comment and review are
the maintainer's and the contributor's own.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
…esmith#1567 write edge

Main moved again while the gate was pending. Verified against origin/main
rather than taken on trust, and two things fall out:

- writeMessageToSession's 5th parameter is now `strategy: WriteStrategy` —
  exactly the slot the PR wanted for `pacing`, which moves to 6th there and 7th
  on submitMessagePaced. Two Enter delays still, so the seam's shape survives.

- One of those constants is a trap. The long-frame Enter is now
  PASTE_ENTER_DELAY_MS = 80, measured safe on claude and codex. Kimi's own
  bisect found 80ms and 100ms are SWALLOWED. The new default lands precisely on
  Kimi's measured failure point, reintroducing the original cluesmith#1201 symptom via a
  change that had no reason to know Kimi exists.

- writeStrategyForApp defaults every unlisted harness to BRACKETED_PASTE, so
  registering KIMI_PROFILE would silently opt Kimi into a paste mode nobody has
  tested it against. If Kimi does not implement it, the markers land as literal
  composer text and framePieces' \n → \r conversion submits every line
  separately — the cluesmith#584 class, worse than before it was fixed. Kimi therefore
  joins agy in PLAIN_CHUNKED until measured, which is what that function's own
  doc comment already prescribes for an unmeasured harness.

Handoff checklist gains step 8 (does Kimi honour bracketed paste?), and the
test plan pins both the strategy and the Enter override on the long-frame branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
Architect asked for the evidence, not the summary — right call, prose gets
skimmed. Both plans now carry the table: 80ms and 100ms swallowed and never
submit; 120/250/500/1000ms submit; threshold ~100-120ms; pinned at 1000ms for
~9x margin, re-verified on 0.34.0 under agent-core-v2.

Stated next to it that PASTE_ENTER_DELAY_MS = 80 was measured 0/29 losses on
claude 2.1.263 and codex 0.146.0 — sound for those two, silent about the one CLI
whose paste-detection window is the reason this seam exists. Putting the two
numbers side by side is what makes the collision unmissable next time.

Also pinned: the unit test asserts the override on BOTH frame branches. A test
covering only the short frame would pass while the feature was broken for every
real message, since a formatted `afx send` is almost always >= 4 lines.

The opt-out-default hazard is the architect's follow-up and is explicitly fenced
out of this PR, recorded in the plan so a later reader does not widen the diff:
fixing the default touches every harness's write path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
…-paste strategy

Reverses this plan's earlier proposal to list Kimi alongside agy as unmeasured.
writeStrategyForApp is now untouched by this work; cluesmith#1653 is closed.

Removed from both plans, but replaced with a record of the decision rather than
silence: "Kimi is not in the opt-out list" reads identically whether it was
considered or missed. The test plan now ASSERTS writeStrategyForApp('kimi') ===
BRACKETED_PASTE, so the decision is pinned and a later edit to that function has
to be deliberate about Kimi.

Mohid's checklist step 8 sharpened rather than dropped. The observable symptom of
an unhonoured bracket is not only stray [200~ text — framePieces converts \n to
\r inside the bracket, so those become Enter keypresses and one message arrives
as N submissions, one per line. Step 8 now asks the specific question ("one
message, or several?") because a vague question returns a vague answer.

Kept as instructed: the enterDelayMs override on the paste path's Enter, and the
bisect table.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
Merge, not rebase — Mohid Makhdoomi's 47 commits and their authorship are
untouched. 1,606 commits of main since the merge base at 4983ea8.

Seven conflicts, resolved as re-derivations rather than textual picks:

- message-write.ts — the write edge moved twice under this branch. cluesmith#1365 turned
  writeMessagePaced into submitMessagePaced (per-terminal lock + in-lock
  precheck); cluesmith#1567 replaced per-line pacing with bracketed-paste chunking and
  took the 5th parameter slot for `strategy`. MessagePacing survives, re-homed as
  the 6th/7th parameter, and now overrides BOTH Enter sites — including the new
  PASTE_ENTER_DELAY_MS (80ms), which is exactly the value Kimi's bisect showed is
  swallowed. Overriding only the short branch would leave every real send broken.

- mailbox-delivery.ts — deleted this branch's local CLASSIFIER_STUCK_DETAILS
  fork; isClassifierStuck delegates to isUnverifiableVerdict, per cluesmith#1482's rule
  that there be exactly one definition of "will this hold clear on its own?".

- hold-verdict.ts / db/types.ts / db/schema.ts — the two new details land in all
  three places. no-region-start mirrors no-region-end; multi-row-draft escalates
  deliberately, since it is the one verdict reached when the classifier could not
  count cells and inferred from box geometry.

- render-gate.ts — cluesmith#1474's cursor/palette marker anchors and this branch's
  region-start bounding are orthogonal and both kept. Also generalized
  markerFgPalette off its hardcoded getCell(0), which is right for every
  row-start marker and wrong for the first profile whose marker is not at column
  0 — precisely what Kimi's boxed `│ >` is.

- mailbox-wiring.ts / tower-routes.ts — pacing resolved at the binding (not
  through the port, unlike `strategy`, which the delivery module derives from the
  gate profile it holds) and on the --interrupt write. --escape stays unpaced,
  with the reason recorded.

- fixtures/gate/README.md, render-gate.test.ts — main's real 1.1.13 agy captures
  supersede the synthesized 1.1.8 ones; the kimi entries are additive. Added a
  note that the kimi fixtures are 0.34.0 and their re-capture is the contributor's
  round, so the version is read as part of the claim.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
…exposed

Full suite: 5916 passed, 0 failed.

Three causes:

1. bugfix-584-send-multiline-pacing (3 failures) — the pacing tests passed
   `pacing` in the 5th argument slot, which cluesmith#1567 took for `strategy`, and
   asserted per-line timings that no longer exist. Rewritten against the real
   write edge, and WIDENED: the override is now pinned on the long/bracketed
   branch (where it displaces PASTE_ENTER_DELAY_MS = 80, the value Kimi's bisect
   showed is swallowed) and on the plain-chunked branch, not just the short one.
   A formatted `afx send` is almost always >= 4 lines, so the long branch is the
   one real messages take — the old short-only coverage would have stayed green
   while the feature was broken for every message anyone sends.

2. render-gate span guardrail (1 failure) — cluesmith#1474 gave AGY_PROFILE cursor-row and
   palette-12 marker anchors, so this branch's synthetic agy screen stopped being
   a marker row at all and the test reported no-composer-marker. The assertion was
   about the marker SPAN, so rather than delete the agy case, added an `agyScreen`
   helper that satisfies the anchors (SGR-94 marker glyph, explicit CUP parking the
   cursor on the composer row, which sits above its bounding rule). Applied to the
   sibling `>x` test too: it still passed, but for the wrong reason — it would have
   failed the anchors before ever exercising the pattern it claims to test.

3. kimi-session-discovery (1 failure) — NOT caused by this merge; both the test and
   its implementation are byte-identical to the pre-merge branch. "ok when at least
   one session carries the load-bearing shape" wrote a good session then a bad one
   and expected `ok`, but the probe deliberately reports drift when the NEWEST
   session is the broken one. It only passed where the two mkdir mtimes tied, so it
   was platform-dependent all along: 5/5 failures on APFS, where mtimeMs is
   sub-millisecond. Fixed with the explicit `touchDir` ordering the very next test
   in the same describe already uses, and its doc now says why every test here must.

Also fixed a merge slip in render-gate.ts: keeping both sides of the GateProfile
conflict dropped the `/**` opening main's markerRequiresCursorRow doc block.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
… an MCP refusal

Suite: 5934 passed, 0 failed.

ensureKimiWorkspaceTrust wrote a trust record for any worktree, unconditionally,
on every kimi builder spawn. The original argument was that this grants strictly
less than the --yolo the builder already runs with. That holds for tool
execution and not for what trust actually controls: whether kimi loads MCP
servers DEFINED BY THE FOLDER. Auto-approving tool calls and letting a checkout
introduce new tool-providing processes are different boundaries, and spawning a
builder onto a contributor branch is a normal flow here.

Two independent refusals, both defaulting to doing nothing:

- project-mcp-config: the worktree ships .mcp.json or .kimi-code/mcp.json.
  Refused even when opted in — the one case where the decision has teeth is the
  one case a human makes. Existence only; the file is never parsed, because a
  folder shipping a BROKEN .mcp.json is still a folder defining servers and a
  parse error must not be what decides we may trust it.
- not-opted-in: the default. harnessOptions.kimi.autoTrustWorkspace, a new
  config namespace deliberately separate from `harness` (whose every entry is
  validated at load against a shape requiring roleArgs/roleScriptFragment, so a
  settings entry there would throw during loadConfig; and built-ins win
  resolution, making `harness.kimi` inert). A security opt-in does not belong in
  a namespace where the neighbouring key is silently ignored.

The MCP check is evaluated BEFORE the opt-in so the log states the strongest
true reason: someone who has opted in and still sees no record needs to hear
"this worktree ships MCP config", not "you did not opt in", which would be false
and implies opting in would fix it.

ensureKimiWorkspaceTrust now returns a KimiTrustDecision rather than a boolean,
because "no record was written" covers a deliberate refusal, an already-trusted
worktree and a failed write — and an operator debugging a builder stalled on the
dialog needs to know which. Every outcome is logged at the call site.

Also replaces the deleted CLASSIFIER_STUCK_DETAILS fork's real value. The
exhaustiveness it bought is now a compile-time tripwire in mailbox-delivery.ts
plus runtime assertions in a new test. The tripwire is in SOURCE, not the test,
because this package excludes the __tests__ glob from tsc — a `satisfies` there
compiles nothing and would have read like a guarantee while enforcing nothing.
Verified by temporarily widening GateVerdict['detail']: both assertions fail with
the message telling you where to classify it. A second assertion catches
MailboxGateDetail (what the column stores) drifting from what the classifier
produces — the exact divergence cluesmith#1482 was filed for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
…aming, document the opt-in

Suite: 5940 passed, 0 failed.

THE RACE (found chasing a "smaller" note in the claude review lane, which only
asked what a builder self-send attributes to). The generated kimi script queues
its task with `afx send` from inside the worktree. spawn.ts starts the session at
spawn.ts:482 and only THEN registers the builder row at :488, while
detectCurrentBuilderId() THROWS when that row is missing (send.ts:167, the cluesmith#1094
anti-spoofing guard). Lose the race and afx fatals, the script warned once and
never retried within that launch, and the builder came up with a role and no
mission — the only trace a line in its own pane. The sole thing preventing it was
node's startup latency exceeding one local HTTP round-trip, which is a
coincidence, not a guarantee.

codev_queue_task now retries to a bounded deadline (30s, CODEV_TASK_QUEUE_DEADLINE_SECS)
before warning. Reordering upsertBuilder ahead of startBuilderSession was the
tempting root fix and is rejected: the row carries terminal_id, which does not
exist until the session is created, so it would mean two upserts on the path
every harness shares.

THE ATTRIBUTION. The sender resolves to the builder's OWN id, so the opening
mission arrived framed `### [BUILDER <id> MESSAGE -> <id>] ###` — a spawn prompt
presented as a peer message from itself, and there is no self-send guard in
handleSend. .builder-prompt.txt is already fully framed, so it now goes with
--raw and arrives as itself.

Tests: the retry and the raw send are pinned both as script text and as
BEHAVIOUR — a stub afx that fails until a sentinel appears reproduces the lost
race, and a second stub that never succeeds pins the fail-soft giving-up path.
Also added `bash -n` parsing of every generated shape: generated shell is the one
artifact here no type checker reads, and this change hit exactly that (a
backtick in a shell comment closed the TypeScript template literal, and a bare
${...} would have been interpolated by JS). The pre-existing queue-state test's
stub afx now reads the body as ${@: -1} rather than $3, so adding a flag to the
send does not turn into a red test with nothing wrong.

Docs (both trees): the trust opt-in, why the default is off, the MCP refusal, and
the consequence that a repo shipping a root .mcp.json needs one interactive trust
per worktree. The write-guard gap is now stated where kimi is documented as
supported, per the review lane's condition for accepting it as follow-up.

Demo driver: scenario 6 rewritten for KimiTrustDecision; 6b (no opt-in -> no
record) and 6c (opted in + .mcp.json -> refused) added. Its own spawn now passes
the opt-in explicitly — without that the demo's kimi would have opened on the
trust dialog and step 1 would have hung rather than failed usefully.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
…tes, and the escalation call

Three passages the cluesmith#1620 changes made stale or incomplete:

- The trust paragraph still argued from "trust grants strictly less than the
  --yolo the builder already runs with" to an unconditional pre-write. That
  reasoning is now stated AND rebutted in place, because the conclusion changed
  but the narrowness that motivated it did not: auto-approving tool calls and
  letting a checkout introduce tool-providing processes are separate boundaries.
  Documents both gates, the KimiTrustDecision return, the ordering (MCP refusal
  first, so the log states the strongest true reason), and the consequence for
  repos shipping a root .mcp.json.

- The pacing paragraph described one Enter delay. Since cluesmith#1567 there are two, and
  the new one is 80ms — the first row of Kimi's own bisect. Both are now named
  with the full bisect numbers, and the reason the long branch is the one that
  matters (a formatted afx send is almost always >= 4 lines) is stated so a
  future refactor cannot lose it by covering only the short branch.

- multi-row-draft and growsWithDraft were absent from arch.md entirely, as was
  the escalation decision. Added, including why multi-row-draft escalates (it is
  the one verdict reached when the classifier could not COUNT and inferred from
  geometry), its accepted cost, and why the local Record fork was deleted in
  favour of a type-level tripwire in source rather than a satisfies in a test
  file this package excludes from tsc.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
…le one

The gemini CMAP lane caught something the issue's scope missed. cluesmith#1620 named the
1201 PLAN as needing a rewrite; the 1201 REVIEW doc was stale in exactly the same
way and for the same reason — it still described seed-kick.ts, message-pacing.ts,
.builder-seed.txt and the `kimi -S` loop, none of which exist. A review artifact
describing code that is not there is worse than none: it is a confident wrong
answer for whoever reads it next. Rewritten to the shipped architecture, with a
header saying what changed and why, and the cluesmith#1620 amendments folded in.

Also adds this lane's own review doc, which dispositions every KEY_ISSUE from all
three lanes of the 2026-09-04 round, states plainly that nobody here ran Kimi,
and names the 0.34.0 -> 0.41.0 drift as the same staleness that made cluesmith#1203
un-mergeable, recurring.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
… knew

Two genuinely new, in the COLD Testing section:

- A suite that pins only the cheap branch can stay green while the feature is
  broken on the branch real inputs take. The Kimi Enter override was tested with
  a short frame; every formatted `afx send` is >= 4 lines and takes the long one,
  whose delay is exactly the value Kimi swallows.
- A test whose outcome depends on two filesystem operations landing in the same
  timestamp tick is platform-dependent, not flaky — different bug, different fix,
  and it will look like neither until you run it several times.

The third finding is NOT new, and saying so is the point. cluesmith#1401 already records
"a guard is not a guard until you have watched it fail", including a type-test
file the build never compiles. What was new is that the same trap survives one
directory INSIDE src/, under an excluded __tests__ glob, where that entry's
"outside src/" heuristic misses it — so it gained a clause (c) rather than a
third near-duplicate entry competing with it. Its own rule was followed: the
union was widened and tsc watched to go red before the guard was trusted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
Suite: 5942 passed, 0 failed.

The existing eight tests all call resolvePacingForSession directly. Every one of
them would have kept passing through both times this seam silently came unwired:
cluesmith#1365 moved submitMessagePaced's parameter list, and cluesmith#1567 then inserted
`strategy` into the exact slot pacing occupied. "The resolver is correct" and
"the resolver is connected to the delivery path" are different claims, and only
the first was covered.

These two go through the real binding — makeDeliveryPorts().writeMessage — with a
long frame (the branch a formatted `afx send` actually takes) and observe the wire
rather than an argument: a kimi builder's Enter lands ~1s after the body, a claude
builder's on the default. Costs about a second each, which is the price of
testing a delay.

Verified by removing resolvePacingForSession from the binding and watching the
kimi case go red, then restoring it — the same discipline lessons-learned cluesmith#1401
asks for, applied to the guard rather than to the thing guarded.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
… lane)

Suite: 5951 passed, 0 failed.

The codex lane caught that my own "generalization" changed nothing. KIMI_MARKER
is anchored (/^\s*│\s*>/), so RegExp.exec().index is always 0 — meaning
markerSpanStart returned exactly what the hardcoded getCell(0) returned, and a
future kimi markerFgPalette would still have sampled the box edge. The trap this
helper was written to remove was still armed, now under a comment claiming
otherwise. Verified before fixing: for ' │ > ', m.index is 0 and the glyph is at
column 3.

The glyph is now identified explicitly by a NAMED group (?<glyph>…). Named, not
positional, for a reason I found by breaking it: the first fix used capture group
1, and agy's marker /^>(\s|$)/ already had a group 1 — its trailing separator.
Sixteen agy tests went red at once because the anchor began sampling the space
after the marker. A positional convention collides with any incidental group; a
named one cannot.

Tests assert the CHARACTER at the returned index for every shipped profile, not
the number — an index is only meaningful against the glyph it should land on, and
asserting `3` would have passed for the wrong reason had the pattern changed.
Plus the end-to-end proof codex asked for: a kimi profile carrying
markerFgPalette:12 classifies a boxed composer clean, and rejects one whose glyph
renders in palette 9, so the anchor is shown to discriminate rather than merely
not-crash. Reverting to the inert version turns 3 of them red.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
… doc contradictions

Suite: 5965 passed, 0 failed. All three lanes returned REQUEST_CHANGES and all
three were right.

claude's findings, all valid and all from the plan's own Test Plan:
- validateHarnessOptions / kimiAutoTrustWorkspace had ZERO coverage — on a
  security opt-in whose validator throws inside loadConfig, the universal config
  path. 14 tests added, including that it fails CLOSED when config is unreadable
  for unrelated reasons.
- writeStrategyForApp('kimi') was unasserted, so the owner's 2026-09-08 decision
  to keep Kimi on bracketed paste was reversible with a green suite. Pinned, with
  the reasoning, so a future edit has to be deliberate about kimi rather than
  inheriting the default by silence — which is how kimi would have inherited it.
- hold-verdict-exhaustive.test.ts's header claimed its `satisfies` "fails to
  compile" on divergence. It does not: this package excludes __tests__ from tsc.
  That contradicted what the same commit had correctly documented in
  mailbox-delivery.ts. Corrected, and the correction left visible in the file —
  a comment that overstates a guard is worse than none, because it stops the next
  person looking for the real one.

codex's findings:
- arch.md quoted the queue command without --raw. Fixed, and the paragraph now
  also carries why --raw and why the retry exist.
- The review doc lacked Files Changed / Commits / How to Test Locally. Added; the
  test instructions are deliberately all non-Kimi, matching the dev-approval scope.
- Follow-up issues have no numbers. Drafted to /tmp for approval rather than
  filed — filing is outward, and the standing rule routes that through a human.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5KHfN4ChsJZNqBRStMky8
@mohidmakhdoomi

Copy link
Copy Markdown
Collaborator Author

@waleedkadous although MoonshotAI/kimi-code#2767 remains unfixed (so --agent-file and --agent <custom_name> do not work), a potential workable approach is to create a Kimi agent profile (e.g. .kimi-code/agents/codev-builder.md) in the builder worktree, with name: agent + override: true + ${base_prompt} in the body → role present in the system prompt, Kimi's own default prompt interpolated ahead of it. This new agent profile replaces the default profile Kimi falls back to while keeping the role scoped to the builder worktree. This is documented behaviour (kimi's agents guide names it explicitly), not a back door. Then use launch the builder without --agent or --agent-file because the agent profile is the default Kimi cli binds anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Kimi Code CLI as a builder

2 participants