From 3bfea6132f67c457b221dd891481502bf7e18875 Mon Sep 17 00:00:00 2001 From: jun Date: Thu, 27 Aug 2026 18:25:44 +0900 Subject: [PATCH 1/5] fix(kiro): enable the code-mode catalog nudge so ALL_TOOLS discovery reaches Kiro Kiro was the one adapter that never opted into the code-mode branch of the tool catalog nudge. src/adapters/kiro.ts called buildNonOpenAIToolCatalogNudgeFromNames without the optional codeModeExecName argument, so codeModeExecWireName() returned undefined and Kiro received the generic nested-helper sentence that never names ALL_TOOLS. Codex code mode advertises ONE freeform exec and reaches spawn_agent and every other deferred helper through nested tools.(...) calls. Without the ALL_TOOLS sentence a routed Kiro model concludes no delegation tool exists: a live kiro/claude-opus-5 task reported "No spawn/subagent tool exists in my current tool catalog." while multi_agent_v1__spawn_agent was in fact registered at 6598 chars with model, reasoning_effort and service_tier intact. Decide code mode on the EMITTED catalog rather than the requested list. The freeform flag only exists on the requested objects, but the shape that matters is what the model receives: the count/byte budget can drop a requested exec_command while exec survives, and a requested-list scan would then find a shell bridge the model cannot call and suppress code mode for a catalog that is code-mode-shaped. Intersecting the two also makes tool_choice:"none" and budget omission correct by construction, since both empty the emitted set. Export isCodexCodeModeExecTool and isBareShellBridgeTool so the Kiro call site reuses the shared semantics instead of matching on the tool name. Plan and audit: devlog/_plan/260827_kiro_subagent_delegation_unblock/ --- .../000_plan.md | 132 ++++++++++++ .../010_exec_discovery_preservation.md | 117 +++++++++++ .../011_root_cause_nudge_gap.md | 197 ++++++++++++++++++ .../012_audit_round1_synthesis.md | 98 +++++++++ .../020_unbounded_kiro_delegation.md | 102 +++++++++ .../030_verification.md | 87 ++++++++ src/adapters/kiro.ts | 26 ++- src/adapters/tool-catalog-nudge.ts | 4 +- tests/kiro-adapter.test.ts | 93 +++++++++ 9 files changed, 853 insertions(+), 3 deletions(-) create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/000_plan.md create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/010_exec_discovery_preservation.md create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/011_root_cause_nudge_gap.md create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/012_audit_round1_synthesis.md create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/020_unbounded_kiro_delegation.md create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/030_verification.md diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/000_plan.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/000_plan.md new file mode 100644 index 00000000000..eccd22ebed5 --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/000_plan.md @@ -0,0 +1,132 @@ +# Kiro subagent delegation — unblock parallel spawning + +Unit: `260827_kiro_subagent_delegation_unblock` · opened 2026-08-27 + +## Owner decision that scopes this unit + +Two things that look like defects are deliberate and stay: + +- The **1024-char description cap** for unverified Kiro models is intentional. + Raising it broke Kiro before (see "Why the cap exists"), so this unit does not + raise it as a blanket change. +- The **5-model `spawn_agent` override list** is intentional + (`MAX_MODEL_OVERRIDES_IN_SPAWN_AGENT = 5`). Not a bug, not in scope. + +What the owner *does* want: **Kiro subagents allowed to spawn in parallel, +without a depth ceiling.** That is the goal of this unit. + +## Symptom (measured, not inferred) + +A top-level `kiro/claude-opus-5` task reports that no delegation tool exists: + +> No spawn/subagent tool exists in my current tool catalog. + +It then lists a 26-name top-level surface with no `spawn_agent` in it. The tool +is in fact registered — a forced `ALL_TOOLS` probe in the same session found +`multi_agent_v1__spawn_agent` at 6598 chars with `model`, `reasoning_effort`, +and `service_tier` all intact. + +So the capability is present and the model cannot see it. + +## Mechanism + +> **Superseded by the P-phase re-read. See `011_root_cause_nudge_gap.md`.** +> The truncation described below is REAL and measured, but it is the second +> line of defense, not the cause. The actual cause is that +> `src/adapters/kiro.ts:477` omits the optional `codeModeExecName` argument, so +> `codeModeExecWireName()` returns `undefined` and Kiro receives a generic +> fallback sentence that never names `ALL_TOOLS` — regardless of any cap. +> This section is retained because the truncation evidence is what located the +> gap, and because the cap would re-break discovery if the nudge ever regressed. + +Codex code mode does not advertise `spawn_agent` at the top level. It defers the +nested tools and tells the model where to look, inside the `exec` tool +description: + +> Some deferred nested tools may be omitted from this description. They are +> still available on the global `tools` object and listed in `ALL_TOOLS`. + +That sentence sits past the 1024-char mark. `toolDescriptionLimit()` in +`src/adapters/kiro-tools.ts:143` gives 9216 chars to `gpt-5.6-sol` and 1024 to +every other Kiro model, so a Claude-family Kiro model receives `exec` truncated +at exactly 1024 chars ending in `…`. + +Confirmed from inside the Kiro session: + +| probe | Kiro model sees | control (native) | +|---|---|---| +| `exec` description length | ~1010-1020, ends `…` | full | +| contains `ALL_TOOLS` | NO | YES | +| contains `Some deferred nested tools…` | NO | YES | +| contains `Shared MCP Types` / `declare const tools` | NO | YES | + +The failure chain, as corrected at P: + +```text +kiro.ts:477 omits codeModeExecName + -> codeModeExecWireName() returns undefined + -> generic fallback replaces the ALL_TOOLS sentence in the system nudge + -> (and independently: exec's own description is cut at 1024, removing it there too) + -> model concludes no spawn tool exists + -> zero delegation attempts, no model/effort/tier override ever formed +``` + +The model is not misreading the spawn schema. It never learns the schema is +reachable. + +## Why the cap exists (do not regress this) + +`devlog/_fin/143_kiro-gateway-parity/125_phase_tool_fallback_hardening.md` +originally moved long descriptions **into the system prompt** and left a pointer +in the tool spec. That was later replaced: `tests/kiro-adapter.test.ts:690` is +now named *"tool descriptions use deterministic model-specific caps **without +prompt injection**"* and asserts `current.content` does **not** contain +`### Tool documentation`. + +So there are two rejected designs already on record: + +1. Send the full long description to Kiro — the reason the cap was added. +2. Relocate it into the prompt — explicitly reverted; the test now guards + against it. + +Any proposal in this unit that reintroduces either is out of bounds. + +## Design constraint that follows + +> **Superseded.** This section reasoned toward a truncation-strategy fix inside +> `kiro-tools.ts`. The implemented direction is `011`: pass `codeModeExecName` +> at the nudge call site, which delivers the discovery contract as system text +> and never touches the cap. + +The reasoning is kept because its conclusion still holds and still constrains +the unit: the load-bearing content is ONE sentence (`ALL_TOOLS` discovery), not +the 5.5 KB of type declarations around it. That is why a system-nudge delivery +works — it carries the sentence without carrying the payload. + +It also names the residual risk `012` records: the nudge is truncated from the +TAIL under budget pressure, and the code-mode sentence sits late in it. Today +there is ~10 KB of headroom, so the risk is dormant, not absent. + +## Work phases + +| phase | file | scope | +|---|---|---| +| wp1 | **`011_root_cause_nudge_gap.md`** | pass `codeModeExecName` at Kiro's nudge call site so the shipped `ALL_TOOLS` sentence reaches Kiro | +| wp2 | `020_unbounded_kiro_delegation.md` | remove the depth-1 ceiling for Kiro parents (host config, not code) | +| wp3 | `030_verification.md` | live proof + full gates + PR against `dev` | + +`010_exec_discovery_preservation.md` is **superseded** by `011`. It is retained +for its reasoning about why the description tail matters, which is what led to +finding the real call-site gap. Do not implement from `010`. + +wp1 is a prerequisite for wp2. Lifting the depth ceiling changes nothing while +the parent still believes the tool does not exist. + +## Non-goals + +- Raising `MAX_KIRO_TOOL_DESCRIPTION_UNVERIFIED` as a blanket change. +- Changing `MAX_MODEL_OVERRIDES_IN_SPAWN_AGENT`. +- Reintroducing tool documentation into the Kiro system prompt. +- Touching `multi_agent_version` catalog pins — `kiro/claude-opus-5` already + pins `v1` correctly in both `~/.codex/models_cache.json` and + `~/.codex/opencodex-catalog.json`. diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/010_exec_discovery_preservation.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/010_exec_discovery_preservation.md new file mode 100644 index 00000000000..708040450b6 --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/010_exec_discovery_preservation.md @@ -0,0 +1,117 @@ +# wp1 — restore `ALL_TOOLS` discovery for Kiro code-mode turns + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp1` + +> **P-phase amendment, 2026-08-27.** The original draft of this document assumed +> the fix had to be a truncation-strategy change inside `truncateDescription()`. +> Re-reading the tree at P found the discovery sentence **already written and +> already shipped** in `src/adapters/tool-catalog-nudge.ts:122`, injected as a +> system nudge that bypasses the 1024 cap entirely — and found that Kiro is the +> one adapter that never enables it. The root cause moved one file; the fix got +> much smaller. The superseded preserve-tail design is kept below as the +> rejected alternative, because the reasoning about *why* the tail matters is +> what led here. + +## The one sentence that must arrive + +```text +Some deferred nested tools may be omitted from this description. They are +still available on the global `tools` object and listed in `ALL_TOOLS`. +``` + +~150 chars. Everything the parent needs to discover `spawn_agent` is in it. +A Kiro Claude model currently receives none of it, because the cap keeps the +first 1024 chars of a 5.5 KB description and this sentence is not in that +window. + +## Why head-truncation is the wrong shape here + +`truncateDescription()` (`src/adapters/kiro-tools.ts:150`) is a pure prefix +slice with a surrogate-safe boundary. It is correct as a byte-safety routine +and wrong as a *documentation* routine: it assumes the first N chars are the +most valuable N chars. + +For `exec` that assumption inverts. The head is prose framing ("Run JavaScript +code to orchestrate/compose tool calls"), which the model can infer from the +tool name. The tail carries the discovery contract and the type declarations, +which it cannot infer at all. + +## Candidate approaches + +### A. Preserve-tail truncation for `exec` only — recommended + +Keep the cap. Change *which* 1024 chars survive for the code-mode tool: keep a +head budget for framing, then append the discovery sentence verbatim. + +Sketch: + +```ts +const KIRO_DISCOVERY_HINT = + "Deferred nested tools are omitted here; they are callable as " + + "`await tools.(...)` and enumerable via the `ALL_TOOLS` global."; +``` + +When a description exceeds the limit **and** the original text contains +`ALL_TOOLS`, reserve the hint's length, head-truncate to `limit - hint.length`, +and append. Output stays exactly at the cap, so the wire-size property the cap +exists to protect is unchanged. + +- Payload size: identical to today (still capped at 1024). +- Blast radius: only descriptions that mention `ALL_TOOLS` — in practice the + single code-mode `exec` tool. +- Does not touch the prompt, so `tests/kiro-adapter.test.ts:700` + (`not.toContain("### Tool documentation")`) stays green. + +### B. Per-tool allowlist limit + +Give `exec` a larger cap while every other unverified-model tool stays at 1024. +Simple, but it is a narrower version of the change that previously broke Kiro, +and it needs a live probe per model family to establish the real ceiling. Keep +as fallback if A proves insufficient. + +### C. Do nothing in the adapter; fix upstream ordering + +Ask for the discovery sentence to move to the top of the `exec` description. +Correct in principle, out of this repository's control, and leaves every +shipped Codex version broken. Rejected as the primary path. + +## Chosen path + +**A**, with **B** held in reserve. + +## Implementation targets + +| file | change | +|---|---| +| `src/adapters/kiro-tools.ts` | add discovery-preserving branch inside the truncation path | +| `tests/kiro-adapter.test.ts` | regression next to the existing cap test at :690 | + +## Regression contract + +The new test must assert all four, or it is not proving the fix: + +1. A >1024-char description containing `ALL_TOOLS` still yields + `spec.description.length === 1024` for an unverified model. +2. That output **contains** `ALL_TOOLS`. +3. A >1024-char description **without** `ALL_TOOLS` is byte-identical to + today's behavior (plain head-truncate ending in `…`). +4. `current.content` still does not contain `### Tool documentation`. + +Assertion 3 is what keeps this from becoming a general cap change. Drive the +test red once before wiring the fix — a preserve-tail assertion passes +vacuously if the fixture description happens to be under the cap. + +## Verification + +```bash +bun x tsc --noEmit +bun test tests/kiro-adapter.test.ts +``` + +Focused per `AGENTS.md`: this is a scoped adapter change, not shared runtime. + +## Exit criterion + +A live `kiro/claude-opus-5` task, asked what tools it has, names +`multi_agent_v1__spawn_agent` **without being told where to look**. Today the +same prompt returns "No spawn/subagent tool exists in my current tool catalog." diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/011_root_cause_nudge_gap.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/011_root_cause_nudge_gap.md new file mode 100644 index 00000000000..48e027b0773 --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/011_root_cause_nudge_gap.md @@ -0,0 +1,197 @@ +# wp1 (revised) — Kiro never enables the code-mode nudge + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp1` + +Supersedes the approach in `010`. Same symptom, correct mechanism, much smaller +fix. + +## The sentence already exists + +`src/adapters/tool-catalog-nudge.ts:122` already emits exactly what the Kiro +model was missing, as a **system nudge** rather than a tool description: + +> Absence from the top-level catalog or from `exec`'s description is not +> absence: deferred helpers stay callable on `tools.`. Discover them from +> the isolate global `ALL_TOOLS`, not `tools.ALL_TOOLS`. + +This is emitted only when `verifiedCodeModeExecName` is truthy. Otherwise the +builder falls through to a generic sentence that never names `ALL_TOOLS`: + +> If a listed tool exposes nested helpers such as a tools.* API, call the listed +> parent tool and use those helpers only inside that tool's input. + +That fallback is what Kiro receives today. + +## Why Kiro alone falls through + +There are two entry points into the same builder, and they are not equivalent: + +| entry point | passes `codeModeExecName`? | used by | +|---|---|---| +| `buildNonOpenAIToolCatalogNudgeForTools` (:133) | yes — detects via `isCodexCodeModeExecTool` | anthropic, google, openai-chat, command-code | +| `buildNonOpenAIToolCatalogNudgeFromNames` (:98) | only if the caller supplies it | **kiro (:477)** | + +`src/adapters/kiro.ts:477` calls the name-only entry point with two arguments +and omits the optional third: + +```ts +const toolCatalogNudge = buildNonOpenAIToolCatalogNudgeFromNames( + kiroToolWireNames(kiroTools), + name => advertisedAlias.get(name) ?? name, +); // <- codeModeExecName never supplied +``` + +`codeModeExecWireName()` (:90) then returns `undefined` for the reason its own +doc comment states: the name-only path *cannot* decide code mode, because +`kiroToolWireNames()` (`kiro.ts:111`) has already reduced the tool objects to +strings and thrown away the `freeform` flag that distinguishes Codex's +JavaScript `exec` from an ordinary structured tool named `exec`. + +So this is not a missing feature. It is a **capability the module documents, +implements, and tests — that Kiro's call site never opts into.** + +## Corrected causal chain + +```text +kiro.ts reduces tools to wire names (freeform flag lost) + -> codeModeExecName omitted at the call site + -> codeModeExecWireName() returns undefined + -> generic fallback sentence replaces the ALL_TOOLS sentence + -> model never learns deferred helpers are discoverable + -> "No spawn/subagent tool exists in my current tool catalog." +``` + +The 1024-char cap is real and does delete the same sentence from the `exec` +description — but it is now the *second* line of defense, not the cause. Fixing +the nudge restores discovery without touching the owner-confirmed cap at all. + +## Diff plan + +### MODIFY `src/adapters/kiro.ts` + +Detect code mode from `parsed.context.tools` — the objects, while `freeform` is +still present — then map to the Kiro wire alias and pass it in. + +Constraints that make this non-trivial and must be honored: + +1. **Alias coordinate system.** The nudge compares `codeModeExecName` against + the advertised set, which for Kiro holds *aliases* (`registry.alias()` + output), not raw wire names. Pass the alias or the guard silently rejects it. +2. **Never call `registry.alias()` to resolve it.** `alias()` REGISTERS + (`kiro-wire.ts:90-95`: on a miss it calls `kiroToolName`, writes `wireToKiro`, + and may write `nameMap`). The existing comment at `kiro.ts:471-474` warns that + calling it for a non-advertised tool pollutes the collision domain. Read + `advertisedAlias`, which is already built directly above the call site. +3. **Respect `tool_choice`.** `convertKiroToolContext` drops every tool when + `toolChoice === "none"` (`kiro-tools.ts:189`); a code-mode name must not be + advertised for a turn whose catalog is empty. +4. **Both predicates run over the EMITTED catalog, not the requested list.** + *(Amended after audit round 1, blocker 1 — High.)* + + The naive implementation evaluates `isCodexCodeModeExecTool` and + `isBareShellBridgeTool` over `parsed.context.tools`. That is wrong in a + reproducible way, and the reviewer reproduced it with a 49-tool catalog: + `exec` is emitted while `exec_command` is dropped by the count/byte budget + (`kiro-tools.ts:215-218`). Scanning the REQUESTED list then finds the omitted + bridge and suppresses code mode — even though the catalog the model actually + receives is code-mode-shaped and has no shell bridge in it. + + The model can only call what was emitted, so the shape that matters is the + emitted one. Resolve BOTH predicates over requested tool objects whose + resolved alias is present in the emitted-name set: + + ```ts + const emitted = new Set(kiroToolWireNames(kiroTools)); + const inCatalog = (t: OcxTool) => + emitted.has(advertisedAlias.get(namespacedToolName(t.namespace, t.name)) + ?? namespacedToolName(t.namespace, t.name)); + ``` + + This keeps constraints 3 and 4 satisfied by construction: an empty catalog + yields an empty `emitted` set, so nothing can be named. + +### MODIFY `src/adapters/tool-catalog-nudge.ts` + +Export `isCodexCodeModeExecTool` and `isBareShellBridgeTool` so `kiro.ts` reuses +the shared predicates instead of duplicating the semantics. Both are currently +module-private; no behavior change, export only. + +### MODIFY `tests/kiro-adapter.test.ts` + +See `030_verification.md` for the regression contract. + +## Scope boundary + +IN: the Kiro call site, two predicate exports, tests. +OUT: `MAX_KIRO_TOOL_DESCRIPTION_UNVERIFIED`, `MAX_MODEL_OVERRIDES_IN_SPAWN_AGENT`, +the nudge's wording, every other adapter. + +## Accept criteria + +1. A Kiro turn advertising a freeform `exec` and no bare shell bridge produces a + system prefix containing `ALL_TOOLS`. +2. A Kiro turn advertising a **structured** `exec` (`freeform` absent) does NOT + contain `ALL_TOOLS` — it keeps the generic fallback. This is the assertion + that proves the semantic predicate is doing the work rather than a name match. +3. A Kiro turn advertising `exec` **and** `exec_command` does NOT claim code + mode: that is the flat-bridge shape. +4. **Emitted-catalog resolution, both directions** *(amended, blocker 1/2):* + - 4a. A budget-omitted `exec_command` beside an emitted freeform `exec` still + yields code mode. This is the blocker-1 regression and it must be driven + red first. + - 4b. A budget-omitted `exec` is NOT named, even though it was requested. + + The original criterion 4 ("the name reaching the nudge is the alias") is + **withdrawn as non-discriminating**. The reviewer is right: the only valid + code-mode tool is bare `exec`, and `kiro-wire.ts:53-55` passes an already- + valid name through unchanged, so asserting `alias === "exec"` cannot tell a + correct alias lookup from a raw-name shortcut. Alias transformation is + covered at its own seam in `tests/kiro-wire`-level naming tests; membership + in the emitted catalog is what this unit must prove. +5. `tests/kiro-adapter.test.ts:690` stays green — no prompt-injected tool docs. + +Criteria 2 and 4b are the activation-grounding controls +(C-ACTIVATION-GROUNDING-01): they are the cases that must NOT fire. Without +them a name-only implementation passes every other assertion. + +## Verification + +```bash +bun x tsc --noEmit +bun test tests/kiro-adapter.test.ts tests/tool-catalog-nudge.test.ts +bun run test # required: this is an adapter change (src/AGENTS.md:26) +``` + +`bun run test` is the completion gate, not the focused run *(amended, blocker +4)*. `src/AGENTS.md:26` requires the full suite for adapter behavior, and this +change is in the adapter layer. + +## Documentation disposition *(blocker 5 — rebutted with evidence)* + +`src/AGENTS.md:28` requires a `docs-site/` update when a change affects +user-visible behavior or configuration. + +**Correction (audit round 2).** An earlier draft of this section claimed the +behavior was undocumented and cited a no-hit `rg`. That was wrong — the search +covered `reference/` and missed `guides/`. The behavior IS documented: + +`docs-site/src/content/docs/guides/codex-integration.md:314-316`: + +> In normal routed code mode, Codex can expose deferred MCP/app tools through +> the official `exec` tool's `tools` global and `ALL_TOOLS`; that path does not +> require the model to see or call `tool_search`. + +That page is written adapter-neutrally and describes the behavior this change +RESTORES for Kiro. So the page does not become wrong — it becomes true for one +more provider. No `docs-site/` edit is required to keep the docs honest, which +is what `src/AGENTS.md:28` exists to protect. + +The correction matters more than the conclusion it survived. The original +rebuttal reached the right answer through a false premise: had the guide instead +said this path works on every routed provider, the same evidence would have +demanded a docs change, and a no-hit `rg` would have hidden that. + +No configuration surface, flag, or command is added. The devlog unit is the +durable record. A `reference/` section covering the nudge across ALL adapters +would be a genuine improvement, but it is its own unit, not a rider on a +one-provider parity fix. diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/012_audit_round1_synthesis.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/012_audit_round1_synthesis.md new file mode 100644 index 00000000000..294c2a9ae75 --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/012_audit_round1_synthesis.md @@ -0,0 +1,98 @@ +# wp1 — audit round 1 synthesis (REVIEW-SYNTHESIS-01) + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp1` · A phase + +Reviewer verdict: **FAIL**, 5 blockers (1 High, 4 Medium). Recorded before any +re-dispatch, per REVIEW-SYNTHESIS-01. + +## Disposition + +| # | severity | disposition | where | +|---|---|---|---| +| 1 | High | **accepted** — real defect in the planned implementation | `011` constraint 4 rewritten | +| 2 | Medium | **accepted** — criterion withdrawn as non-discriminating | `011` accept criteria 4a/4b | +| 3 | Medium | **accepted** — stale cross-references | `000` phase table, `030` rollback | +| 4 | Medium | **accepted** — full suite is the gate | `011` verification block | +| 5 | Medium | **rebutted with evidence** | `011` documentation disposition | + +## Root cause of blocker 1 (the one that mattered) + +The plan said "detect code mode from `parsed.context.tools` — the objects, while +`freeform` is still present." That sentence is half right and the half that is +wrong is load-bearing. + +`freeform` does only exist on the requested objects, so detection must start +there. But the *decision* is about the shape of the catalog the model receives, +and those two sets differ whenever the budget drops something. The reviewer +built the divergent case: 49 tools, `exec` emitted, `exec_command` omitted. A +requested-list scan sees a shell bridge that the model cannot call and concludes +"not code mode" — suppressing the exact sentence this unit exists to restore, +in precisely the crowded-catalog sessions where delegation matters most. + +The fix is to intersect: take the predicates from the requested objects, then +keep only those whose alias survived into the emitted set. + +A pleasing side effect is that constraints 3 (`tool_choice: "none"`) and 4 +(budget omission) stop being separate rules to remember. Both reduce to "the +emitted set is empty, so nothing is named." Fewer independent invariants is +fewer ways to be wrong later. + +## Why blocker 5 is rebutted rather than folded + +`src/AGENTS.md:28` is a real instruction and the reviewer was right to raise it. +The rebuttal is factual, not procedural: `rg` across `docs-site/src/content/docs` +finds no mention of `ALL_TOOLS`, `code mode`, or the catalog nudge anywhere, +including `reference/adapters.md`. There is no page this change makes untrue. + +The instruction exists to stop docs drifting from code. Writing the first-ever +description of an internal prompt-assembly detail, as a rider on a parity bug +fix, does not serve that purpose — and a partial description covering only Kiro +would itself be misleading, since four other adapters have had this behavior all +along. + +## Cross-blocker conflicts + +None. Blockers 1 and 2 both push toward the emitted catalog as the source of +truth and were folded as one amendment. + +## Independent corroboration + +A second read-only lane audited the injection budget in parallel and returned +**BOUND-RISK: NONE**: `boundedInjectedInstruction` (`kiro.ts:398`) truncates from +the tail and the code-mode sentence sits late in the nudge, so a budget squeeze +*would* remove exactly the restored sentence — but a maximum-pressure synthetic +catalog measured 6,176 injected chars against a 16,384 bound, leaving 10,208 +free. The code-mode nudge is 718 chars longer than the fallback it replaces. + +Recorded because the margin is a fact this unit now depends on: if +`MAX_KIRO_TOOL_COUNT` or the nudge grows substantially later, the tail-truncation +ordering becomes live and the sentence is what gets cut first. + +## Round 2 + +Verdict moved **FAIL -> GO-WITH-FIXES (blockers=1)**. + +| # | round-2 status | +|---|---| +| 1 High | CLOSED — reviewer re-reproduced 4a (code mode now enabled) and 4b (suppressed), and confirmed the intersect has no hole: namespaced `exec` is excluded by `tool-catalog-nudge.ts:32`, and the completion tool cannot collide because `kiro-wire.ts:87-88` rejects that requested name | +| 2 Medium | CLOSED — 4b shown deterministically testable: equal priority keeps request order (`kiro-tools.ts:210`), so 48 fillers then `exec` omits `exec` | +| 3 Medium | **STILL-OPEN in round 2, closed after** — `000_plan.md` still asserted the superseded cause; now carries supersession headers on Mechanism, the failure chain, and the design-constraint section | +| 4 Medium | CLOSED | +| 5 Medium | CLOSED, **with a factual correction against me** | + +### The correction worth keeping + +My round-1 rebuttal of blocker 5 cited an `rg` that returned no hits for +`ALL_TOOLS` in `docs-site/`. That was false. The reviewer found +`guides/codex-integration.md:314-316` describing exactly this path; my search +had covered `reference/` and missed `guides/`. + +The conclusion survived — the page is adapter-neutral, so restoring Kiro parity +makes it *more* true rather than wrong — but it survived for a different reason +than I gave. Had that guide said the path already worked on every routed +provider, the correct disposition would have flipped to a docs edit, and a +no-hit search would have concealed it. + +This is the concrete value of the adversarial lane: it caught a load-bearing +claim that was convenient and wrong, in the one section where I was arguing +against the reviewer rather than with them. diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/020_unbounded_kiro_delegation.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/020_unbounded_kiro_delegation.md new file mode 100644 index 00000000000..fb21a18f461 --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/020_unbounded_kiro_delegation.md @@ -0,0 +1,102 @@ +# wp2 — allow unbounded parallel Kiro delegation + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp2` + +Depends on wp1. Lifting a ceiling the parent cannot see changes nothing. + +## Owner intent + +> kiro 서브에이전트 무한 병렬 파견을 허용할께 + +Read as two separate limits, because they are enforced in different places: + +- **depth** — can a spawned child spawn again? Today: no. +- **width** — how many children may run at once? Today: an upstream default. + +## Measured starting state + +`ocx v2 status` / `ocx agent` on this host: + +```text +multi_agent_v2: OFF +multi_agent_mode: v1 — ALL models forced to v1 surface +agents.enabled: (unset — upstream default true) +agents.max_depth: (unset — upstream default 1) +max_threads: (unset — codex default) +``` + +`agents.max_depth` is V1-only upstream (`src/codex/features.ts:334`), and this +host runs the V1 surface, so it is live rather than ignored. + +Depth 1 is why the first probe was inconclusive: a spawned child (`Nash`) +correctly had no spawn tool. That was the ceiling behaving as configured, not +the bug. The bug only became visible at the **top level**, where depth is not +the constraint and the tool was still missing. + +## Levers that already exist + +No new subsystem is needed. Every control is implemented: + +| lever | reader | writer | CLI | +|---|---|---|---| +| `[agents] max_depth` | `getAgentsMaxDepth()` `src/codex/features.ts:345` | `setAgentsMaxDepth()` :819 | `ocx v2 ...` | +| `[agents] enabled` | `src/codex/features.ts` | same | `ocx agent` | +| concurrent threads | `maxConcurrentThreadsPerSession` | `src/server/management/agent-settings-routes.ts:385` | `ocx v2` | + +Validation already bounds `agentsMaxDepth` to signed i32 +(`agent-settings-routes.ts:293-296`), so "unbounded" in practice means a large +finite depth, not a null sentinel. + +## Decision needed before implementing + +**Is unbounded depth global, or Kiro-scoped?** + +`[agents] max_depth` is a Codex-global TOML key. It has no provider dimension. +Raising it lifts the ceiling for *every* provider on this host, not only Kiro. + +Two options: + +1. **Global raise** — set `max_depth` high once. One line, immediate, and + honest about what it does. Every provider gets it. +2. **Kiro-scoped** — would require a new opencodex-owned gate, since upstream + has no per-provider depth. Materially more work and a new config surface. + +Recommend option 1 unless the owner wants other providers held at depth 1. This +is the one open question in this unit. + +## Runaway risk, stated plainly + +Unbounded depth × unbounded width is exponential. A depth-5 tree where each +parent spawns 4 children is 1024 leaf agents, each holding Kiro quota. + +Two properties make that survivable, and both should be verified rather than +assumed: + +- **Width still bounds the tree.** `maxConcurrentThreadsPerSession` caps + simultaneity even when depth is unlimited. Confirm it applies per-session and + not per-parent before relying on it. +- **Kiro quota fails closed.** The adapter already has retry/cooldown handling + (`src/adapters/kiro-retry.ts`); exhaustion should surface as a refused spawn, + not a wedged tree. + +Deliberate non-recommendation: do not add a depth-based safety valve in this +unit. The owner asked for the ceiling removed; adding a different ceiling in its +place would not honor that. Width is the correct place for a bound. + +## Implementation + +```bash +# after the depth question is answered +ocx v2 status # capture before-state +# raise [agents] max_depth via the existing writer, then: +ocx v2 status # prove the new value is persisted +``` + +Config writes go through `setAgentsMaxDepth()` rather than hand-editing +`~/.codex/config.toml`, so the existing validation and write serialization +apply. + +## Exit criterion + +`ocx v2 status` reports the raised depth, and a spawned Kiro child can itself +spawn a grandchild. wp3 proves it end to end. diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/030_verification.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/030_verification.md new file mode 100644 index 00000000000..650d0788446 --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/030_verification.md @@ -0,0 +1,87 @@ +# wp3 — live verification that a Kiro parent delegates in parallel + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp3` + +A green unit test proves the adapter emits the right bytes. It does not prove a +Kiro model *acts* on them. This unit's whole failure mode was a capability that +existed and went unused, so the exit gate is behavioral. + +## What does not count as proof + +- `bun test tests/kiro-adapter.test.ts` green — necessary, not sufficient. +- The model reciting `ALL_TOOLS` contents when told to look there. That is the + forced probe that already passed *before* any fix; it proves registration, + not discovery. +- `ocx v2 status` showing a raised depth — proves config, not behavior. +- A spawn attempt that errors. A refused spawn is not a completed delegation. + +## Probe ladder + +Each rung must pass before the next is meaningful. + +### 1. Unprompted discovery (proves wp1) + +Fresh top-level `kiro/claude-opus-5` task. Ask what tools are available for +delegating work. **Do not mention `ALL_TOOLS` or `exec`.** + +Pass: it names `multi_agent_v1__spawn_agent` on its own. +Baseline to beat: *"No spawn/subagent tool exists in my current tool catalog."* + +### 2. Single spawn with explicit overrides + +Ask it to spawn one child with an explicit `model` and `reasoning_effort`. + +Pass: `spawn_agent` returns an `agent_id`, and the child reports the requested +model and effort — not the inherited parent values. This is the rung that +closes the original report ("모델 지정, 에포트, 속도 지정을 못하는거 같아"). + +### 3. Parallel width + +Ask for three independent children in one round, then `wait_agent` on all three. + +Pass: three distinct `agent_id`s, overlapping lifetimes, all reaching a final +status. Sequential completion means width is still serialized somewhere and the +parallel claim is unproven. + +### 4. Depth (proves wp2) + +Instruct a spawned child to spawn a grandchild. + +Pass: the grandchild completes and returns through the chain. Baseline: at +depth 1 the child has no spawn tool at all — the exact `Nash` result from +triage. + +## Evidence to capture per rung + +Record verbatim, not paraphrased: + +- the `agent_id` values returned, +- each child's self-reported model and effort, +- `ocx observe logs` rows showing the Kiro upstream calls, +- for rung 3, the wall-clock overlap. + +## Rollback trigger + +*(Rewritten after audit round 1, blocker 3: wp1 is now nudge activation, not +preserve-tail truncation.)* + +Revert wp1 if enabling the code-mode nudge for Kiro produces a Kiro +`ValidationException`, a 400, or a measurable behavior regression on any Kiro +model family. The change only swaps one sentence of injected system text for a +longer one, so the plausible failure is size or wording interaction, not schema +rejection — but the cap history says treat Kiro payload changes as guilty until +proven green. + +Do NOT respond to such a failure by raising +`MAX_KIRO_TOOL_DESCRIPTION_UNVERIFIED` or by moving tool docs into the prompt. +Both are owner-confirmed out of bounds; a failure here means the unit returns to +P, not that the constraints bend. + +Revert wp2 if unbounded depth produces a tree that does not terminate or +exhausts Kiro quota without a clean refusal. + +## Unit closure + +All four rungs pass, evidence recorded here, then move the unit directory to +`devlog/_fin/`. Per `AGENTS.md`, a `_fin` unit records work already visible in +public git history, so closure follows the shipped fix rather than preceding it. diff --git a/src/adapters/kiro.ts b/src/adapters/kiro.ts index 052d5b29fc6..b4161be1964 100644 --- a/src/adapters/kiro.ts +++ b/src/adapters/kiro.ts @@ -33,6 +33,7 @@ import type { OcxTextContent, OcxToolCall, OcxToolResultMessage, + OcxTool, OcxUsage, } from "../types"; import type { ProviderAdapter } from "./base"; @@ -42,7 +43,7 @@ import { sniffImageDimensions } from "./anthropic-image-guard"; import { fetchKiroWithRetry, noteKiroTransientThrottle } from "./kiro-retry"; import { convertKiroToolContext } from "./kiro-tools"; import { identifyRoutedModel } from "./identity"; -import { buildNonOpenAIToolCatalogNudgeFromNames } from "./tool-catalog-nudge"; +import { buildNonOpenAIToolCatalogNudgeFromNames, isBareShellBridgeTool, isCodexCodeModeExecTool } from "./tool-catalog-nudge"; import { KIRO_COMPLETION_INSTRUCTIONS, KIRO_COMPLETION_RETRY_MESSAGE, @@ -474,9 +475,32 @@ export function buildKiroPayload( // name for a tool that was never advertised and pollute the collision domain. const advertisedAlias = new Map(); for (const [alias, wireName] of registry.nameMap) advertisedAlias.set(wireName, alias); + // Code mode is decided on the EMITTED catalog, not the requested list. + // + // `freeform` only exists on the requested tool objects -- `kiroToolWireNames` has already + // reduced the emitted catalog to strings -- so the predicates must read the objects. But the + // SHAPE that matters is the one the model receives: the count/byte budget can drop a requested + // `exec_command` while `exec` survives, and scanning the requested list would then find a shell + // bridge the model cannot call and suppress code mode for a catalog that is code-mode-shaped. + // Intersecting the two keeps `tool_choice: "none"` and budget omission correct for free: both + // empty the emitted set, so nothing can be named. + const emittedToolNames = new Set(kiroToolWireNames(kiroTools)); + const emittedAlias = (tool: OcxTool): string | undefined => { + const wireName = namespacedToolName(tool.namespace, tool.name); + // Read the recorded mapping; `registry.alias()` would REGISTER a name here. + const alias = advertisedAlias.get(wireName) ?? wireName; + return emittedToolNames.has(alias) ? alias : undefined; + }; + const requestedTools = parsed.context.tools ?? []; + const emittedCodeModeExec = requestedTools.find(tool => isCodexCodeModeExecTool(tool) && emittedAlias(tool)); + const emittedShellBridge = requestedTools.some(tool => isBareShellBridgeTool(tool) && emittedAlias(tool)); + const codeModeExecName = emittedCodeModeExec && !emittedShellBridge + ? emittedAlias(emittedCodeModeExec) + : undefined; const toolCatalogNudge = buildNonOpenAIToolCatalogNudgeFromNames( kiroToolWireNames(kiroTools), name => advertisedAlias.get(name) ?? name, + codeModeExecName, ); const boundedNudge = toolCatalogNudge ? boundedInjectedInstruction(toolCatalogNudge, injectedChars) : undefined; if (boundedNudge) systemParts.push(boundedNudge); diff --git a/src/adapters/tool-catalog-nudge.ts b/src/adapters/tool-catalog-nudge.ts index b03a3b6a41f..b14ff098372 100644 --- a/src/adapters/tool-catalog-nudge.ts +++ b/src/adapters/tool-catalog-nudge.ts @@ -28,7 +28,7 @@ const NEIGHBOR_AGENT_TOOL_NAMES = ["Read", "Grep", "Glob", "Bash", "LS"] as cons const CODEX_UNIFIED_EXEC_TOOL_NAME = "exec"; const CODEX_SHELL_BRIDGE_TOOL_NAMES = ["exec_command", "shell_command"] as const; -function isCodexCodeModeExecTool(tool: Pick): boolean { +export function isCodexCodeModeExecTool(tool: Pick): boolean { return !tool.namespace && tool.name === CODEX_UNIFIED_EXEC_TOOL_NAME && tool.freeform === true; } @@ -45,7 +45,7 @@ function isCodexCodeModeExecTool(tool: Pick): boolean { +export function isBareShellBridgeTool(tool: Pick): boolean { return !tool.namespace && (CODEX_SHELL_BRIDGE_TOOL_NAMES as readonly string[]).includes(tool.name); } diff --git a/tests/kiro-adapter.test.ts b/tests/kiro-adapter.test.ts index 17398265bde..571208a8b81 100644 --- a/tests/kiro-adapter.test.ts +++ b/tests/kiro-adapter.test.ts @@ -1237,3 +1237,96 @@ describe("boundedInjectedInstruction surrogate safety", () => { expect(Buffer.byteLength(result!, "utf8")).toBeGreaterThan(0); }); }); + +describe("kiro code-mode catalog nudge", () => { + // Codex code mode advertises ONE freeform `exec` and reaches everything else through nested + // `tools.(...)` helpers. The nudge sentence naming `ALL_TOOLS` is the only place a routed + // model learns those helpers are discoverable, and Kiro was the one adapter that never enabled + // it: `buildNonOpenAIToolCatalogNudgeFromNames` was called without `codeModeExecName`, so a + // Kiro model concluded no spawn/subagent tool existed and never delegated. + async function kiroSystemText(tools: unknown[], modelId = "claude-sonnet-4.5"): Promise { + const { body } = await createKiroAdapter(provider).buildRequest(parsedWith([{ role: "user", content: "hi" }], tools, modelId)); + return JSON.parse(body).conversationState.currentMessage.userInputMessage.content as string; + } + + const codeModeExec = { name: "exec", description: "Run JavaScript", freeform: true, parameters: { type: "object" } }; + + test("names ALL_TOOLS when a freeform exec is advertised without a bare shell bridge", async () => { + const content = await kiroSystemText([codeModeExec, { name: "wait", description: "Resume", parameters: { type: "object" } }]); + + expect(content).toContain("ALL_TOOLS"); + expect(content).toContain("Codex code mode"); + // The generic fallback must be gone, not merely accompanied. + expect(content).not.toContain("If a listed tool exposes nested helpers such as a tools.* API"); + }); + + test("keeps the generic fallback for a STRUCTURED tool that merely shares the name exec", async () => { + // A provider may advertise an ordinary structured `exec` that takes a shell string. Telling + // that turn its body is JavaScript would be false, so the semantic `freeform` flag decides — + // not the name. This is the control: a name-only implementation passes every other assertion + // in this block and fails here. + const content = await kiroSystemText([{ name: "exec", description: "Run a shell command", parameters: { type: "object" } }]); + + expect(content).not.toContain("ALL_TOOLS"); + expect(content).toContain("If a listed tool exposes nested helpers such as a tools.* API"); + }); + + test("does not claim code mode when a bare shell bridge sits beside exec", async () => { + const content = await kiroSystemText([codeModeExec, { name: "exec_command", description: "Run a shell command", parameters: { type: "object" } }]); + + expect(content).not.toContain("ALL_TOOLS"); + }); + + test("decides on the EMITTED catalog: a budget-omitted shell bridge no longer suppresses code mode", async () => { + // Resolving the predicates over the REQUESTED list is wrong in a reproducible way: the bridge + // can be dropped by the count budget while `exec` survives, so the model receives a + // code-mode-shaped catalog containing no shell bridge at all. Suppressing the nudge there + // withholds discovery in exactly the crowded-catalog sessions where delegation matters most. + const filler = Array.from({ length: MAX_KIRO_TOOL_COUNT - 1 }, (_, index) => ({ + name: `filler_${String(index).padStart(3, "0")}`, + description: `Filler ${index}`, + parameters: { type: "object" }, + })); + const { body } = await createKiroAdapter(provider).buildRequest( + parsedWith([{ role: "user", content: "hi" }], [...filler, codeModeExec, { name: "exec_command", description: "Run a shell command", parameters: { type: "object" } }]), + ); + const current = JSON.parse(body).conversationState.currentMessage.userInputMessage; + const emitted = current.userInputMessageContext.tools.map((tool: { toolSpecification: { name: string } }) => tool.toolSpecification.name); + + expect(emitted).toContain("exec"); + expect(emitted).not.toContain("exec_command"); + expect(current.content).toContain("ALL_TOOLS"); + }); + + test("does not name a code-mode exec that the catalog budget dropped", async () => { + // The mirror of the case above, and the reason the fix intersects rather than unions: a tool + // the model cannot call must never be named as its execution surface. Equal-priority tools + // keep request order, so 48 fillers ahead of `exec` omit `exec` deterministically. + const filler = Array.from({ length: MAX_KIRO_TOOL_COUNT }, (_, index) => ({ + name: `filler_${String(index).padStart(3, "0")}`, + description: `Filler ${index}`, + parameters: { type: "object" }, + })); + const { body } = await createKiroAdapter(provider).buildRequest( + parsedWith([{ role: "user", content: "hi" }], [...filler, codeModeExec]), + ); + const current = JSON.parse(body).conversationState.currentMessage.userInputMessage; + const emitted = current.userInputMessageContext.tools.map((tool: { toolSpecification: { name: string } }) => tool.toolSpecification.name); + + expect(emitted).not.toContain("exec"); + expect(current.content).not.toContain("ALL_TOOLS"); + }); + + test("advertises no code mode for a tool_choice:none turn", async () => { + const parsed = { + modelId: "claude-sonnet-4.5", + stream: true, + options: { toolChoice: "none" }, + context: { messages: [{ role: "user", content: "hi" }], tools: [codeModeExec] }, + } as unknown as OcxParsedRequest; + const { body } = await createKiroAdapter(provider).buildRequest(parsed); + const content = JSON.parse(body).conversationState.currentMessage.userInputMessage.content as string; + + expect(content).not.toContain("ALL_TOOLS"); + }); +}); From 8f702b2b51ef1d8611f96f43d78d03f7782283e5 Mon Sep 17 00:00:00 2001 From: jun Date: Thu, 27 Aug 2026 18:59:09 +0900 Subject: [PATCH 2/5] fix(kiro): reserve catalog room for the code-mode execution path Under Codex code mode, shell, file edits, apply_patch and every MCP helper are reachable only as nested tools.(...) calls inside one freeform exec tool. Kiro's catalog budget ranked that tool as ordinary filler, so a crowded session could drop it and emit a catalog in which every admitted tool is uncallable. Cursor pins its execution path for the same reason (request-builder.ts, #399). Priority alone does not fix it. loadedFromToolSearch tools outrank exec and arrive unbounded -- the Responses parser pushes every tool_search_output spec -- so a session holding MAX_KIRO_TOOL_COUNT loaded tools exhausts the count budget before reaching the exec tier and drops the one tool that makes the other 48 callable. Reserve instead of evict. The fill loop projects exec into every count and byte check, so it admits one fewer tool rather than taking one back. That distinction is what keeps #2475's loaded-result guarantee intact: Cursor's evictNonExecutionPath exempts only the execution path and could evict a loaded tool. Byte checks measure the projected final array because the budget is computed over the serialized array, where separators shift. Omissions are now derived by set difference. The previous candidates.slice(omittedAt) assumed every candidate after the first rejection was omitted, which stops being true once one of them is reserved and admitted -- the notice would have named exec unavailable while it was on the wire. Emitted order is rebuilt from the sorted candidates so the wire order stays loaded -> exec -> gateway -> filler. The wp1 test asserting an omitted exec is never named now uses a structured exec, which stays ordinary filler and stays droppable, so both invariants keep coverage. Plan and audit: devlog/_plan/260827_kiro_subagent_delegation_unblock/040, /041 --- .../013_wp1_check_evidence.md | 92 ++++++++ .../040_exec_catalog_priority.md | 207 ++++++++++++++++++ .../041_wp2_audit_synthesis.md | 75 +++++++ src/adapters/kiro-tools.ts | 54 +++-- tests/kiro-adapter.test.ts | 90 +++++++- 5 files changed, 500 insertions(+), 18 deletions(-) create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/013_wp1_check_evidence.md create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/040_exec_catalog_priority.md create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/041_wp2_audit_synthesis.md diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/013_wp1_check_evidence.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/013_wp1_check_evidence.md new file mode 100644 index 00000000000..5d59fadcb42 --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/013_wp1_check_evidence.md @@ -0,0 +1,92 @@ +# wp1 — C-phase evidence + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp1` · C phase + +## Static gates + +| gate | result | +|---|---| +| `bun x tsc --noEmit` | exit 0 | +| `bun run test` | **15169 pass, 0 fail** across 951 files (119.30s), exit 0 | +| `bun test tests/kiro-adapter.test.ts` | 64 pass, 0 fail | +| `bun test tests/tool-catalog-nudge.test.ts` | 17 pass, 0 fail | + +The full suite is the gate here, not the focused run: `src/AGENTS.md:26` requires +it for adapter behavior. + +## Red-first transcript + +The 6 new tests were run BEFORE the implementation. Two failed and four passed, +which is the correct split — the four that passed are the must-NOT-fire controls, +and a test that only ever passes proves nothing. + +```text +(fail) names ALL_TOOLS when a freeform exec is advertised without a bare shell bridge +(pass) keeps the generic fallback for a STRUCTURED tool that merely shares the name exec +(pass) does not claim code mode when a bare shell bridge sits beside exec +(fail) decides on the EMITTED catalog: a budget-omitted shell bridge no longer suppresses code mode +(pass) does not name a code-mode exec that the catalog budget dropped +(pass) advertises no code mode for a tool_choice:none turn + 4 pass, 2 fail +``` + +The second failure is the reviewer's blocker-1 case reproduced as a test. Its +red output shows 48 of 49 tools emitted with `exec_command` omitted, `exec` +present, and the nudge still carrying the generic fallback: + +```text +Expected to contain: "ALL_TOOLS" +Received: "[opencodex] Kiro's outbound catalog budget allows 48 of 49 client tools +this turn. Omitted and unavailable this turn: exec_command. ... If a listed tool +exposes nested helpers such as a tools.* API, ..." +``` + +After the fix all 6 pass. + +## Activation grounding (C-ACTIVATION-GROUNDING-01) + +The new branch is conditional, so "suite green" is not evidence on its own. Each +path was driven and observed: + +| path | driven by | observed | +|---|---|---| +| fires | freeform `exec`, no bridge | nudge contains `ALL_TOOLS` + `Codex code mode`, generic fallback absent | +| must not fire — semantics | structured `exec` (no `freeform`) | generic fallback retained | +| must not fire — flat bridge | `exec` + `exec_command` both emitted | no `ALL_TOOLS` | +| fires despite omission | 49 tools, bridge dropped by budget | `ALL_TOOLS` present | +| must not fire — omitted exec | 48 fillers ahead of `exec` | `exec` absent from catalog, no `ALL_TOOLS` | +| must not fire — no catalog | `tool_choice: "none"` | no `ALL_TOOLS` | + +## Live proof (the criterion that closes the unit) + +Proxy restarted onto this branch (`ocx restart`, healthy PID 56020 port 10100, +kiro logged in). A fresh top-level `kiro/claude-opus-5` task was asked, in +Korean, whether it had any tool for delegating work to subagents. **The prompt +never mentioned `ALL_TOOLS`, `exec`, `spawn_agent`, or code mode.** + +Reply (verbatim excerpt): + +> 있습니다. 서브에이전트 파견용 도구는 multi_agent_v1__spawn_agent이고, 모델과 +> 추론 강도 모두 지정할 수 있습니다. 참고로 이 도구들은 최상위 도구 목록에는 안 +> 보이고, exec(코드 모드) 안에서 await tools.multi_agent_v1__spawn_agent({...}) +> 형태로 호출하는 중첩 헬퍼로 노출돼 있습니다. + +It then listed `model`, `reasoning_effort`, `service_tier`, and `fork_context` +with their inheritance semantics, plus the five advertised model overrides and +their effort ladders. + +Baseline from the same model before the fix: + +> No spawn/subagent tool exists in my current tool catalog. + +That is the whole unit in two quotes. The reported symptom — "모델 지정, 에포트, +속도 지정을 못하는거 같아" — resolves not because those parameters changed, but +because the model can now find the tool that carries them. + +## Residual + +The nudge is truncated from the TAIL under injection-budget pressure and the +code-mode sentence sits late in it. Measured headroom today is ~10 KB of 16384 +(`012`), so the risk is dormant. If `MAX_KIRO_TOOL_COUNT` or the nudge grows +materially, this sentence is the first thing cut — and the failure would be +silent, exactly like the one this unit just fixed. diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/040_exec_catalog_priority.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/040_exec_catalog_priority.md new file mode 100644 index 00000000000..730c012c6ec --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/040_exec_catalog_priority.md @@ -0,0 +1,207 @@ +# wp2 — protect the code-mode `exec` tool from Kiro catalog-budget eviction + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp2` + +## Why this is a separate work-phase + +wp1 restored the sentence that tells a Kiro model deferred helpers exist. That +sentence is worthless if the tool it points at was never sent. Under code mode +`exec` is not one tool among many — it is the ONLY door to shell, file edits, +`apply_patch`, and every MCP helper. Dropping it does not degrade the turn, it +ends it. + +Kiro currently ranks it as filler: + +```ts +// src/adapters/kiro-tools.ts:175-179 +function boundedCatalogPriority(tool: OcxTool): number { + if (tool.loadedFromToolSearch) return 0; + if (tool.toolSearch) return 1; + return 2; +} +``` + +Cursor solved the same problem for the same reason and says so in the code: +*"Execution path ... outranks filler so a crowded catalog cannot drop the Codex +shell bridge (#399)"* (`cursor/request-builder.ts:52-55`). + +## The trap, found by the research lane + +The obvious move — give `exec` priority 0, or a new -1 — **regresses #2475**. + +Priority 0 today is `loadedFromToolSearch`, and `09062014e fix(kiro): prioritize +tool search results within catalog budget (#2475)` exists precisely because Kiro +was dropping tools a `tool_search` had just loaded. Its regression test demands +the loaded tool first and the gateway second: + +```ts +// tests/kiro-adapter.test.ts:764-769 +expect(names.slice(0, 2)).toEqual([loadedTool.name, searchGateway.name]); +``` + +Sharing tier 0 makes declaration index the tie-break, so an `exec` declared +earlier silently displaces them. Tier -1 demotes them unconditionally. + +Worse, the admission loop stops at the FIRST candidate that does not fit and +discards the entire remaining suffix: + +```ts +// src/adapters/kiro-tools.ts:214-221 +for (const [index, entry] of candidates.entries()) { + if (convertedTools.length >= MAX_KIRO_TOOL_COUNT + || serializedToolCatalogBytes([...convertedTools, entry.converted]) > MAX_KIRO_TOOL_CATALOG_BYTES) { + omittedAt = index; + break; + } +``` + +So one byte-hungry tool promoted to the front can evict everything behind it. +Promoting `exec` to the very front is not a free safety improvement; it is a new +way to lose the tools #2475 protects. + +## Chosen ladder + +```text +0 loadedFromToolSearch (unchanged — #2475 keeps its guarantee) +1 semantic code-mode exec (NEW) +2 toolSearch gateway (was 1) +3 everything else (was 2) +``` + +`exec` outranks the gateway and the ordinary catalog while staying behind +already-loaded search results. The reasoning: a loaded tool is one the model +asked for by name this turn, so displacing it breaks an explicit request; `exec` +outranks the *gateway* because it reaches every nested helper on THIS turn while +the gateway needs a discovery round-trip first. + +*(Corrected after audit: an earlier draft said a gateway without `exec` "can +search but never act." That is overstated — +`tests/responses-tool-conformance.test.ts:212-216` shows a searched tool becomes +callable on a later turn. The honest argument is latency and directness, not +impossibility.)* + +Detection uses the shared semantic predicate `isCodexCodeModeExecTool` +(`tool-catalog-nudge.ts:31`, exported in wp1), not a name match. A structured +`exec` that runs a shell string is ordinary filler and must stay at tier 3. + +## A tier alone does NOT achieve this (audit blocker 1, High) + +The first draft of this document stopped at the tier change and argued that +ordering-only was a virtue: *"it cannot admit a tool that does not fit."* The +reviewer turned that sentence into a counterexample. + +`src/responses/parser.ts:661-666` pushes `tool_search_output` specs with an +unbounded `loadedToolSpecs.push(...specs)`, and `:726-748` marks every one +`loadedFromToolSearch: true` — tier 0. Accumulate 48 loaded tools across a +session and the fill loop reaches `convertedTools.length >= MAX_KIRO_TOOL_COUNT` +before it ever reaches tier 1, so `exec` is omitted anyway. + +That outcome is worse than it first looks. Those 48 loaded tools are reachable +ONLY as nested `tools.(...)` helpers inside `exec`. Dropping `exec` to keep +all 48 produces a catalog where every admitted tool is uncallable. Keeping 47 and +`exec` produces 47 callable tools. + +## Reservation, not eviction + +Cursor guarantees admission by evicting occupants (`evictNonExecutionPath`, +`request-builder.ts:126-137`). Kiro will not copy that: the routine exempts only +execution-path tools, so it can evict a `loadedFromToolSearch` tool — the #2475 +regression by another route. + +Reserve instead. Before filling, set aside one count slot and `exec`'s serialized +bytes; fill the remaining room by priority; then admit `exec`. + +The distinction is not cosmetic. Eviction removes a tool that was already +admitted, which is what makes it capable of undoing #2475. Reservation only +lowers the room the fill loop sees, so the loop admits one fewer tool and never +takes one back. #2475 asks that loaded results be prioritized *within* the +bounded catalog; it does not promise an unbounded number of them. + +## Diff plan, revised + +```ts +const execIndex = candidates.findIndex(entry => isCodexCodeModeExecTool(entry.tool)); +const execEntry = execIndex >= 0 ? candidates[execIndex] : undefined; +// fill loop skips execEntry, checks count against (MAX - 1) and bytes against a +// projection that always includes exec, then exec is admitted after the loop +``` + +`omittedTools` is then derived by set difference rather than `candidates.slice`, +because `exec` is skipped during the fill and admitted afterwards. Emitted order +is restored from the sorted candidate positions, so the wire order stays +loaded -> exec -> gateway -> filler. + +## Diff plan + +### MODIFY `src/adapters/kiro-tools.ts` + +```ts +import { isCodexCodeModeExecTool } from "./tool-catalog-nudge"; + +function boundedCatalogPriority(tool: OcxTool): number { + if (tool.loadedFromToolSearch) return 0; + if (isCodexCodeModeExecTool(tool)) return 1; + if (tool.toolSearch) return 2; + return 3; +} +``` + +Check the import direction first: `tool-catalog-nudge.ts` must not import from +`kiro-tools.ts`, or this creates a cycle. + +### MODIFY `tests/kiro-adapter.test.ts` + +## A wp1 test must be REPLACED, not preserved (audit blocker 2, Medium) + +wp1 landed this assertion four hours ago: + +```ts +// tests/kiro-adapter.test.ts:1301 "does not name a code-mode exec that the catalog budget dropped" +expect(emitted).not.toContain("exec"); +expect(current.content).not.toContain("ALL_TOOLS"); +``` + +Its fixture is 48 fillers followed by a freeform `exec` — exactly the shape wp2 +now intends to make survive. Implementing wp2 without touching it guarantees a +red suite. + +The two are not in conflict about behavior; they were written against different +worlds. wp1 asserted the CONSEQUENCE of exec being droppable: if the model cannot +call `exec`, do not advertise it as the execution surface. wp2 removes the +premise. The rule wp1 actually encodes — never name a tool the catalog omitted — +is still exactly right, and still needs a test. + +So the fixture is re-pointed rather than deleted. wp1's rule keeps its own test +using a **structured** `exec`, which stays tier 3 and stays droppable; the +freeform fixture becomes wp2's survival test. Both invariants keep a home. + +## Accept criteria + +1. An over-budget catalog declaring a freeform `exec` LAST still emits `exec`, + and the wp1 nudge then names `ALL_TOOLS`. Red-first: fails today. +2. **48 loaded tools plus a last-declared `exec`**: `exec` is emitted, exactly + one loaded tool is omitted, and the omission notice names it. This is the + blocker-1 reservation case; a tier-only implementation fails it. +3. An **over-budget** fixture declared gateway -> exec -> loaded emits + loaded -> exec -> gateway with none of the three omitted. + *(Amended per blocker 3: the fixture MUST exceed a budget. The sort at + `kiro-tools.ts:207-211` only runs when `exceedsBudget` is true, so a + three-tool fixture would pass identically before and after the change and + prove nothing.)* +4. A **structured** `exec` declared last in an over-budget catalog is still + dropped, and is still not named — the must-NOT-fire control, and wp1's + invariant preserved on its own fixture. +5. `tests/kiro-adapter.test.ts:728` and `:789` (declared-prefix order) stay + green: those fixtures hold only ordinary tools, all tier 3, so the + `|| a.index - b.index` tie-break keeps them byte-identical. +6. `tests/kiro-adapter.test.ts:736-770` (#2475) stays green untouched: its + fixture contains no `exec`, and loaded/gateway/ordinary land in distinct + tiers 0/2/3 with no tie to break. + +## Verification + +```bash +bun x tsc --noEmit +bun test tests/kiro-adapter.test.ts +bun run test +``` diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/041_wp2_audit_synthesis.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/041_wp2_audit_synthesis.md new file mode 100644 index 00000000000..84190a76ada --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/041_wp2_audit_synthesis.md @@ -0,0 +1,75 @@ +# wp2 — audit round 1 synthesis (REVIEW-SYNTHESIS-01) + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp2` · A phase + +Reviewer verdict: **FAIL** — 1 High, 2 Medium. All three accepted; none rebutted. + +## Root cause of my planning error + +I wrote "a tier change is ordering only" as a safety argument. The reviewer read +the same sentence as a defect report, and was right: ordering-only means the +change cannot deliver the protection the work-phase exists to provide. + +The counterexample is constructible from code, not hypothetical. +`src/responses/parser.ts:661-666` accumulates `tool_search_output` specs with an +unbounded push, all marked tier 0. At 48 loaded tools the fill loop exhausts +`MAX_KIRO_TOOL_COUNT` before reaching tier 1 and `exec` is dropped — the precise +failure wp2 claims to prevent. + +What makes it worse than an ordinary omission: those 48 loaded tools are +reachable only as nested `tools.(...)` helpers inside `exec`. Dropping +`exec` to keep all 48 yields a catalog in which every admitted tool is +uncallable. That inverts the budget's purpose — it optimizes the count while +zeroing the capability. + +## Reservation vs eviction + +I rejected Cursor's `evictNonExecutionPath` for a real reason: it exempts only +execution-path tools, so it can evict a `loadedFromToolSearch` tool and recreate +#2475. That reasoning survives. What did not survive is concluding that +therefore nothing should be done. + +Reservation is the third option I missed. Eviction removes an already-admitted +tool; reservation lowers the room the fill loop sees. Only the former can undo +#2475, because only the former takes something back. + +## The test collision (blocker 2) is the more interesting finding + +wp2's accept criterion contradicted a test wp1 landed hours earlier: +`tests/kiro-adapter.test.ts:1301` asserts that 48 fillers plus a freeform `exec` +emits no `exec` and names no `ALL_TOOLS`. + +Both are correct about different worlds. wp1 encoded a CONSEQUENCE of the +then-current behavior — if the model cannot call `exec`, do not advertise it — +while wp2 removes the premise that `exec` gets dropped at all. The underlying +rule (never name an omitted tool) is untouched and still needs coverage. + +Resolution: re-point the fixture instead of deleting the test. The freeform case +becomes wp2's survival proof; wp1's rule keeps its own test on a **structured** +`exec`, which remains tier 3 and remains droppable. Neither invariant loses its +home, and the diff records that the change was deliberate. + +This is the concrete cost of one-work-phase-one-cycle discipline being worth it: +wp1's test is what made wp2's plan falsifiable within one audit round. + +## Blocker 3: a test that proves nothing + +My criterion 2 fixture (loaded + exec + gateway, three tools) never exceeds a +budget, and `kiro-tools.ts:207-211` only sorts when `exceedsBudget` is true. It +would have passed identically before and after the implementation. + +Amended to an over-budget fixture declared adversarially (gateway -> exec -> +loaded), so the assertion can only pass if the comparator actually ran. + +## Corrections the reviewer made to my rationale + +I claimed a `tool_search` gateway without `exec` "can search but never act." +Overstated: `tests/responses-tool-conformance.test.ts:212-216` shows a searched +tool becomes callable on a later turn. The honest ranking argument is latency and +directness — `exec` reaches every nested helper this turn, the gateway needs a +discovery round-trip first — not impossibility. + +Confirmed clean by the same review: no import cycle (`tool-catalog-nudge.ts` +imports only `../types`), the omission notice makes no ordering claim, and the +alias registry is populated in declaration order at `kiro-tools.ts:187-205` +before any sorting, so reordering cannot disturb `nameMap`. diff --git a/src/adapters/kiro-tools.ts b/src/adapters/kiro-tools.ts index 48f31ece344..e4e2c11a044 100644 --- a/src/adapters/kiro-tools.ts +++ b/src/adapters/kiro-tools.ts @@ -1,6 +1,7 @@ import type { OcxParsedRequest, OcxTool } from "../types"; import { namespacedToolName } from "../types"; import { normalizeKiroModelId } from "../providers/kiro-models"; +import { isCodexCodeModeExecTool } from "./tool-catalog-nudge"; import { createKiroToolNameRegistry, type KiroToolNameRegistry } from "./kiro-wire"; const MAX_KIRO_TOOL_DESCRIPTION_UNVERIFIED = 1024; @@ -174,8 +175,14 @@ function omittedToolCatalogNotice(kept: number, omitted: readonly OcxTool[], reg function boundedCatalogPriority(tool: OcxTool): number { if (tool.loadedFromToolSearch) return 0; - if (tool.toolSearch) return 1; - return 2; + // Codex code mode reaches shell, file edits, apply_patch and every MCP helper ONLY as nested + // `tools.(...)` calls inside this one tool. Dropping it does not shrink the catalog, it + // makes the rest of the catalog uncallable -- so it outranks the search gateway and filler. + // It stays BEHIND `loadedFromToolSearch` because those are tools the model asked for by name + // this turn (#2475). Cursor pins its execution path the same way (request-builder.ts, #399). + if (isCodexCodeModeExecTool(tool)) return 1; + if (tool.toolSearch) return 2; + return 3; } export function convertKiroToolContext( @@ -209,19 +216,40 @@ export function convertKiroToolContext( const candidates = exceedsBudget ? convertedEntries.toSorted((a, b) => boundedCatalogPriority(a.tool) - boundedCatalogPriority(b.tool) || a.index - b.index) : convertedEntries; - const convertedTools: unknown[] = []; - let omittedAt = candidates.length; - for (const [index, entry] of candidates.entries()) { + // Reserve a seat for the code-mode execution path before filling the rest. + // + // Priority alone cannot save it. `loadedFromToolSearch` tools outrank it and arrive unbounded + // (the Responses parser pushes every `tool_search_output` spec), so a session that accumulated + // MAX_KIRO_TOOL_COUNT loaded tools would exhaust the budget before reaching tier 1 and drop the + // one tool through which all of them are actually callable. + // + // Reservation rather than eviction: this lowers the room the fill loop sees, so it admits one + // fewer tool. It never removes a tool that already fit, which is what keeps #2475's loaded-result + // guarantee intact -- Cursor's `evictNonExecutionPath` exempts only the execution path and could + // evict a loaded tool instead. + const reserved = candidates.find(entry => isCodexCodeModeExecTool(entry.tool)); + const admitted = new Set(); + const filled: unknown[] = []; + for (const entry of candidates) { + if (entry === reserved) continue; + // Measure the projected FINAL array: the byte budget is computed over the serialized array, so + // subtracting a standalone size would misjudge it by the separators JSON adds between entries. + const projected = reserved ? [...filled, entry.converted, reserved.converted] : [...filled, entry.converted]; if ( - convertedTools.length >= MAX_KIRO_TOOL_COUNT - || serializedToolCatalogBytes([...convertedTools, entry.converted]) > MAX_KIRO_TOOL_CATALOG_BYTES - ) { - omittedAt = index; - break; - } - convertedTools.push(entry.converted); + projected.length > MAX_KIRO_TOOL_COUNT + || serializedToolCatalogBytes(projected) > MAX_KIRO_TOOL_CATALOG_BYTES + ) break; + filled.push(entry.converted); + admitted.add(entry.index); } - const omittedTools = candidates.slice(omittedAt).map(entry => entry.tool); + if (reserved) admitted.add(reserved.index); + // Rebuild in sorted-candidate order so the wire order stays loaded -> exec -> gateway -> filler. + // Pushing the reserved entry after the loop would place it last instead. + const convertedTools = candidates.filter(entry => admitted.has(entry.index)).map(entry => entry.converted); + // Derive omissions by set difference. The old `candidates.slice(omittedAt)` assumed every + // candidate after the first rejection was omitted, which stops being true once one of them was + // reserved and admitted: the notice would name `exec` unavailable while it is on the wire. + const omittedTools = candidates.filter(entry => !admitted.has(entry.index)).map(entry => entry.tool); return { tools: convertedTools, systemAdditions: omittedTools.length > 0 ? [omittedToolCatalogNotice(convertedTools.length, omittedTools, registry)] : [], diff --git a/tests/kiro-adapter.test.ts b/tests/kiro-adapter.test.ts index 571208a8b81..959a59ff572 100644 --- a/tests/kiro-adapter.test.ts +++ b/tests/kiro-adapter.test.ts @@ -1298,17 +1298,17 @@ describe("kiro code-mode catalog nudge", () => { expect(current.content).toContain("ALL_TOOLS"); }); - test("does not name a code-mode exec that the catalog budget dropped", async () => { - // The mirror of the case above, and the reason the fix intersects rather than unions: a tool - // the model cannot call must never be named as its execution surface. Equal-priority tools - // keep request order, so 48 fillers ahead of `exec` omit `exec` deterministically. + test("does not name a STRUCTURED exec that the catalog budget dropped", async () => { + // A tool the model cannot call must never be named as its execution surface. wp2 makes a + // code-mode `exec` survive the budget, so this invariant is now demonstrated on a structured + // `exec`: it stays ordinary filler, so 48 fillers ahead of it drop it deterministically. const filler = Array.from({ length: MAX_KIRO_TOOL_COUNT }, (_, index) => ({ name: `filler_${String(index).padStart(3, "0")}`, description: `Filler ${index}`, parameters: { type: "object" }, })); const { body } = await createKiroAdapter(provider).buildRequest( - parsedWith([{ role: "user", content: "hi" }], [...filler, codeModeExec]), + parsedWith([{ role: "user", content: "hi" }], [...filler, { name: "exec", description: "Run a shell command", parameters: { type: "object" } }]), ); const current = JSON.parse(body).conversationState.currentMessage.userInputMessage; const emitted = current.userInputMessageContext.tools.map((tool: { toolSpecification: { name: string } }) => tool.toolSpecification.name); @@ -1330,3 +1330,83 @@ describe("kiro code-mode catalog nudge", () => { expect(content).not.toContain("ALL_TOOLS"); }); }); + +describe("kiro code-mode exec survives the catalog budget", () => { + // Under code mode `exec` is not one tool among many: shell, file edits, apply_patch and every + // MCP helper are reachable ONLY as nested `tools.(...)` calls inside it. A catalog that + // drops `exec` to keep more helpers admits tools the model cannot call at all. Cursor pins its + // execution path for the same reason (request-builder.ts, #399); Kiro ranked it as filler. + const codeModeExec = { name: "exec", description: "Run JavaScript", freeform: true, parameters: { type: "object" } }; + const fillerTools = (count: number) => Array.from({ length: count }, (_, index) => ({ + name: `ordinary_${String(index).padStart(3, "0")}`, + description: `Ordinary ${index}`, + parameters: { type: "object" }, + })); + + async function emitted(tools: unknown[]): Promise<{ names: string[]; notice: string }> { + const { body } = await createKiroAdapter(provider).buildRequest(parsedWith([{ role: "user", content: "hi" }], tools)); + const current = JSON.parse(body).conversationState.currentMessage.userInputMessage; + return { + // Drop Kiro's private completion tool, which is appended after the client catalog. + names: current.userInputMessageContext.tools.slice(0, -1).map((tool: { toolSpecification: { name: string } }) => tool.toolSpecification.name), + notice: current.content.split("\n\n", 1)[0] as string, + }; + } + + test("a last-declared code-mode exec survives an over-budget catalog", async () => { + const { names } = await emitted([...fillerTools(MAX_KIRO_TOOL_COUNT + 20), codeModeExec]); + + expect(names).toContain("exec"); + expect(names).toHaveLength(MAX_KIRO_TOOL_COUNT); + }); + + test("reserving exec costs exactly one loaded tool, not the execution path", async () => { + // The reservation tradeoff, stated as a test. A session can accumulate an unbounded number of + // tool_search results (responses/parser.ts pushes every spec), all of which outrank exec. If + // the fill loop ran unreserved, 48 loaded tools would exhaust the count budget and drop the + // one tool that makes the other 48 callable. + const loaded = Array.from({ length: MAX_KIRO_TOOL_COUNT }, (_, index) => ({ + name: `loaded_${String(index).padStart(3, "0")}`, + description: `Loaded ${index}`, + parameters: { type: "object" }, + loadedFromToolSearch: true, + })); + const { names, notice } = await emitted([...loaded, codeModeExec]); + + expect(names).toContain("exec"); + expect(names).toHaveLength(MAX_KIRO_TOOL_COUNT); + // Exactly one loaded tool pays for the reservation, and the notice names it honestly. + expect(names.filter(name => name.startsWith("loaded_"))).toHaveLength(MAX_KIRO_TOOL_COUNT - 1); + expect(notice).toContain("loaded_047"); + expect(notice).not.toContain("`exec`"); + }); + + test("emits loaded -> exec -> gateway order in an over-budget catalog", async () => { + // Declared adversarially (gateway first, loaded last) so the assertion can only pass if the + // priority comparator actually ran. The sort is gated on exceedsBudget, so the fixture must + // exceed the count budget or this would pass identically without the change. + const gateway = { name: "tool_search", description: "Search deferred tools", parameters: { type: "object" }, toolSearch: true }; + const loaded = { name: "codex_app__send_message_to_thread", description: "Send", parameters: { type: "object" }, loadedFromToolSearch: true }; + const { names, notice } = await emitted([gateway, ...fillerTools(MAX_KIRO_TOOL_COUNT + 20), codeModeExec, loaded]); + + expect(names.slice(0, 3)).toEqual([loaded.name, "exec", gateway.name]); + expect(notice).not.toContain(loaded.name); + expect(notice).not.toContain(gateway.name); + }); + + test("a byte-budget catalog still reserves room for exec", async () => { + // The count budget is the easy case. Bytes are where a naive reservation breaks: the budget is + // measured over the serialized ARRAY, so exec must be projected into every fit check rather + // than subtracted as a standalone size. + const heavy = Array.from({ length: 40 }, (_, index) => ({ + name: `heavy_${String(index).padStart(3, "0")}`, + description: `Heavy ${index}`, + parameters: { type: "object", properties: { blob: { type: "string", description: "x".repeat(8_000) } } }, + })); + const { names } = await emitted([...heavy, codeModeExec]); + const serialized = new TextEncoder().encode(JSON.stringify(names)).byteLength; + + expect(names).toContain("exec"); + expect(serialized).toBeGreaterThan(0); + }); +}); From a4d9fb879bd7f58b8212805c4e87844f5cf2f7ca Mon Sep 17 00:00:00 2001 From: jun Date: Thu, 27 Aug 2026 19:08:46 +0900 Subject: [PATCH 3/5] docs(devlog): record wp2 check evidence and the live parallel-delegation proof --- .../042_wp2_check_evidence.md | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/042_wp2_check_evidence.md diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/042_wp2_check_evidence.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/042_wp2_check_evidence.md new file mode 100644 index 00000000000..2a648dc9433 --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/042_wp2_check_evidence.md @@ -0,0 +1,113 @@ +# wp2 — C-phase evidence + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp2` · C phase + +## Static gates + +| gate | result | +|---|---| +| `bun x tsc --noEmit` | exit 0 | +| `bun run test` | **exit 0**, `Ran 15185 tests across 951 files` (124.51s) | +| `bun test tests/kiro-adapter.test.ts` | 68 pass, 0 fail | + +Test count moved 15181 -> 15185: the four new wp2 cases. No test was deleted; +the wp1 omission test was re-pointed to a structured `exec` fixture. + +## Red-first transcript + +All four new tests failed before the implementation — `0 pass, 4 fail`. The byte +budget case is the clearest, emitting eleven heavy tools and no execution path: + +```text +expect(names).toContain("exec") +Expected to contain: "exec" +Received: [ "heavy_000", "heavy_001", ... "heavy_010" ] +(fail) a byte-budget catalog still reserves room for exec +``` + +After the change all four pass and the file totals 68/0. + +## What each test pins + +| test | invariant | +|---|---| +| last-declared exec survives an over-budget catalog | the basic wp2 claim | +| reserving exec costs exactly one loaded tool | the reservation tradeoff is bounded at ONE, and the notice names the displaced tool | +| emits loaded -> exec -> gateway order | the comparator actually ran (fixture is over-budget and declared adversarially: gateway first, loaded last) | +| a byte-budget catalog still reserves room | exec is PROJECTED into the byte check, not subtracted | +| (re-pointed) structured exec is dropped and not named | wp1's invariant, on a tier-3 fixture | + +## Reviewer-flagged traps, and what the code does + +Three defects were predicted at audit before any code existed. Each is closed by +construction rather than by a passing assertion alone: + +1. **Byte subtraction would be wrong.** `serializedToolCatalogBytes` measures + `JSON.stringify` of the whole array, so a standalone size misjudges the + separators. The loop measures `[...filled, entry.converted, reserved.converted]`. +2. **`candidates.slice(omittedAt)` would report a ghost omission.** With a + reserved entry admitted after the break point, the suffix contains a tool that + IS on the wire — the notice would have said `exec` was unavailable and counted + 50 of 49. Omissions are now a set difference. +3. **A post-loop push would land `exec` last.** Emitted order is rebuilt by + filtering the sorted `candidates`, preserving loaded -> exec -> gateway -> filler. + +## Interaction with #2475 + +`tests/kiro-adapter.test.ts:736-770` passes untouched. Reservation can displace +one loaded tool, and the new test asserts exactly that bound: 48 loaded tools +plus an `exec` emits 47 loaded tools, `exec`, and an omission notice naming +`loaded_047`. + +That tradeoff is the honest reading of #2475, which prioritizes loaded results +*within* a bounded catalog rather than promising an unbounded number of them. The +alternative — keeping all 48 and dropping `exec` — optimizes the count while +making every one of those 48 tools uncallable. + +## Live end-to-end proof (wp1 + wp2 together) + +Proxy restarted onto both commits (healthy, PID 74847, port 10100). A fresh +top-level `kiro/claude-opus-5` task was asked to delegate two subtasks in +parallel, each spawned with `model: gpt-5.6-luna` and `reasoning_effort: low`. + +It did the whole thing. Verbatim excerpts: + +> 파견에 사용한 도구 이름: multi_agent_v1__spawn_agent ... 다만 이 도구는 상위 +> 툴 카탈로그에 노출되지 않고 exec 코드 모드 안에서 +> `await tools.multi_agent_v1__spawn_agent({...})` 형태로만 호출됩니다. 그래서 +> 처음에 ALL_TOOLS를 뒤져 스키마를 확인한 뒤 파견했습니다. + +That sentence is the unit's thesis, stated back by the model that could not do +it this morning: it found `ALL_TOOLS`, read the schema, and spawned. + +Returned agent ids `01a042b0-1011-...` (17) and `01a042b0-10a6-...` (21), with +correct answers from both. The whole loop — spawn, wait, collect, close — ran +inside `exec`. + +### The model out-verified my own acceptance criterion + +Asked whether the overrides took effect, the children self-reported +`claude-opus-5, effort unspecified`. The parent refused that answer, went to the +rollout files, and found: + +```text +"model":"kiro/claude-opus-5" <- the PARENT's model, in the identity prompt +"model":"gpt-5.6-luna" <- the child's actual turn context +"effort":"low" +``` + +Its conclusion, which is now recorded doctrine for this unit: + +> 서브에이전트에게 "네 모델이 뭐냐"고 물어서 오버라이드를 검증하려는 접근은 +> 이번처럼 항상 틀린 답을 줍니다. 검증이 필요하면 +> `~/.codex/sessions/`의 해당 롤아웃 `.jsonl`을 보는 쪽이 확실합니다. + +This matters beyond a nice anecdote. The goal's stated acceptance evidence was +"the child reports the requested model and effort" — which, as measured here, +is an unreliable oracle: a child reads its identity prompt, not its runtime +config. The criterion is hereby superseded by the rollout-file check. + +A third agent appeared because a `wait_agent` call used `target` instead of +`targets`; the parent misread the resulting error as a spawn failure and retried. +The spawns had succeeded. All three were closed. Noted for honesty — it is a +harness-schema stumble, not a defect in this unit. From d44c406aa45d32d5c4602adcbdd2fae2dc7af089 Mon Sep 17 00:00:00 2001 From: jun Date: Thu, 27 Aug 2026 19:17:36 +0900 Subject: [PATCH 4/5] test(kiro): assert the byte budget holds when exec is reserved The byte-budget regression asserted only that the serialized name list was non-empty, which passes even if exec were admitted on top of an already-full catalog -- the exact failure mode the projected-array measurement exists to prevent. Assert the real invariant instead: exec is present, the catalog is genuinely truncated, and the emitted specs stay within MAX_KIRO_TOOL_CATALOG_BYTES. Found by the pre-merge review lane. --- .../050_ship.md | 58 +++++++++++++++++++ tests/kiro-adapter.test.ts | 17 ++++-- 2 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/050_ship.md diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/050_ship.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/050_ship.md new file mode 100644 index 00000000000..1b0457f8d53 --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/050_ship.md @@ -0,0 +1,58 @@ +# wp3 — ship: branch, gates, PR against `dev` + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp3` + +## What is being shipped + +Branch `codex/kiro-subagent-delegation-unblock`, cut from `origin/dev` +`a57b9620`, three commits: + +| commit | content | +|---|---| +| `0cfbb394a` | wp1 — enable the code-mode catalog nudge for Kiro | +| `e1019d30d` | wp2 — reserve catalog room for the code-mode execution path | +| `605c07bd5` | wp2 evidence devlog | + +## Pre-flight state + +| check | result | +|---|---| +| `bun x tsc --noEmit` | exit 0 | +| `bun run test` | exit 0, 15185 tests / 951 files | +| `bun run privacy:scan` | `Privacy scan passed` | +| working tree | clean | +| `gh auth status` | logged in as `lidge-jun` | + +`privacy:scan` is run because `src/AGENTS.md:27` requires it for changes touching +requests or logging, and this unit changes what is injected into an upstream +request body. + +## PR requirements this repo enforces + +From `AGENTS.md` and `.github/PULL_REQUEST_TEMPLATE.md`: + +- Target `dev`. `main` is release-only, and the `enforce-target` check rejects + wrong-base PRs. +- Fill every template section: Summary, Verification, Checklist. `enforce-target` + also rejects empty, thin, or malformed descriptions. +- No GUI screenshot needed: nothing under `gui/` is touched. +- No `Closes #N`: this unit has no filed issue. + +## Push authorization + +`DEV-GIT-PUSH-01` makes pushing an ESCALATE action requiring explicit user +approval. Granted in the originating request: *"구현완료하고 pr 올려놔 별도 브랜치 +잡고 출발"* — that names the branch and the PR, so the approval covers pushing +this branch and opening this PR. It does not extend to promoting to `main`, +merging, or any other remote mutation. + +## Sequence + +1. `git push -u origin codex/kiro-subagent-delegation-unblock` +2. `gh pr create --base dev` with the filled template +3. Verify the PR renders, targets `dev`, and reports its checks + +## Acceptance + +PR exists, targets `dev`, description carries all three template sections with +real verification output, and the URL is reported to the user. diff --git a/tests/kiro-adapter.test.ts b/tests/kiro-adapter.test.ts index 959a59ff572..c08bd7e7173 100644 --- a/tests/kiro-adapter.test.ts +++ b/tests/kiro-adapter.test.ts @@ -1394,19 +1394,26 @@ describe("kiro code-mode exec survives the catalog budget", () => { expect(notice).not.toContain(gateway.name); }); - test("a byte-budget catalog still reserves room for exec", async () => { + test("a byte-budget catalog reserves room for exec without breaching the budget", async () => { // The count budget is the easy case. Bytes are where a naive reservation breaks: the budget is // measured over the serialized ARRAY, so exec must be projected into every fit check rather - // than subtracted as a standalone size. + // than subtracted as a standalone size. Admitting exec on top of an already-full catalog would + // satisfy "exec survives" while blowing the ceiling, so both halves are asserted here. const heavy = Array.from({ length: 40 }, (_, index) => ({ name: `heavy_${String(index).padStart(3, "0")}`, description: `Heavy ${index}`, parameters: { type: "object", properties: { blob: { type: "string", description: "x".repeat(8_000) } } }, })); - const { names } = await emitted([...heavy, codeModeExec]); - const serialized = new TextEncoder().encode(JSON.stringify(names)).byteLength; + const { body } = await createKiroAdapter(provider).buildRequest( + parsedWith([{ role: "user", content: "hi" }], [...heavy, codeModeExec]), + ); + const current = JSON.parse(body).conversationState.currentMessage.userInputMessage; + const specs = current.userInputMessageContext.tools.slice(0, -1); + const names = specs.map((tool: { toolSpecification: { name: string } }) => tool.toolSpecification.name); + const serializedBytes = new TextEncoder().encode(JSON.stringify(specs)).byteLength; expect(names).toContain("exec"); - expect(serialized).toBeGreaterThan(0); + expect(names.length).toBeLessThan(heavy.length + 1); + expect(serializedBytes).toBeLessThanOrEqual(MAX_KIRO_TOOL_CATALOG_BYTES); }); }); From 9421938523e9d91e07447af72e7c8cb09805bf2b Mon Sep 17 00:00:00 2001 From: jun Date: Thu, 27 Aug 2026 19:32:40 +0900 Subject: [PATCH 5/5] docs(devlog): record the remote shard 4/4 triage CI reports test 4/4 red on this PR. Verified on ssh lidge-ai that dev fails the same shard: this branch dies in batch 5/20 with 68 failures, dev b5563d438 dies in batch 10/20 with 47 in a different suite, and every failing file passes standalone on both checkouts. It is a pre-existing batch-isolation defect in the sharding harness, not a regression from this change. --- .../051_ci_shard4_triage.md | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 devlog/_plan/260827_kiro_subagent_delegation_unblock/051_ci_shard4_triage.md diff --git a/devlog/_plan/260827_kiro_subagent_delegation_unblock/051_ci_shard4_triage.md b/devlog/_plan/260827_kiro_subagent_delegation_unblock/051_ci_shard4_triage.md new file mode 100644 index 00000000000..521727759a5 --- /dev/null +++ b/devlog/_plan/260827_kiro_subagent_delegation_unblock/051_ci_shard4_triage.md @@ -0,0 +1,57 @@ +# wp3 — CI `test 4/4` triage (remote-only verification) + +Unit: `260827_kiro_subagent_delegation_unblock` · work-phase `wp3` · C phase + +All verification in this document was run on `ssh lidge-ai` at the user's +instruction. No local test run backs any claim here. + +## Finding + +**`test 4/4` fails on `dev` too. It is not caused by this branch.** + +| checkout | commit | shard 4/4 | same file standalone | +|---|---|---|---| +| this branch | `ad1627d38` | **FAIL** — 148 pass / 68 fail, batch 5/20 | **68 pass / 0 fail** | +| `dev` | `b5563d438` | **FAIL** — 145 pass / 47 fail, batch 10/20 | **68 pass / 0 fail** | + +Run on `/home/lidgeai/ocx-ci/opencodex` with the CI's own sharder, +`bash scripts/ci/run-bun-test-batches.sh 4/4`. + +## Why this is a pre-existing batch-isolation defect + +Three independent signals, none of which depend on trusting the branch: + +1. **`dev` fails the same shard.** The baseline is red before any commit from + this unit exists. +2. **Different batches, different counts.** The branch dies in batch 5/20 with 68 + failures; `dev` dies in batch 10/20 with 47, in a different suite (`CL-07 task + effectiveness producer`). A defect introduced by this branch would not move + the failure to a different batch and a different test family on `dev`. +3. **Every failing file passes alone.** `tests/codex-reset-credit-recovery.test.ts` + is 68 pass / 0 fail standalone on BOTH checkouts. The failures only appear + when the file shares a batch process. + +One failure names the mechanism directly: *"refuses process-state reset without +the repository test preload"*. These are process-state-sensitive suites that +need a fresh process or a preload; a batch that co-locates them loses it. That is +the batching harness, not the Kiro adapter. + +## This branch's own surface, verified remotely + +```text +lidge-ai:/home/lidgeai/ocx-ci/opencodex (ad1627d38) +bun test tests/kiro-adapter.test.ts + 68 pass + 0 fail + 331 expect() calls +``` + +## Disposition + +Not fixed here, and not claimed green either. Repairing shard batching is a +repository-wide harness concern that predates this unit; folding it in would +silently widen a Kiro adapter PR into CI infrastructure work, and would make the +diff impossible to review as one change. + +What this unit owes is honesty about it: the PR reports `test 4/4` red, states +that `dev` reproduces it, and gives the commands to re-run both sides.