diff --git a/devlog/_fin/260914_triage_round_lanes/000_plan.md b/devlog/_fin/260914_triage_round_lanes/000_plan.md new file mode 100644 index 0000000000..60b12d4c83 --- /dev/null +++ b/devlog/_fin/260914_triage_round_lanes/000_plan.md @@ -0,0 +1,126 @@ +# 260914 — Triage round lanes: two merge rounds, four worktree lanes + +Status: OPEN. Opened 2026-09-14 KST. Owner: main orchestration session +`01a09bc0-bb13-7393-8526-785a3aeacb91`, goalplan slug +`opencodex-2026-09-14-triage-delivery-loop-hotl-o`. + +## Objective + +Land 9-11 pull requests into `dev` across two merge rounds, then close the issues +and pull requests those merges resolve or supersede. The input is the 2026-09-14 +triage of 64 open PRs and 60 open issues; this unit is the delivery half of it. + +## Why this shape + +The triage queue mixes two kinds of work that cannot be orchestrated the same way. +PRs #4528, #4529, #4511 and #4512 belong to outside contributors: the work there is +verification and merge, and nothing can be stacked on a fork branch. The remaining +items — the #4515 follow-up, #4519, #4530 and #4516 — are ours to author, and only +those can carry a branch chain. Treating the whole queue as one stack is the failure +mode this unit exists to avoid. + +Ordering inside a chain follows gate speed, not importance. #4519 is the highest +priority item in the triage and it sits at the TOP of its lane, because it needs an +independent security review that can stall for days. A stalled bottom layer freezes +everything above it; a stalled top layer costs nothing once its parent has landed. + +## Verified repository facts (2026-09-14) + +These were read live and decide the merge mechanics. Refresh before acting on them. + +- `dev` ruleset carries `deletion`, `non_fast_forward` and `pull_request` + (1 approval, code-owner review required). There is **no** required-status-check + rule and **no** strict up-to-date rule, so a `dev` move does not mechanically + invalidate another PR's checks. Batch merging disjoint PRs inside one CI + generation is therefore sound, and the residual risk is semantic, not textual. +- Cross-platform CI runs in 9-11 minutes (runs observed 2026-09-13T16:54-17:03 and + 17:12-17:23). +- Contributor fork PRs sit at `action_required`: their workflow runs have never + executed. #4528 and #4529 both show this. Approving those runs is the first + merge-track action, not the last. +- `src/web-search/passthrough-bridge.ts` holds both `resolveOllamaWebSearchEndpoint` + (119-243) and `sidecarSettingsForBridge` (689). The two lane-A layers collide in + one file, which is what makes lane A a real chain rather than two parallel PRs. + +## Lane map + +Four `environment: worktree` threads. Write-sets are disjoint by construction; that +disjointness is what licenses the batch merge above. + +| Lane | Orchestrator | Bottom layer | Top layer | Write set | +|---|---|---|---|---| +| L1 | `anthropic/claude-opus-5` high | sidecar backend/model agreement | #4519 endpoint destination policy | `src/web-search/`, `src/server/responses/core.ts` (~6290) | +| L2 | `kimi/k3[1m]` high | catalog parser preserves `supportsImages` | capability propagation + override precedence | `src/adapters/devin/`, `src/codex/catalog/provider-fetch.ts` | +| L3 | `anthropic/claude-opus-5` high | #4516 spare-budget argument restoration | budget/ordering regressions | `src/adapters/cursor/` | +| L4 | `kimi/k3[1m]` high | #4529 carry, version-skew refusal plus its unknown-version case | #4512 live-failure regressions | `src/cli/index.ts`, `src/cli/system-restart-client.ts`, `tests/cli/`, `tests/server/audio-dictation.test.ts` | + +Lane A's top layer is security-gated. If it stalls past round 2, cut it loose from +the chain and re-base it directly on `dev` rather than holding the lane open. + +L4 is not a tests-only lane. The #4529 carry writes `src/cli/index.ts` and +`src/cli/system-restart-client.ts`, so it carries the same structure obligation as +any other source change. Structure owners per lane: L1 `structure/runtime.md`; +L2 `structure/catalog.md` and `structure/adapters/registry.md`; L3 +`structure/runtime.md` and `structure/providers/cursor.md`; L4 the owner of +`src/cli/`. Each lane resolves its owner from `structure/INDEX.md` at its own P and +updates the doc in the same PR, because `bun run structure:check` fails on a doc +that no longer matches the tree. + +## Worker policy + +Two surfaces, each doing what it is for. A **lane** needs its own branch, its own +CI and its own merge, so every lane is a `create_thread` task with +`environment: worktree`. A **worker inside a lane** is a bounded slice of that +lane's own tree, so it is a subagent, spawned by the lane thread into the lane's +worktree. Subagents belonging to different lanes cannot collide because the +worktrees differ. + +Four lane threads: `anthropic/claude-opus-5` on L1 and L3, `kimi/k3[1m]` at high +effort on L2 and L4. Each lane loads `$codexclaw:cxc-loop` and runs its own scoped +PABCD cycle with its own goal and FSM, because a thread owns both. + +Inside a lane, workers are `devin/swe-2` and `xai/grok-4.6` subagents at roughly +2:3, with no cap on count. Both ids are accepted by `spawn_agent` even though +`devin/swe-2` is absent from the advertised override list in the tool description; +that omission was verified as a documentation gap on 2026-09-14, not a real +restriction. Only one subagent writes at a time, and no subagent runs a +branch-level git operation. + +## Proof policy + +No local product suite, typecheck, build or install. Every PR body labels them +NOT RUN. The only proof is hosted Cross-platform CI at the exact head SHA, +dispatched explicitly with `gh workflow run ci.yml --ref -F lane=all`, +because a sync or rebase does not reliably queue it. Non-tip chain commits carry +`[skip ci]`. Pushes are `--no-verify` and fast-forward only; no shared branch is +force-pushed. Each round closes with one post-merge `dev` run as the joint proof +for that batch. + +## Work-phase map + +| Phase | Outcome | Depends on | +|---|---|---| +| wp1 | This roadmap, at diff level, before any lane dispatch | — | +| wp2 | Round 1: lanes created, bottom-layer PRs, merge-track approvals, batch merge, post-merge dev CI | wp1 | +| wp3 | Round 2: top-layer PRs, retarget after parents land, batch merge, post-merge dev CI | wp2 | +| wp4 | Closure sweep: resolved issues closed with merge references, superseded PRs and issues closed with written reasons | wp3 | + +Phase documents: `010_wp2_round1.md`, `020_wp3_round2.md`, `030_wp4_closure.md`. + +## Out of scope + +#4022, #4259, #2562, #3283, #3742, #3738, #4020 and #4299; any account-pool +redesign; any Lab import into `src/router.ts`, `src/server/lifecycle.ts` or +`src/server/responses/core.ts`; security write-ups in any tracked directory +(scratch only, per AGENTS.md); rewriting another maintainer's branch; and the +uncommitted `src/codex/inject.ts` and `src/codex/sync.ts` changes observed in the +parent checkout, whose author was never identified. + +## Terminal outcomes + +DONE needs 9 or more PRs merged, each with a CI run id at its merged SHA or an +explicitly recorded decision to merge without observing CI, both rounds closed with +a post-merge `dev` run, the named issues closed with merge references, and the +superseded set closed with written reasons. NEEDS_HUMAN is reserved for the +security review on #4519 and #4528. A heartbeat firing, a wait timeout or a +compaction is none of these. diff --git a/devlog/_fin/260914_triage_round_lanes/010_wp2_round1.md b/devlog/_fin/260914_triage_round_lanes/010_wp2_round1.md new file mode 100644 index 0000000000..b753527feb --- /dev/null +++ b/devlog/_fin/260914_triage_round_lanes/010_wp2_round1.md @@ -0,0 +1,184 @@ +# 010 — wp2: Round 1 + +Bottom layers plus the clean half of the merge track. Target: five PRs merged into +`dev` in one CI generation, closed by a post-merge `dev` run. + +Every source fact below was read live on 2026-09-14 by read-only recon agents. +Re-verify line numbers before editing; `dev` moves. + +## Round 1 contents + +| # | Branch or PR | Owner | Deliverable | +|---|---|---|---| +| 1 | `codex/260914-l1-bridge-backend-model` | L1 | sidecar model bound to the bridge backend | +| 2 | `codex/260914-l2-devin-catalog-supports-images` | L2 | catalog parser preserves field 5 as true/false/unknown | +| 3 | `codex/260914-l3-cursor-spare-budget` | L3 | spare-budget restoration of recent invocations | +| 4 | #4511 (`maoxin1234:fix/vision-native-model-capabilities`) | main | merge as-is | +| 5 | #4512 (`maoxin1234:fix/audio-outcome-accounting`) | main | merge as-is | + +## L1 bottom — bridge backend/model agreement + +File: `src/web-search/passthrough-bridge.ts`. + +`sidecarSettingsForBridge` (677-705) takes `sidecar.model` from the global +`config.webSearchSidecar` blob without checking that the global backend equals the +bridge backend, and `core.ts:6290-6296` passes that blob whole. A global +`{backend:"openai", model:"gpt-5.6-luna"}` therefore reaches +`runAnthropicWebSearch` when a provider sets `webSearchBridge.backend: "anthropic"`. +Credentials do not leak — `resolvePassthroughWebSearchBridgeAuth` (155-181) +inspects only the named backend — so this is a model and settings defect, not a key +defect, and the PR body must say exactly that. + +```diff +- sidecar?: Pick; ++ sidecar?: Pick; + +- const model = backend === "anthropic" ? sidecar.model ?? DEFAULT_ANTHROPIC_BRIDGE_MODEL +- : backend === "xai" ? sidecar.model ?? DEFAULT_XAI_BRIDGE_MODEL +- : backend === "gemini" ? sidecar.model ?? DEFAULT_GEMINI_BRIDGE_MODEL +- : sidecar.model ?? DEFAULT_OPENAI_BRIDGE_MODEL; ++ const model = modelForBridgeBackend(backend, sidecar); +``` + +`modelForBridgeBackend` applies `sidecar.model` only when +`resolveSidecarBackend(sidecar.backend) === backend`, and otherwise returns the +backend default. Reuse `resolveSidecarBackend` from `src/web-search/index.ts`; do +not invent a second resolver. `planWebSearch` (index.ts:112-208) already pins the +backend first and is the precedent to follow. + +Tests go in `tests/web-search/web-search-passthrough-bridge.test.ts`: a global +openai model with an anthropic bridge yields `claude-sonnet-5`; a matching backend +keeps the override; repeat the matrix for xai and gemini. Structure owner is +`structure/runtime.md`, hosted-search bridge contract at L238-247. + +Destination policy stays out of this layer. That is L1 top. + +## L2 bottom — Devin catalog preserves supportsImages + +File: `src/adapters/devin/cloud-direct/catalog.ts`. + +`parseCatalogBuffer` (107-141) has arms for fields 1, 4, 18 and 22 and no default, +so `ClientModelConfig` field 5 is dropped by omission. `disabled` defaults to +`false` when absent; image support must not copy that pattern, because an absent +field has to stay unknown. That is the #1796 precedent, and Antigravity already +implements the tri-state at `src/providers/antigravity-models.ts`:587-592. + +```diff + export interface ModelCatalogEntry { + modelUid: string; label: string; disabled: boolean; contextWindow?: number; ++ /** ClientModelConfig #5. Present true/false asserts; omitted means unknown. */ ++ supportsImages?: boolean; + } ++ let supportsImages: boolean | undefined; + } else if (sf.num === 4 && sf.wire === 0) { disabled = sf.value === 1n; ++ } else if (sf.num === 5 && sf.wire === 0) { supportsImages = sf.value === 1n; + byUid.set(modelUid, { modelUid, label: label || modelUid, disabled, + ...(contextWindow > 0 ? { contextWindow } : {}), ++ ...(supportsImages !== undefined ? { supportsImages } : {}), + }); +``` + +Extend the file-header schema comment with `#5 supportsImages bool` in the same +commit; the comment is the only schema documentation this parser has. + +Test: extend `"the catalog parser reads the per-account context window"` in +`tests/providers/devin-adapter.test.ts` (300-323) with three cases built from +`encodeVarintField(5, 1)`, `encodeVarintField(5, 0)` and omission, asserting +`true`, `false` and `undefined`. Encoder helpers already exist at +`src/adapters/devin/cloud-direct/wire.ts`:54-63. + +PR #4511 touches only `src/vision/eligibility.ts` and its test, so there is no file +overlap. Do not wait on it and do not restack into it. + +Corrections returned by L2 after doing the work: `encodeVarintField` is at +`wire.ts`:63-65, not 54-63; `parseCatalogBuffer` spans 107-142; the file-header +schema comment omits field 18 as well as field 5, so the patch documents both and +carries per-field provenance instead of a blanket "verified against extension.js" +claim, because field 5 identity is corroborated from external ClientModelConfig +documentation rather than re-read from the vendor bundle. For this layer the only +structure owner that actually documents the Devin adapter in prose is +`structure/adapters/registry.md`; `structure/catalog.md` has no Devin mention and +owns the round-2 provider-fetch surface instead. + +## L3 bottom — Cursor spare-budget restoration + +File: `src/adapters/cursor/protobuf-request.ts`. The per-call 2 KiB cap +(`CURSOR_INVOCATION_ARGUMENTS_BYTE_LIMIT`, line 84) is applied at envelope +construction in `toolCallArgumentsText` (937-967) and never revisited, even when +almost all of the 192-root and 512 KiB envelope is unused. A 4,693-byte successful +call loses its tail inside a 6,011-byte replay. + +Insert a second pass after `selected` is assembled (near line 694) and before the +return. It spends only leftover aggregate bytes, newest `toolResult` first, skips +`outputElided` roots, and never evicts a retained root. It does not remove the cap: +admission still uses the 2 KiB prefix. + +Two review-surviving details from recon: + +- Gate on `echoToolResultInRoot && replayedCalls` rather than + `externalModel && replayedCalls`. Native `composer-2.5` echoes results into roots + but is not an external wire model, so the narrower gate would leave it capped. +- Use the callback form of `String.prototype.replace` so replacement patterns + appearing inside serialized arguments are not expanded by the replace call. + +The two existing 600 KiB tests in +`tests/providers/cursor/cursor-tool-result-invocation.test.ts` stay unchanged and +must stay green: `"PROBE a huge argument must not evict the result output from root +replay"` (244) and `"the truncated invocation line stays within the declared +argument budget"` (261). They are the proof that the cap still bites under +pressure. + +The PR body states that this claims nothing about #3506 causation. The four +rejected patches there were 1,648, 1,396, 1,670 and 1,900 bytes, all under the cap. + +## Merge track in round 1 + +Every contributor PR has Cross-platform CI queued and stuck at `action_required`; +none has ever executed a job. Approving those runs is the first action of the +round, not the last. + +- #4511 at `6d926244101cb6234b66634464007268a06b1d89`: no outstanding reviewer ask, + non-draft, readiness 4/4. Approve CI, wait for green, merge. +- #4512 at `0235ce604cb907185295e42b05754d11d11a0bb2`: non-draft, readiness 4/4, + one open CodeRabbit ask for live `handleExternalLive` regressions. Do not push a + carry commit onto that branch. The readiness gate binds to the exact head, so a + new push resets the checklist and re-drafts the PR. Merge as-is and deliver the + asked coverage as our own follow-up PR in round 2. +- #4528 (2/4) and #4529 (0/4) are drafts. Approve their CI in round 1 so round 2 + has real information, but plan them as carries rather than direct merges. + +## Merge and proof mechanics + +Each lane pushes with `--no-verify`, fast-forward only. Non-tip chain commits carry +`[skip ci]`. For each PR, dispatch CI explicitly, because a sync or rebase does not +reliably queue it: + +```sh +gh workflow run ci.yml --ref -F lane=all +``` + +One ruleset term the batch argument has to respect: +`require_extra_approval_for_unattributed_changes` is true on `dev`. A carry that +replays another author's commits can therefore need an approval beyond the standard +one. The mitigation is to author each carry as our own commits with a +`Co-authored-by` trailer rather than cherry-picking the contributor's commits; the +trailer is what GitHub reads for credit, and the change stays attributed to the +pushing author for the ruleset. + +Wait for that run at the exact head SHA, then merge. Because `dev` carries no +required-status-check rule and no strict up-to-date rule, the five PRs may merge +back to back inside one generation; their write-sets are disjoint, so the residual +risk is semantic rather than textual. Close the round with one post-merge `dev` +Cross-platform CI run and record its id. + +Every PR body fills Summary, Verification and Checklist, and states plainly that +the local suite, typecheck, build and install were NOT RUN, naming the hosted run +id and SHA as the only proof. + +One thing the lanes discovered that makes this policy stronger than intended: a +fresh lane worktree has no `node_modules`, so a focused `bun test` cannot execute +at all without `bun install`. Local focused runs are therefore impossible in a lane +rather than merely disallowed, and hosted CI is the only evidence that could exist. +The one check that runs without install is `bun run structure:check`, used for +debugging the doc gate and never cited as proof. + diff --git a/devlog/_fin/260914_triage_round_lanes/020_wp3_round2.md b/devlog/_fin/260914_triage_round_lanes/020_wp3_round2.md new file mode 100644 index 0000000000..b3bc96451b --- /dev/null +++ b/devlog/_fin/260914_triage_round_lanes/020_wp3_round2.md @@ -0,0 +1,201 @@ +# 020 — wp3: Round 2 + +Top layers, the carries, and the security-gated remainder. Target: four to six more +PRs merged, bringing the loop total to 9-11. + + +## Round-2 state at entry (recorded 2026-09-14) + +Round 1 landed six PRs and the tree underneath this document moved, so these facts +replace the assumptions the sections below were written against. + +`dev` is `866367a6f`. It carries all six round-1 merges plus a +`chore(release): open dev at 2.55.0` commit pushed by a separate release train. +That push is also why the first post-merge `dev` run was cancelled: the concurrency +group killed it when the version bump arrived. The joint proof was retaken at the +new tip, run `34778300807`, success with no failing jobs. Expect the same to happen +again — if a `dev` run is cancelled, check whether `dev` moved before treating it +as a failure, and retake the proof at the tip. + +Three round-2 targets are already resolved or partly resolved: + +- #4501 and #4502 are CLOSED as COMPLETED. Repository automation closed them when + #4511 and #4512 merged, so wp4 does not need to close them. +- #4529 is CLOSED with a comment naming #4545 and the merge commit + `e30f1d27eec4ac22baeb6d5212413bc645bb6786`. The carry rationale in the L4 section + below is now settled history rather than a pending decision. +- #4522 is still OPEN and is now closable: the carry that fixes it has landed. + It belongs in the wp4 sweep with `e30f1d27e` as its reference. + +#4528 moved to head `d1d8d45f2` after the author pushed again, which reset its +readiness checklist and left fresh workflow runs waiting at `action_required`. +Those were approved on entry to round 2, so its first real CI verdict should exist +by the time the round is ready to merge. + +One process correction carried forward from round 1: a push to a branch with an +open PR already queues a full Cross-platform CI run, so the explicit +`gh workflow run` dispatch in the sections below is a fallback, not a routine step. +Doing both produced eight full runs for four PRs and roughly two hundred jobs +competing for the same macOS runners; four duplicates were cancelled to clear it. +Dispatch explicitly only when no run appears for the exact head. + +## Retarget mechanics + +Round 1 merges the bottom of each chain. Delete the bottom branch on merge and +GitHub retargets the open child PR to `dev` by itself, which is the cheapest +correct path. Then bring `dev` into the child by merging it, not by rebasing: +`dev` blocks non-fast-forward pushes, and an append-only merge keeps the child +pushable with `--no-verify` and no force. Re-dispatch CI after that merge, because +the child head changed and the earlier green no longer describes it. + +## L1 top — endpoint destination policy (#4519) + +The policy already exists and is already applied to provider `baseUrl`. The defect +is that no call site applies it to `webSearchBridge.endpoint`, so an operator +endpoint is returned verbatim after a shape-only check and the serving provider key +is sent there as a Bearer token. + +Reuse `providerDestinationConfigError` from `src/lib/destination-policy.ts`:306. +Metadata destinations always fail; loopback, localhost and private require the +existing `allowPrivateNetwork` opt-in or the registry default. Do not write a new +classifier. + +Two call sites, both needed, because config-time alone does not cover a file that +was edited by hand. + +An audit against the real signatures corrected where the config-time half goes. +`providerWebSearchBridgeConfigError` takes `(value: unknown)` and receives only the +bridge object, so neither the provider name nor `allowPrivateNetwork` is in scope +inside it and the sketch below does not compile as placed. Put the check at its sole +caller, `src/server/auth-cors.ts`:769, where `name`, `raw` and `typed` are already +in scope, or widen the helper's signature and update that caller. Either is fine; +the PR says which. + +The real signature is +`providerDestinationConfigError(name: string, provider: Pick): string | null`, +and every message it returns begins with `baseUrl`, including the secure-transport +arm, so one `/^baseUrl/` rewrite covers them all. + +The file-load path at `src/config.ts`:684 only schema-parses with +`.catch(undefined)` and runs no error function, so a hand-edited config never reaches +the config-time check at all. That makes the runtime planner check the load-bearing +one rather than a belt over a brace, and the PR description should say so. It is the +same shape `baseUrl` itself already has. + +```diff +# src/config.ts providerWebSearchBridgeConfigError (522-545) ++ const destinationError = providerDestinationConfigError(providerName, { ++ baseUrl: endpoint, allowPrivateNetwork: provider.allowPrivateNetwork, ++ }); ++ if (destinationError) return destinationError.replace(/^baseUrl/, "webSearchBridge.endpoint"); + +# src/web-search/passthrough-bridge.ts resolveOllamaWebSearchEndpoint (119-134) + if (configured !== undefined) { +- return originOf(configured) === undefined ? undefined : configured; ++ if (originOf(configured) === undefined) return undefined; ++ if (providerDestinationConfigError(providerName, { ++ baseUrl: configured, allowPrivateNetwork: provider.allowPrivateNetwork, ++ })) return undefined; ++ return configured; + } +``` + +The runtime check needs the provider name for the registry default; the planner +already holds the provider object, so thread the name through +`planPassthroughWebSearchBridge`. The management write at +`src/server/auth-cors.ts`:769-771 has `name` and `raw` in scope already. + +The comment at `src/config.ts`:508-512 claims the planner re-validates the origin +before any key is sent. That is weak rather than false: the planner does re-check +the origin, but only through `originOf`, which is a URL-shape parse and not a +destination assessment. Tighten the comment to say which check actually runs, and +say it precisely, because this is the security-gated PR and an overstated defect +claim in its own description is the fastest way to lose a reviewer. + +Tests: metadata endpoint rejected at config write and leaves the bridge disarmed at +plan time; a private address rejected without opt-in and accepted with it; the +canonical `https://ollama.com/api/web_search` still passes. Do not fold DNS +resolution into this slice; `baseUrl` does not do it at this boundary either. + +This layer is security-sensitive under MAINTAINERS.md and needs independent review. +If that review has not happened by the time the rest of round 2 is ready, merge the +rest and leave this PR open rather than holding the round. + +## L2 top — capability propagation and precedence + +Carry the parsed flag through to the client. Three hops, all named by recon: + +```diff +# src/adapters/devin/live-models.ts DevinUsableModelsResult (140-142) +- | { ok: true; models: string[]; contextWindows: ...; efforts: ... } ++ | { ok: true; models: string[]; contextWindows: ...; efforts: ...; inputModalities: Record } + +# src/codex/catalog/provider-fetch.ts Devin branch (1736-1750) ++ ...(liveResult.inputModalities[id]?.length ++ ? { inputModalities: liveResult.inputModalities[id] } : {}), + ...catalogHintsFromProviderConfig(...), +``` + +Collapse policy across the effort variants of one base, mirroring the `Math.min` +treatment of context windows: all known values true gives `["text","image"]`, all +known false gives `["text"]`, and any unknown or any disagreement omits the key. +One unsuffixed unknown row must not poison a measured image base, and a single +false must not be overridden by its siblings. + +Precedence needs no Devin special case. `applyProviderConfigHints` (753-827) already +puts exact `modelCapabilities[id].inputModalities` first, then the fuzzy legacy +record, then the sidecar-consumer rewrite that adds image so the app does not block +attachments, and the live row survives only when none of those fired. Keep +`catalogHintsFromProviderConfig` spreading last, which is what makes that ordering +real. + +Tests: the `fetchDevinUsableModels` collapse matrix against a fake cached catalog; +a live image row with an exact operator text-only declaration still taking the +existing sidecar path; and the existing `"discovery-derived text-only rows are NOT +advertised image"` case staying green. + +## L3 top — budget and ordering regressions + +Three cases in `tests/providers/cursor/cursor-tool-result-invocation.test.ts`: spare +space preserves a complete just-over-cap successful call; spare space restores the +newest call without evicting an older result; a checkpoint-covered call keeps its +tail in the result suffix. Add UTF-8 round-trip and an `outputElided` skip if the +lane has room. Port the fixture out of scratch into the suite rather than leaving +it in `.tmp`. + +## L4 — the carries + +Both carries need a `Co-authored-by` trailer in a branch commit so it survives the +squash. Prose credit is not equivalent; `missing_coauthor_credit` in +`.github/scripts/pr-carry-attribution.cjs` is the check, and CREDITS.md is the list +of 27 landings that already got this wrong. + +L4-a carries #4529 by Voyagerroc-Lab: the version-skew refusal plus the `"unknown"` +health-version case that CodeRabbit asked for. That case must be authored, not +cloned. The `"0.0.0"` placeholder test it parallels exists only inside #4529's own +head diff; `dev` has no such test, and `0.0.0` appears nowhere under `tests/`. The +carry brings the placeholder test along and adds the unknown case beside it. The +carry also writes `src/cli/index.ts` and `src/cli/system-restart-client.ts`, so it +owes a structure update and `bun run structure:check` like any source change. The carry +exists because #4529 is a draft at 0/4 whose readiness gate the author has not +completed; carrying is the repository-sanctioned way past that, and #4529 closes +with credit once the carry lands. + +L4-b is a follow-up rather than a carry: #4512 merges as-is in round 1, and this PR +adds the two live `handleExternalLive` regressions to +`tests/server/audio-dictation.test.ts` — invalid answer data yielding client 502 +while `recordCodexUpstreamOutcome` books 200, and alias-registration failure +yielding client 503 with the same booked 200. They do not belong in +`tests/server/audio-transcriptions.test.ts`; that file already covers the +transcription half. + +## #4528 + +Draft at 2/4, 21 files, security-adjacent, and its only live code ask is one +Turkish wording fix at `docs-site/src/content/docs/tr/guides/combos.md`:410. The +stale CodeRabbit thread about image-failover documentation is already satisfied by +the current head and should be resolved rather than acted on. Decide in round 2 on +evidence: if its approved CI run is green and an independent security review is +available, merge; otherwise leave it open and record NEEDS_HUMAN for that item +alone. + diff --git a/devlog/_fin/260914_triage_round_lanes/030_wp4_closure.md b/devlog/_fin/260914_triage_round_lanes/030_wp4_closure.md new file mode 100644 index 0000000000..2d1866d0ba --- /dev/null +++ b/devlog/_fin/260914_triage_round_lanes/030_wp4_closure.md @@ -0,0 +1,64 @@ +# 030 — wp4: Closure sweep + +## The finding that shapes this phase + +A full cross-reference of 61 open issues and 68 open PRs against every PR merged in +the last ten days produced an empty CLOSE-NOW list. Not one open item could be +closed on merged work alone. Several backlog PRs name an issue in their body while +saying explicitly that they do not close it, and the reported code path is still +present in the tree for every issue examined. + +So this phase does not sweep the backlog. It closes only what this loop actually +fixes, and it records the rest honestly. + +## Closable by our own merges + +| Issue | Closed by | Condition | +|---|---|---| +| #4530 | L2 bottom and top | catalog carries field 5 and the client advertises it | +| #4516 | L3 bottom and top | spare-budget restoration merged with its regressions | +| #4519 | L1 top | destination policy merged; blocked until security review | +| #4501 | #4511 | merged in round 1 | +| #4502 | #4512 | merged in round 1; L4-b adds the live coverage | +| #4522 | L4-a carry | carry merged, original credited | +| #4527 | #4528 | only if #4528 lands | + +Each close comment names the merge commit and the file and line that changed. A +close without that pointer is not acceptable here, because the whole reason the +sweep is this narrow is that unverified closes were the failure mode found. + +## Partially addressed — leave open, state the residual + +#4429 keeps mixed-tool continuation fail-closed at +`src/web-search/passthrough-bridge.ts`:594 even though #4515 armed the non-Ollama +backends; the merged PR body says so itself. #4312 still maps only +`max_output_tokens` to HTTP 200 at `src/server/request-log.ts`:966, so an Anthropic +refusal still reads as a retryable 502. #4191 still carries a 90-second prelude +timeout at `src/server/responses/codex-ws-wire.ts`:22; the WS diagnostics that +landed made the failure observable, not absent. #4311 still refuses paginated +history writes at `src/codex/history-provider.ts`:275 with no native writer behind +it. #3661, #3522, #3506, #4505, #3781, #3376, #3377, #3375 and #3719 are in the +same shape. + +For each of these, the action is a comment naming the residual and the evidence +line, not a close. That comment is worth writing because the next triage will +otherwise re-derive the same conclusion from scratch. + +## Superseded + +No open PR was verified as already fully present on `dev`. The two candidates that +look superseded are not: #2562 and #3283 are overlapping Antigravity pool designs +where neither is on `dev` and a maintainer has to pick, and #4242 was not carried +by the merged #4351, which says as much in its own body. + +Therefore this phase closes no PR as superseded. If that changes because a round-2 +merge lands something an open PR also contains, the close names the superseding +merge commit and quotes the current-tree evidence. + +## Unit closeout + +Move this unit to `devlog/_fin/` once the rounds are recorded, with the merge +commits, the CI run ids, and an explicit note of anything merged without observing +CI. Record the deviation list too: the empty CLOSE-NOW result, and any lane that +did not produce its top layer. + diff --git a/devlog/_fin/260914_triage_round_lanes/040_outcome.md b/devlog/_fin/260914_triage_round_lanes/040_outcome.md new file mode 100644 index 0000000000..708d306fd2 --- /dev/null +++ b/devlog/_fin/260914_triage_round_lanes/040_outcome.md @@ -0,0 +1,82 @@ +# 040 — Outcome + +Closed 2026-09-14. Ten pull requests landed on `dev` across two merge rounds, run by +four worktree lane threads. `dev` ended at `e97ed7afd`. + +## What landed + +| Round | PR | Change | Merge | Exact-head CI | +|---|---|---|---|---| +| 1 | #4511 | native vision honors operator `modelCapabilities` | `10d61fc2b` | 34758154482 | +| 1 | #4512 | audio routes record the real upstream status | `9b2fc10bc` | 34758292008 | +| 1 | #4545 | restart refuses a version-skewed CLI (carries #4529) | `e30f1d27e` | 34775280313 | +| 1 | #4548 | bridge search model bound to the bridge backend | `1a9423469` | 34776449529 | +| 1 | #4547 | Devin catalog keeps `supportsImages` as a tri-state | `6329f3038` | 34775751844 | +| 1 | #4543 | Cursor replay refunds spare bytes to clipped arguments | `f7e4af080` | 34776361364 | +| 2 | #4553 | live-outcome booking regressions for #4512 | `2176c5bc0` | 34779112640 | +| 2 | #4554 | budget and ordering regressions for the refund pass | `56c956715` | 34779412583 | +| 2 | #4556 | Devin capability propagation and precedence | `72335fc6a` | 34782050873 | +| 2 | #4557 | counter-read folds for the propagation layer | `e97ed7afd` | 34783132657 | + +Joint proof: post-merge `dev` runs 34778300807 at `866367a6f` and 34782580496 at +`72335fc6a`, both success with no failing jobs. Every merge verified the check +run's `head_sha` against the PR head immediately before merging, so no PR landed on +CI that described a different commit, and no merge was taken without observing CI. + +Issues closed with merge references: #4501 and #4502 by repository automation, +#4529 naming #4545, and #4522, #4530 and #4516 by this unit after an independent +audit of each claim against the tree. + +## What did not land, and why + +**#4555 (#4519 endpoint destination policy) is green and deliberately unmerged.** +MAINTAINERS.md requires explicit security review for credential-handling changes, +and this endpoint receives the serving provider's API key as a Bearer token. The +`dev` self-integration exception covers a missing second approval; it does not +cover that review. An adversarial security review ran instead and returned **fail** +on a real finding: a provider keyed under a custom name with a loopback endpoint +used to arm and now disarmed with no operator-visible signal, because the config +load path runs no error function and the plan-time refusal is silent by design. The +lane fixed it with one deduped warning per provider and endpoint that names the +remedy and omits the URL. Head `e8b36b0e2` is green; the PR waits on @Ingwannu. + +**#4527 stays open** because its fix, #4528, is a draft whose author pushed a new +head mid-round; its runs were re-approved but it was never ready to merge. + +Eleven issues were examined and deliberately left open with their residuals named, +including #4429, #4312, #4191, #4311, #3522, #3661, #4469, #4505 and #3506. The +closure cross-reference that opened this unit found an empty CLOSE-NOW list across +61 issues and 68 PRs, and that held: every close here was created by tonight's own +merges, not discovered in the backlog. No open PR was verified as superseded. + +## What this unit learned + +**A no-local-verification policy moves the cost to CI, and the cost is real.** L3's +first head failed the `gates` Typecheck step. That was found by repository CI, +relayed to the lane, fixed, and re-verified. Later a lane reported the stronger +fact: a fresh lane worktree has no `node_modules`, so a focused local run cannot +execute at all. Hosted CI is not merely the preferred evidence here; it is the only +evidence that exists. + +**The lanes throttled themselves.** Each lane both pushed and dispatched CI +explicitly, producing eight full runs for four PRs and roughly two hundred jobs +competing for the same macOS runners. A push to a branch with an open PR already +queues a run; the explicit dispatch is a fallback for a rebase or base sync, which +is what the roadmap had meant. Four duplicates were cancelled and the rule was +corrected. Round 2 ran only `pull_request` events. + +**A cancelled `dev` run is not a failure.** Twice a `dev` proof run was cancelled by +the concurrency group when a separate release train pushed a version bump. The +proof was retaken at the new tip both times. Read why a run cancelled before +treating it as red. + +**Attribution has to be a trailer.** #4545 and #4553 both carry `Co-authored-by` +trailers, verified present in the squash commits after merging. Prose credit would +have vanished at squash time, which is how the 27 entries in CREDITS.md happened. + +**Every audit gate found something.** The roadmap audit returned near-pass on three +blockers including a carry that would have written `src/cli/` without the structure +obligation the plan assigned it. The round-2 audit found a diff sketch that +referenced variables not in scope and would have cost a CI round trip. The security +review found the silent regression. None of these were style notes. +