mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-08-29 03:44:06 +08:00
docs(review): add seven-lens review of PR #13513
This commit is contained in:
Johnny Eric Amancio
@@ -0,0 +1,436 @@
|
||||
# Broken Pipeline Chains — PR #13513
|
||||
|
||||
## Scope and method
|
||||
|
||||
Reviewer 5 of 7; this report is the only file written by this reviewer. Review checkout: `/Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports`.
|
||||
|
||||
**Lens verdict: safe after specific fixes.** The original two merge-relative P2 findings are preserved: one from the upstream range, one from SDK regeneration/adaptation and also present on the supplied main control. The completed static follow-up adds two **pre-existing Kilo P2** TUI findings, excluded from this PR's regression verdict, and one precisely scoped human-verification item. No new P0/P1 issue was established.
|
||||
|
||||
Reviewed exact HEAD `6a7d6bc002319ac2987bcde3d6c63efcafc07021` against actual base/merge base `bf1cf502a3c511e9daf6a43244568ae4e83473a8`, not against main. Main control: `62998965e9fb0d9ed89011c62498b39801dbbb4f`. Verified the provided authoritative local upstream refs:
|
||||
|
||||
| Ref suffix | Commit |
|
||||
|---|---|
|
||||
| upstream-v1.18.18 | `31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d` |
|
||||
| upstream-v1.18.19 | `2b72179c663cadcb54f54d9f19221b3fb3d11fb6` |
|
||||
| upstream-v1.18.20 | `7248bc1964b13fa67e601733f89ee9dc6dfa0563` |
|
||||
|
||||
Confirmed 59 changed files (3 added/56 modified), 1,524 insertions/647 deletions, 95 reachable commits and two first-parent commits. `91ca95bad927436131ea4783a470885a381ce6ad` has base and pristine .20 parents and the same tree as base. Final HEAD has `91ca95bad9` and transformed `9563af96a012effc25df5a11eaa1f7633161a742` parents. Pristine .18 → .20 changes 181 files, so that larger upstream inventory must not be mistaken for this PR's 59-file delta.
|
||||
|
||||
Read root `AGENTS.md`, `REVIEW.md`, `TESTING.md`, `kilo-steer`, merge-review validation/audit references, merge-minimizer guidance, upstream merge documentation, and applicable CLI/test/HttpApi/LLM/core-tool/VS Code instructions. Inspected the complete handwritten production delta, generated-client transport and signature changes, all changed-file marker locations, and removed marker lines. The marker inventory contains **737 marker-bearing lines in 29 changed files**; this counts delimiters/imports as well as behavior, not 737 independent features.
|
||||
|
||||
Method: trace producers through transformations, registration/service graphs, persistence/events and consumers; exercise production functions and existing implementation tests; compare suspicious behavior to base, main, pristine .20 and merge parents. Inline historical controls transpile exact functions from `git show` in memory rather than modifying the checkout. Compilation alone is not treated as semantic proof.
|
||||
|
||||
**Static follow-up completeness:** completed bounded producer → intermediate transformation/state/registration → concrete consumer tracing for **737/737 marker-bearing lines in 29/29 changed files: 17 production files and 12 test files**, including historical regions and all four removed marker-bearing lines. Delimiters, adjacent imports and related declarations were grouped by behavior; type-only markers terminate at their schema/public-type consumer rather than being counted as independent runtime flows. No marker region remains omitted from the in-repository source trace. This replaces the first round's incomplete-source-audit qualification; it does **not** claim runtime execution of every branch or external-provider certification.
|
||||
|
||||
The audit used **20 semantic families**: search/spawn; core projection/replay; core context/compaction; CLI arguments/stdin; CLI cloud/local/daemon dispatch; headless event/reply ownership; catalog/model fields; provider credentials/headers; plugin registration/lifecycle; request option/schema/cache lowering; transport timeout/WebSocket settlement; stream error/retry; billing/routed metadata; session rows/listing/plan paths; session fork/deletion/sandbox inheritance; Task orchestration; tool visibility/host RPC; TUI prompt/memory arbitration; TUI command/export/feedback; and TUI part rendering/approval/diffs. Marked test regions were traced into their corresponding production family, including fixture/layer substitutions and final assertions. The generated SDK was also audited beyond its marker inventory. Material conclusions and exact exceptions are grouped below, not presented as a per-file checklist.
|
||||
|
||||
## Findings
|
||||
|
||||
### P2 — SDK-wide `throwOnError` no longer reaches Kilo's error normalizer
|
||||
|
||||
**Location:** `packages/sdk/js/src/v2/gen/client/client.gen.ts:201`; consumer `packages/sdk/js/src/error-interceptor.ts:19`; registration `packages/sdk/js/src/v2/client.ts:100`.
|
||||
|
||||
**Provenance:** introduced by merge adaptation/SDK regeneration relative to this PR's actual base. Base and pristine .20 pass the control; HEAD fails. The supplied main also fails. The pristine and transformed-parent generated transport blobs have no delta; the rewritten transport appears in final HEAD. This is not a pristine .18 → .20 runtime change, nor an issue already present in this PR's immediate base.
|
||||
|
||||
**Broken chain:** `createKiloClient({ throwOnError: true })` → generated `_config` → request's effective `throwOnError` → HTTP error body → error interceptors → `wrapClientError` → caller's rejected promise. The generated request correctly resolves `options.throwOnError ?? _config.throwOnError` for whether to throw, but sends the **unmerged per-call `options`** to interceptors. `wrapClientError` sees no `opts.throwOnError` and returns the raw JSON object. The outer request then throws that object.
|
||||
|
||||
**Concrete failure:** an SDK consumer configuring error throwing once at client creation receives `{ name: "NotFoundError", data: { message: "session missing" } }` instead of an `Error`; `error.message` is undefined and `String(error)` is `[object Object]`. Error-formatting and `instanceof Error` handling lose the useful server diagnostic. This is reproducible through the public SDK, not only the internal transport. Most inspected in-repo callers supply `throwOnError` per call, so a broad TUI/extension failure is **not** claimed.
|
||||
|
||||
**Proof/control:** a fake 404 at the fetch boundary with the real public client produces `errorInstance:false,message:null` with the client-wide default, and `errorInstance:true,message:"session missing"` with the identical option passed per call. The exact historical transport plus the unchanged real Kilo interceptor gives:
|
||||
|
||||
```text
|
||||
{"ref":"HEAD","errorInstance":false,"message":null}
|
||||
{"ref":"bf1cf502a3c511e9daf6a43244568ae4e83473a8","errorInstance":true,"message":"session missing"}
|
||||
{"ref":"62998965e9fb0d9ed89011c62498b39801dbbb4f","errorInstance":false,"message":null}
|
||||
{"ref":"7248bc1964b13fa67e601733f89ee9dc6dfa0563","errorInstance":true,"message":"session missing"}
|
||||
```
|
||||
|
||||
**Alternative challenged:** the new behavior might be an intentional generic SDK error-policy change. That does not explain away the Kilo regression: the handwritten wrapper explicitly promises normalized thrown errors, remains installed, and works when the effective option happens to be present per call. The break is the missing default-option pass-through.
|
||||
|
||||
**Fix direction:** ensure the Kilo interceptor receives the effective `throwOnError` value, including client defaults, while preserving raw tuple errors when effective throwing is false. Prefer a narrow handwritten-wrapper or generator compatibility seam rather than an unmaintained manual generated edit. Cover global true/per-call undefined, global true/per-call false, per-call true, and network/request-construction errors.
|
||||
|
||||
### P2 — Disabled workspace listing becomes an empty deduplication source for `syncList`
|
||||
|
||||
**Location:** `packages/opencode/src/control-plane/workspace.ts:717`, with internal reader at `:729` and writer at `:759`.
|
||||
|
||||
**Provenance:** introduced by the upstream .18 → .20 range, retained in the merge. Base and supplied main do not duplicate rows; HEAD and pristine .20 do.
|
||||
|
||||
**Broken chain:** `POST /experimental/workspace/sync-list` → `WorkspaceHttpApi.syncList` → `Workspace.syncList` → `Workspace.list` → existing-name set → adapter discovery → insert new workspace ID. The new flag-off `return []` is correct for presentation, but `syncList` also uses `list` as its database deduplication source. It is not itself flag-gated.
|
||||
|
||||
**Concrete failure:** with a registered adapter returning a previously stored workspace, each sync-list request while `experimentalWorkspaces=false` inserts another row for the same workspace name/directory. Listing hides those rows while disabled; turning the feature back on reveals duplicates with independent IDs. This affects API/SDK callers and mixed client/server flag states; normal same-process flag-off TUI menu use is not claimed, because its workspace command is hidden.
|
||||
|
||||
**Reachability:** `packages/opencode/src/server/routes/instance/httpapi/api.ts:98` adds the workspace API unconditionally. `groups/workspace.ts:85` declares the sync-list endpoint without a feature-flag middleware, and `handlers/workspace.ts:51` calls the service without a flag check. The SDK exposes it, and TUI dialogs call it at `packages/tui/src/component/dialog-workspace-list.tsx:91` and `dialog-workspace-create.tsx:81`. The workspace SQL table has only an ID primary key, not a unique project/name constraint (`packages/core/src/control-plane/workspace.sql.ts:6`).
|
||||
|
||||
**Proof/control:** executed the exact `fromRow`, `list` and `syncList` implementations from each commit with real Effect/Drizzle and an in-memory SQLite table. Seeded one row, returned the same workspace from a controlled adapter boundary, and invoked `syncList` twice. Assertions checked row count and calls into the sync-start boundary:
|
||||
|
||||
```text
|
||||
{"ref":"HEAD","enabled":false,"rows":3,"attemptedSync":2,"visible":0}
|
||||
{"ref":"HEAD","enabled":true,"rows":1,"attemptedSync":0,"visible":1}
|
||||
{"ref":"bf1cf502a3c511e9daf6a43244568ae4e83473a8","enabled":false,"rows":1,"attemptedSync":0,"visible":1}
|
||||
{"ref":"bf1cf502a3c511e9daf6a43244568ae4e83473a8","enabled":true,"rows":1,"attemptedSync":0,"visible":1}
|
||||
{"ref":"62998965e9fb0d9ed89011c62498b39801dbbb4f","enabled":false,"rows":1,"attemptedSync":0,"visible":1}
|
||||
{"ref":"62998965e9fb0d9ed89011c62498b39801dbbb4f","enabled":true,"rows":1,"attemptedSync":0,"visible":1}
|
||||
{"ref":"7248bc1964b13fa67e601733f89ee9dc6dfa0563","enabled":false,"rows":3,"attemptedSync":2,"visible":0}
|
||||
{"ref":"7248bc1964b13fa67e601733f89ee9dc6dfa0563","enabled":true,"rows":1,"attemptedSync":0,"visible":1}
|
||||
```
|
||||
|
||||
**Important bound on proof:** adapter enumeration, ID generation and the sync-start boundary were controlled; the real service logic and SQL writes were executed. This is not a full HTTP reproduction or a real remote-workspace connection. In fact, production `startSync` already exits when disabled at `workspace.ts:442`; **no flag-off remote connection bypass is alleged**. The defect is duplicate persistent rows before that guard.
|
||||
|
||||
**Alternative challenged:** disabling the feature could legitimately hide workspace state. That is compatible with this finding: hiding rows must not make a write-side reconciliation operation forget they exist. The enabled control proves the existing deduplication logic still works when the presentation filter is absent.
|
||||
|
||||
**Fix direction:** make `syncList` a no-op while disabled, or have its deduplication query read stored workspaces independently of the user-visible list gate. Add disabled/repeated-sync/re-enable coverage.
|
||||
|
||||
## Additional findings from the completed historical-marker trace
|
||||
|
||||
These do **not** add merge-relative regressions. Both are present in actual base and supplied main and should be handled as separate existing-product follow-ups.
|
||||
|
||||
### P2, pre-existing Kilo — Non-blocking suggestions never reach the imported TUI renderer
|
||||
|
||||
**Marker regions:** `packages/tui/src/routes/session/index.tsx:71–76` and `:261–308`; missing dispatch at `:1955–2013` and `:3049–3072`. The new upstream reasoning change does not modify these branches.
|
||||
|
||||
**Full chain:** registry imports/initializes `SuggestTool` → advertises it for CLI/VS Code at `packages/opencode/src/tool/registry.ts:313` → `kilocode/suggestion/tool.ts:74–79` publishes `blocking:false` plus message/call IDs → `Suggestion.show` stores the pending action and publishes `suggestion.shown` → `context/sync.tsx:353–356` updates `store.suggestion` → the session route filters it out of `blockingSuggestion` → the dedicated `Suggest`/`SuggestBar` renderer is never dispatched. `toolDisplay("suggest")` returns `generic`; the `Suggest` identifier occurs only in its import. Its intended consumer at `kilocode/suggestion/tui/render.tsx:63–64` is therefore unreachable from this route.
|
||||
|
||||
**Concrete failure:** built-in suggestions do not show their action buttons in the TUI, so users cannot accept the offered action through that UI. The tool awaits the pending result (`suggestion/tool.ts:96`), but a later prompt or abort can dismiss it; this is not described as an unavoidable permanent hang. VS Code's distinct rendering path is not implicated.
|
||||
|
||||
**Proof/control:** exact historical `toolDisplays`/`toolDisplay` execution and source-reference assertions, with ordinary registered tool rendering as the architectural control, produce:
|
||||
|
||||
```text
|
||||
{"ref":"HEAD","suggestDisplay":"generic","SuggestReferences":1,"renderedSuggest":false}
|
||||
{"ref":"bf1cf502a3c511e9daf6a43244568ae4e83473a8","suggestDisplay":"generic","SuggestReferences":1,"renderedSuggest":false}
|
||||
{"ref":"62998965e9fb0d9ed89011c62498b39801dbbb4f","suggestDisplay":"generic","SuggestReferences":1,"renderedSuggest":false}
|
||||
```
|
||||
|
||||
The strongest alternative—a second inline rendering site—was checked by searching `SuggestBar`, `<Suggest` and suggest dispatch across TUI and the Kilo suggestion render directory. The only `SuggestBar` invocation is inside the unused `Suggest` component. The blocking footer is a separate control and intentionally excludes the built-in tool's `blocking:false` request.
|
||||
|
||||
**Fix direction:** restore the `suggest` tool-display entry and dispatch `Suggest` with its matching pending request, `InlineTool` and `BlockTool`. Verify accept, new-prompt dismiss, abort and tool-result metadata rendering. No source fix was applied.
|
||||
|
||||
### P2, pre-existing Kilo — Parent/child navigation executes group-wide process cleanup
|
||||
|
||||
**Marker region:** `packages/tui/src/routes/session/index.tsx:354–390`; lifecycle caller `packages/tui/src/app.tsx:1152`.
|
||||
|
||||
**Full chain:** parent → child session navigation updates route ID → the keyed `<Show>` disposes the old `Session` component → its `onCleanup` calls `stopProcesses(processSessionID)` unconditionally → `processSessions` expands the parent and siblings → SDK `backgroundProcess.stopSession` → `kilocode/server/httpapi/handlers/background-process.ts:47–51` → `BackgroundProcess.stopSession` at `kilocode/background-process/index.ts:1231–1246` → ordinary session-lifetime processes are terminated. Persistent processes are outside that map, and a child process with parent lifetime is transferred rather than terminated by that particular call.
|
||||
|
||||
**Concrete failure:** merely opening a child task can stop an ordinary development server or watcher belonging to its parent session. The same-group guard at route line 384 only protects an update of a surviving component; it cannot suppress the separate cleanup triggered by keyed remounting.
|
||||
|
||||
**Proof/control:** ran the exact marked cleanup block from HEAD/base/main inside real Solid reactive primitives, with the same one-argument keyed `Show` callback shape used by `app.tsx`; only SDK calls were recorded instead of stopping OS processes. The materially different unkeyed control preserves the component and exercises the same-group guard:
|
||||
|
||||
```text
|
||||
{"ref":"HEAD","keyed":false,"navigation":"parent -> child","stopRequests":[]}
|
||||
{"ref":"HEAD","keyed":true,"navigation":"parent -> child","stopRequests":["parent","child"]}
|
||||
{"ref":"bf1cf502a3c511e9daf6a43244568ae4e83473a8","keyed":false,"navigation":"parent -> child","stopRequests":[]}
|
||||
{"ref":"bf1cf502a3c511e9daf6a43244568ae4e83473a8","keyed":true,"navigation":"parent -> child","stopRequests":["parent","child"]}
|
||||
{"ref":"62998965e9fb0d9ed89011c62498b39801dbbb4f","keyed":false,"navigation":"parent -> child","stopRequests":[]}
|
||||
{"ref":"62998965e9fb0d9ed89011c62498b39801dbbb4f","keyed":true,"navigation":"parent -> child","stopRequests":["parent","child"]}
|
||||
```
|
||||
|
||||
**Alternative challenged:** the existing same-group guard might already preserve the processes. It does in the unkeyed control, but the actual app uses a keyed session-ID boundary. This proof covers lifecycle and emitted stop requests, not a manual TUI/OS-process kill exercise.
|
||||
|
||||
**Fix direction:** own group-lifetime cleanup outside the keyed session component, or make unmount cleanup consult the destination group before sending stop requests. Preserve cleanup on actual group exit/application shutdown. No source fix was applied.
|
||||
|
||||
## Notable non-findings and bounded follow-ups, grouped by chain
|
||||
|
||||
### Provider construction → option lowering → wire request
|
||||
|
||||
- **Cloudflare:** auth/env account and gateway metadata → custom loader → native OpenAI/Anthropic or unified Workers AI model → AI SDK encoder → gateway envelope was exercised using the **actual custom loader extracted from production**, not just the test's mirrored `gatewayModel` helper. HEAD and pristine .20 emit OpenAI `v1/responses` and Anthropic `v1/messages`; both Workers ID forms retain `compat/chat/completions`. The synthetic Cloudflare token is present in the outer gateway auth header, absent from third-party upstream envelopes, and present for Workers AI. Base demonstrably forwarded it into the unified third-party envelope. This is a preserved upstream fix, not a lost credential pass-through.
|
||||
- Catalog/config model `api.npm` resolution precedes variants in `provider.ts:1298` and `:1523`; `ProviderTransform.providerOptions` chooses the native option namespace before `LLM.stream` uses it. Reasoning/cache/tool-schema transforms remain connected. Native LLM opt-in does not accidentally intercept Cloudflare after the npm rewrite: `native-runtime.ts:55` rejects that provider ID. Explicit custom npm/baseURL combinations and live BYOK billing remain unverified.
|
||||
- **Cerebras:** plugin import → internal registration (`plugin/index.ts:93`) → `Plugin.trigger` → `LLMRequestPrep.prepare` → `streamText.maxOutputTokens` remains connected. Kilo's existing `maxOutputTokensForRequest` independently preserves the same omission at `provider/transform.ts:1741`, including when default plugins are disabled. The new hook is redundant with that Kilo guard, but is not a broken chain. Tests cover configured cap, absent cap, and other-provider controls.
|
||||
- Removed Cloudflare `chat.params` cap suppression was checked against the new native routing; it is not silently lost on the standard path. Qwen's removed temperature/top-p defaults are intentional upstream changes; explicit agent settings still precede transform defaults in request preparation. Kilo gateway options, Ling defaults, Gemini schema sanitation, cache breakpoints and reasoning-summary hooks remain at their existing call sites; transform tests executed.
|
||||
- Completed credential writer/reader tracing: `ProviderConnectDialog.tsx:413–441` constructs Bedrock/Vertex metadata → `provider-actions.ts:295–307` preserves string metadata in `auth.set` → `ControlHttpApi.authSet` → `Auth.set` stores it → `cloud-auth.ts` discriminates structured credentials → provider loader supplies Bedrock `credentialProvider` or Vertex `googleAuthOptions`/OAuth fetch. `providerKey` suppresses accidental treatment of access-key IDs/service-account JSON as bearer tokens. Azure endpoint/resource precedence reaches the SDK constructor without retaining both options; Snowflake's missing-credential message reaches the custom model-loader error. Kilo/OpenRouter/Cerebras/Nvidia/Vercel/Zenmux branded headers reach `resolveSDK` and the selected SDK's HTTP fetch; the Cerebras post-hook intentionally overwrites the inline label with `kilo`.
|
||||
- Completed catalog-field tracing: gateway model parser → `ModelsDev.get`/model cache → model/config patches → `Provider.Model`/`toPublicInfo` → generated model type and extension mirror. `recommendedIndex` feeds TUI/model-picker ordering, `prompt` feeds `session/system.ts`, `isFree` feeds disclosure/export eligibility, `mayTrainOnYourPrompts` feeds filtering/privacy, `hasUserByokAvailable` feeds BYOK disclosure, `terminalBench` feeds model-info/sidebar panels, `autoRouting.models` feeds the extension's `autoChoices`, and `ai_sdk_provider` chooses the native Kilo constructor. Provider `description` and `metadata.noteKey/icon/priority` reach provider display/catalog consumers; `modelsEmpty` reaches prompt/CLI errors. No field was dismissed as connected merely because it appeared in a schema.
|
||||
- Catalog refresh has a concrete invalidation consumer: core `ModelsDev.refresh` → `ModelsRefresh.notify` → registered listener → `ScopedCache.invalidateAll(state.cache)`; its finalizer removes the listener. Kilo small-model ID priority and fallback reach title generation, branch/commit-message generation and prompt enhancement. The native Kilo option-lowering exception requiring human verification is called out precisely below.
|
||||
|
||||
### Codex token refresh → residency → HTTP/WebSocket → retry/cleanup
|
||||
|
||||
- Current auth is read before routing; coordinated Kilo refresh settles before residency extraction (`codex.ts:486–539`). The new claim is derived from the refreshed access token and is only added on the rewritten Codex endpoint. Auth-store schema changes are unnecessary: the access token was already persisted, and this is a derived header rather than a new durable field.
|
||||
- `ws.ts` forwards close code to the pool; code 1009 activates per-session HTTP fallback immediately. `ws-pool.ts:146` still consumes the discarded failed stream, so the newly fast fallback does not remove Kilo's rejection cleanup. Abort/reset, connection reuse, ordinary stream retry and session-deletion/dispose paths remain connected and have existing implementation test coverage.
|
||||
- The residency/refresh/WebSocket tests were run with synthetic credentials/local boundaries. No enterprise account or provider-side residency enforcement was tested.
|
||||
|
||||
### Provider errors → normalized stream → retry → persisted terminal state
|
||||
|
||||
- `rawFinishReason=network_error` now fails the adapter with `ResponseStreamError`, rather than emitting a successful finish step. `MessageV2.fromError` recognizes it as retryable; processor retry policy consumes it. Adapter state is recreated for each `LLM.stream` attempt, preventing the skipped finish/reset branch from leaking counters into the next request.
|
||||
- Kilo's error-frame normalization still unwraps `response.failed`, wrapped and bare error records before the shared parser. New capacity/try-again prose reaches fallback retryability. Existing login/billing-action exclusions, explicit retry limits, offline handling and cancellation remain connected; provider-error, retry, processor and compaction suites exercised these paths.
|
||||
- Successful steps still pass raw billing usage, routed model ID and response headers through `LLMAISDK` → processor → `Session.getUsage`/step-finish storage. Provider-reported Kilo/OpenRouter cost wins before estimated pricing. Finite pricing guards do not replace that precedence. Cost/routed-model/response-metadata tests pass.
|
||||
- No claim is made that every provider's arbitrary prose or all retry-after/cancellation timing combinations were independently covered.
|
||||
|
||||
### Subagent creation/resume → permissions → foreground/background settlement
|
||||
|
||||
- Task validation still rejects primary agents and cross-parent resumption. Parent session restrictions and Kilo read-only/MCP ceilings enter child permissions; current restrictions refresh on resume. Sandbox inheritance and platform registration occur before `session.created` publication, with explicit reconstruction on resume. Model/variant/workflow selection reaches child prompts and tool metadata. Registry visibility is not mistaken for enforcement: tool execution retains sandbox wrappers and inherited session policy.
|
||||
- The new terminal child-tool-error check is inside the same `runTask` and process finalizer as assistant errors. It reaches foreground failure via `BackgroundJob.wait`, or background notification via `renderOutput` with the resumable task ID. Cost propagation still brackets each invocation and covers success/error/cancellation; cancellation does not turn into a successful task result. Existing task/nesting/model/sandbox tests execute these implementations.
|
||||
- Actual extracted `run.ts` event loops prove the new `session.created` chain approves a newly observed child without waiting for Task metadata, preserves the root-Task metadata fallback for resumed children, and leaves unrelated ordinary permission requests untouched.
|
||||
- **Pre-existing Kilo follow-ups, not merge findings:** (1) `run.ts:932` rejects `skillShell`/`sandboxEscalation` asks before session filtering; an unrelated session's special ask can be rejected by an attached run. (2) metadata-tracked resumed children are not added to the new `sessions` set, and `KiloRunAuto.track` accepts only root Task parts, so a newly created grandchild beneath a resumed child remains untracked. Both behaviors were reproduced against HEAD, actual base and supplied main. They are excluded from this merge verdict; whole-process user-visible hang/interference reproductions were not performed.
|
||||
|
||||
Exact event-loop output (controlled SDK reply boundary; all logic came from the actual loop):
|
||||
|
||||
```text
|
||||
{"ref":"HEAD","replies":[{"requestID":"new","reply":"once"},{"requestID":"other-skill","reply":"reject"},{"requestID":"resumed","reply":"once"}]}
|
||||
{"ref":"bf1cf502a3c511e9daf6a43244568ae4e83473a8","replies":[{"requestID":"other-skill","reply":"reject"},{"requestID":"resumed","reply":"once"}]}
|
||||
{"ref":"62998965e9fb0d9ed89011c62498b39801dbbb4f","replies":[{"requestID":"other-skill","reply":"reject"},{"requestID":"resumed","reply":"once"}]}
|
||||
```
|
||||
|
||||
### Core session/context persistence → headers/compaction → reader
|
||||
|
||||
- New core session headers enter `LLM.request.http`, survive route-default merging at `packages/llm/src/route/client.ts:178`, and become transport headers at `route/transport/http.ts:102`. Compaction now copies `input.request.http`; core runner tests assert parent/session affinity preservation.
|
||||
- The projector's new epoch markers annotate retained behavior, not newly introduced resets. Move/revert still reset the compatibility epoch; runner initialization/preparation still supplies expected location. Stored-message normalization/encoding, nullable legacy sequence exclusion and legacy promotion replay hooks remain registered. Core projector, compaction, runner and released-writer compatibility tests pass.
|
||||
- Compaction still writes the compatibility `include` field alongside `recent`. This PR does not alter schema/migration files. The tests are synthetic released-writer controls, not an actual installed-client old → new → old UI exercise.
|
||||
- Ripgrep's surrogate-safe truncation occurs before `Match.make`; Kilo context selection, truncation/partial flags and match decoration survive the mapping. Spawn-bound validation remains attached after preparation, and bounded cancellation/settlement helpers remain connected. Real ripgrep, target replacement and settlement tests pass.
|
||||
|
||||
### HTTP/source schema → generated SDK → TUI/extension consumers
|
||||
|
||||
- Provider handler is composed in production `httpapi/server.ts:186`; `Auth.node` and Kilo model-cache dependencies are already present at `:240`/`:250`. New persisted-credential connected status is not a missing service-layer dependency. Disabled/enabled filters and prompt-training filtering precede valid-provider selection; `failed` remains separate.
|
||||
- Provider list → `fetchProviderData` → `KiloProvider`'s `providersLoaded` message → webview provider context (`context/provider.tsx:58–63`) carries `connected` unchanged. The field is still `string[]`; no new mirrored field is required. The new auth read does not serialize the credential map. Tests cover route responses, runtime fetch-option removal, failed-cache recovery and SDK errors.
|
||||
- Generated required-argument corrections, simplified unknown index signatures, SSE generic changes, SDK property registration and Kilo endpoints were inspected. No route addition/removal is hidden in this regeneration. SDK HTTP/SSE tests exercise representative current routes. The specific default-error-options regression is isolated above; a passing per-call error suite does not cover it.
|
||||
- Encrypted reasoning metadata survives `reasoning-end` → processor part metadata/end time → storage/SSE → TUI `ReasoningPart` → `ReasoningHeader.encrypted`. Blank opaque parts now render without an expansion affordance, and the Kilo `partID` → routed-model badge connection remains. Text reasoning and completed duration use the existing fields. No visual/manual TUI test was performed.
|
||||
- Websearch registration still routes through Kilo tool visibility, permission checks and sandbox-aware HTTP; adding `opencode-go` to availability does not bypass these execution guards. Existing Kilo-auth/Exa transport tests pass.
|
||||
- The changed `customize-opencode` Markdown remains unregistered in the core builtin registry (`plugin/internal.ts:112`), so its new global `.jsonc` sentence is not injected by Kilo's builtin skill registration. Kilo's `kilo-config` path remains separate. No new literal model-facing prompt was introduced by the inspected changed production code; changes to sampling, advertised websearch availability, error tool results and encrypted reasoning display are behavioral changes, not prompt-file edits.
|
||||
- The four removed marker-bearing lines are delimiter/placement changes in the provider handler and the rewritten websearch availability expression. Their surrounding Kilo model filtering, metadata, failed-provider response and `ProviderV2.ID.kilo` branch remain present. No removal of those Kilo behaviors was found.
|
||||
|
||||
### Remaining historical session, registry and TUI chains — static completion
|
||||
|
||||
- **Session rows, listing and plans:** shared Revert brands round-trip through `toRow`/`fromRow`; `workspace` state comes from revert processing and remains in the public result. Lightweight summary diffs persist through the projector/row fields rather than being confused with full patch payloads. `listByProject` → `KiloSession.filters` → SQL and `listGlobal` → worktree-family filtering → experimental handler's `worktreeName` enrichment → generated type → TUI session picker/extension session-mention reader were followed. `Session.plan`'s `.kilo/plans` value reaches plan prompts, plan-exit file resolution and follow-up handling. Compatibility facades delegate to the same Session service via `AppRuntime`; their LayerNode dependencies were inspected.
|
||||
- **Fork/deletion lifetimes:** fork model-at-cutoff selection feeds the new session; cloned message/part IDs, zeroed historical costs and compaction tail IDs feed stored copies. `prepareForkedPart` → `KiloPartLifecycle` and `remapChildren` → child clone/map → `task_id`, metadata and output rewriting → Task resume validation closes the child-reference loop. `carryForkDiff` writes the fork's cumulative/base diff keys consumed by `SessionSummary` and diff APIs. Removal closes jobs, confinement, attribution, process/terminal state, published session state and export capture in the inspected order; FK-safe publishing only swallows the specific deleted-session FK condition. Turn-open/close wrappers publish through the legacy Bus, consumed by memory lifecycle, TUI notifications and extension attention—not an absent EventV2-only listener.
|
||||
- **Sandbox and Task inheritance:** Agent Manager issues a counted server-side grant; session creation consumes it and hands its source directory/session to policy inheritance before publication. Policy snapshots have writer/readers in the sandbox store/current-profile path; `SessionTools` executes through `SandboxPolicy.executeTool`, and code-mode MCP calls independently use `executeMcp`. The registry's network flag is an availability filter, not the sole security boundary. Parent-lifetime background processes transfer in `stopSession`; ordinary processes terminate and persistent processes use a separate map. The separate keyed-TUI cleanup defect is not hidden by this valid Task-finalizer chain.
|
||||
- **All registry additions:** imports → `infos`/`build`/`extra` → builtin definitions → per-request visibility → JSON schema lowering → tool execution → processor output were followed for recall/memory, background/interactive processes, chart/image, notification/file delivery, notebook, Agent Manager and scout tools. Optional notebook/host availability and client/experimental flags have corresponding gates. Notebook and Agent Manager requests retain operation/request/session IDs through Bus/SSE → extension bridges → SDK reply/reject endpoints → matching deferred; timeout/cancel/disposal settles the wait. Notification goes through `KiloSessions.sendAgentNotification`; image generation uses the provided HTTP client; file-delivery attachments preserve their result metadata. Memory visibility's root-key cache is invalidated by bootstrap's memory status/updated subscriptions. Plugin discovery calls only recognized server exports, preserving external plugin compatibility while skipping named constants.
|
||||
- **TUI event arbitration and feedback:** question/suggestion/network/terminal producers → event schemas/Bus/SSE → sync maps → session-group selection → prompt component → SDK accept/reject/write/resize/close → server handler were traced. Non-blocking suggestion dispatch is the explicit broken exception above. Permission provenance is produced by `SessionTools.ask`, preserved by `processor.metadata`/completion, and consumed by `stateMetadata` → `describeApproval` → inline/block badges; todo's suppression flag reaches the block badge. Memory error sink → Bus → `MemoryTuiEvents` → toast is wired; the `app.tsx:1152` keyed boundary remounts its captured session filter and cleanup, so a stale-session memory-listener hypothesis was rejected. Feedback command IDs reach registered keybindings and `submitFeedback`, which checks telemetry consent and sends the selected assistant's permitted fields to `Telemetry.trackFeedback`.
|
||||
- **TUI text/status/export:** background status/count, interactive `closedBy`/exit code and semantic-search result arrays are produced by their tools, preserved in tool metadata and read by the dedicated renderer selected by `toolDisplays`. Task initialization/Starting text derives from real child session/tool state. Question dismissal metadata survives into the compact/expanded display. Edit/apply-patch producers provide `diff`/`files[].patch`; `splitDiffHunks` preserves file/hunk headers for the actual diff widget. Markdown-table formatting feeds rendered text only. Copy/export fetch the no-limit messages endpoint (`handlers/session.ts:133`), then `formatTranscript` and clipboard/editor/file output; they do not use the truncated UI hydration store. Routed-model context/part ID reaches both reasoning and block headers and the step/footer fallback.
|
||||
- **Every marked test region:** the 12 files' marked setup, input, expectation and teardown regions were read with their imported implementation call sites. Tests terminate in real transform/adapter/SDK/plugin methods or Session/Task/processor/projector state, with external LLM/host/snapshot boundaries substituted where declared. Cost tests persist child assistant costs before executing the real propagation path; terminal errors inspect `Cause`; output-cap tests inject `RuntimeFlags`; plugin fixture paths run through real plugin discovery. No-op snapshot replacements bound only the cancellation fixtures. The compaction ready-timeout catch means that one test can interrupt before its intended plugin boundary, so its passing result is not evidence of that exact timing point. Existing skips and the assertion-free SDK scenario remain explicitly weaker evidence. This is static test-chain coverage, not a new execution claim.
|
||||
|
||||
### Exact remaining human-verification item
|
||||
|
||||
**HV-1 — Native Anthropic option semantics through Kilo, pre-existing/activation-dependent; not an established merge bug.** Marker `packages/opencode/src/provider/transform.ts:1705–1709` delegates to `kilocode/provider-options.ts:6–28`. The source graph is resolved through Kilo model `ai_sdk_provider` → `kiloCustomLoaders` → `createKilo().anthropic` → the native Anthropic SDK. However, the adapter sets `anthropic.effort` from `openrouter.verbosity`, not `openrouter.reasoning.effort`, and selects adaptive thinking only from `reasoning.enabled`. A variant shaped only as `{ reasoning: { effort: "high" } }` therefore lacks native effort and disables thinking in this adapter. The helper is unchanged from actual base/main. What is missing is a captured, supported **native-Anthropic Kilo catalog/config variant contract plus outgoing request** proving this shape is activated in a shipped selection; the generic/OpenRouter path is not evidence of native activation. Verify that exact combination before treating it as an established product bug or changing the mapping. This is not an uninspected source branch or a new PR finding.
|
||||
|
||||
## Commands and results
|
||||
|
||||
Every tool command explicitly used the isolated checkout as `workdir`. Package selection was supplied with `bun test --cwd ...`; no root `bun test`, install, branch change, source edit, commit, push or GitHub mutation was performed. Existing test preloads allocate disposable XDG/home/config state and in-memory databases, but **that is not proof of hermetic config reads**. The parent reports primary-checkout config read leakage in the separate config-review run; this lens did not record every filesystem/config read and makes no claim that all its earlier package tests were isolated from ancestor/primary-checkout configuration. The later inline source controls use synthetic arguments/in-memory state and do not initialize project Config services. Some existing provider tests perform network catalog discovery, visible in their logs; these are not live inference tests.
|
||||
|
||||
**Runtime limitation:** commands used **Bun 1.3.14**, while root `package.json:7` pins **`bun@1.4.0`**. No Bun installation or pinned-runtime rerun was performed. In particular, the rejection-handling marker at `provider/provider.ts:67` explicitly concerns Bun 1.4; earlier test passes cannot certify that runtime-specific cancellation behavior. The static source conclusions do not depend on claiming a pinned-runtime test pass.
|
||||
|
||||
The following are **verbatim terminal result summaries**, with noisy timestamped discovery logs omitted, not claims that every stdout line is reproduced.
|
||||
|
||||
### Focused implementation suites
|
||||
|
||||
```sh
|
||||
bun test --cwd packages/opencode ./test/plugin/cerebras.test.ts ./test/plugin/cloudflare.test.ts ./test/plugin/codex.test.ts ./test/plugin/openai-ws.test.ts ./test/provider/cf-ai-gateway-e2e.test.ts ./test/provider/error.test.ts ./test/kilocode/provider/error.test.ts ./test/session/retry.test.ts ./test/kilocode/run-auto.test.ts ./test/tool/task.test.ts
|
||||
```
|
||||
|
||||
```text
|
||||
185 pass
|
||||
0 fail
|
||||
470 expect() calls
|
||||
Ran 185 tests across 10 files. [20.47s]
|
||||
```
|
||||
|
||||
```sh
|
||||
bun test --cwd packages/opencode ./test/session/llm.test.ts ./test/session/processor-effect.test.ts ./test/kilocode/session-processor-retry-limit.test.ts ./test/kilocode/task-nesting.test.ts ./test/kilocode/tool-task-model.test.ts ./test/kilocode/provider-list-failed-state.test.ts ./test/server/httpapi-provider.test.ts ./test/server/sdk-error-shape.test.ts
|
||||
```
|
||||
|
||||
```text
|
||||
96 pass
|
||||
1 skip
|
||||
0 fail
|
||||
331 expect() calls
|
||||
Ran 97 tests across 8 files. [44.49s]
|
||||
```
|
||||
|
||||
The skip is `returns public v2 provider not found errors` at `test/server/httpapi-provider.test.ts:264`.
|
||||
|
||||
```sh
|
||||
bun test --cwd packages/core ./test/ripgrep.test.ts ./test/session-runner.test.ts
|
||||
```
|
||||
|
||||
```text
|
||||
92 pass
|
||||
0 fail
|
||||
277 expect() calls
|
||||
Ran 92 tests across 2 files. [1.82s]
|
||||
```
|
||||
|
||||
```sh
|
||||
bun test --cwd packages/opencode ./test/provider/transform.test.ts ./test/provider/provider.test.ts ./test/session/compaction.test.ts ./test/control-plane/workspace.test.ts ./test/kilocode/provider-cost.test.ts ./test/kilocode/session-routed-model.test.ts ./test/kilocode/session-response-metadata.test.ts ./test/kilocode/provider/first-byte.test.ts ./test/kilocode/codex-refresh.test.ts ./test/kilocode/sandbox/session.test.ts ./test/kilocode/permission/next.reply-routing.test.ts ./test/kilocode/tool/websearch-kilo-exa.test.ts
|
||||
```
|
||||
|
||||
```text
|
||||
(fail) provider loaded from env variable [5075.52ms]
|
||||
^ this test timed out after 5000ms.
|
||||
|
||||
685 pass
|
||||
1 skip
|
||||
1 fail
|
||||
1483 expect() calls
|
||||
Ran 687 tests across 11 files. [85.89s]
|
||||
```
|
||||
|
||||
This command named 12 paths but executed 11 files: `test/kilocode/codex-refresh.test.ts` does not exist. Correct Codex paths were subsequently run below. The skip is `projects a compaction message to v2 (v2 projector disabled)` at `test/session/compaction.test.ts:619`. A teardown log also reported `failed to kill process group` / `EPERM` for a test background-process group; this was not a test assertion failure or a proved merge regression.
|
||||
|
||||
```sh
|
||||
bun test --cwd packages/opencode ./test/provider/provider.test.ts -t '^provider loaded from env variable$'
|
||||
```
|
||||
|
||||
```text
|
||||
1 pass
|
||||
99 filtered out
|
||||
0 fail
|
||||
3 expect() calls
|
||||
Ran 1 test across 1 file. [2.16s]
|
||||
```
|
||||
|
||||
```sh
|
||||
bun test --cwd packages/opencode ./test/kilocode/codex-auth-refresh.test.ts ./test/kilocode/codex-refresh-user-agent.test.ts ./test/server/httpapi-sdk.test.ts ./test/kilocode/provider/first-byte.test.ts
|
||||
```
|
||||
|
||||
```text
|
||||
(fail) HttpApi SDK > matches generated SDK instance read routes [5007.20ms]
|
||||
^ this test timed out after 5000ms.
|
||||
|
||||
36 pass
|
||||
1 fail
|
||||
121 expect() calls
|
||||
Ran 37 tests across 4 files. [21.85s]
|
||||
```
|
||||
|
||||
```sh
|
||||
bun test --cwd packages/opencode ./test/server/httpapi-sdk.test.ts -t 'matches generated SDK instance read routes'
|
||||
```
|
||||
|
||||
```text
|
||||
1 pass
|
||||
20 filtered out
|
||||
0 fail
|
||||
Ran 1 test across 1 file. [4.36s]
|
||||
```
|
||||
|
||||
Both timeout reruns used the original deadlines, without source edits or timeout inflation. They are classified as non-reproducing timing/network-sensitive failures, not established merge regressions. No full base-suite run was performed. **The filtered SDK route test has no result assertions:** `serverPathParity` at `httpapi-sdk.test.ts:230` simply runs the scenario, and the scenario returns captured values without comparing them. Its passing rerun proves request execution/settlement only, not correct status/content. Other tests in that file, the provider route tests and the inline SDK controls do execute assertions; their evidence must not be conflated with this weaker scenario.
|
||||
|
||||
```sh
|
||||
bun test --cwd packages/core ./test/session-projector.test.ts ./test/session-compaction.test.ts ./test/kilocode/database-migration-compat.test.ts ./test/kilocode/search-target.test.ts ./test/kilocode/ripgrep-settlement.test.ts
|
||||
```
|
||||
|
||||
```text
|
||||
21 pass
|
||||
0 fail
|
||||
68 expect() calls
|
||||
Ran 21 tests across 5 files. [2.31s]
|
||||
```
|
||||
|
||||
### Static checks
|
||||
|
||||
```sh
|
||||
bun run --cwd packages/opencode typecheck
|
||||
bun run --cwd packages/sdk/js typecheck
|
||||
```
|
||||
|
||||
Each exited 0 with:
|
||||
|
||||
```text
|
||||
$ tsgo --noEmit
|
||||
```
|
||||
|
||||
```sh
|
||||
bun run lint -- packages/opencode/src/provider packages/opencode/src/plugin packages/opencode/src/session/retry.ts packages/opencode/src/session/session.ts packages/opencode/src/session/llm/ai-sdk.ts packages/opencode/src/tool/task.ts packages/opencode/src/tool/registry.ts packages/opencode/src/cli/cmd/run.ts packages/opencode/src/control-plane/workspace.ts packages/opencode/src/server/routes/instance/httpapi/handlers/provider.ts packages/core/src/session packages/core/src/ripgrep.ts packages/sdk/js/src/v2 packages/tui/src/routes/session/index.tsx
|
||||
```
|
||||
|
||||
Exited 0:
|
||||
|
||||
```text
|
||||
Found 582 warnings and 0 errors.
|
||||
Finished in 4.0s on 81 files with 130 rules using 18 threads.
|
||||
```
|
||||
|
||||
Warnings were not promoted to semantic review findings; no clean-warning claim is made.
|
||||
|
||||
### Reproducible in-memory controls
|
||||
|
||||
SDK historical control command (real generated transport and real Kilo interceptor, current helper dependencies held fixed):
|
||||
|
||||
```sh
|
||||
bun -e 'import assert from "node:assert/strict"; import * as utils from "./packages/sdk/js/src/v2/gen/client/utils.gen.ts"; import * as sse from "./packages/sdk/js/src/v2/gen/core/serverSentEvents.gen.ts"; import * as body from "./packages/sdk/js/src/v2/gen/core/utils.gen.ts"; import {wrapClientError} from "./packages/sdk/js/src/error-interceptor.ts"; const deps={...utils,...sse,...body}; for (const ref of ["HEAD","bf1cf502a3c511e9daf6a43244568ae4e83473a8","62998965e9fb0d9ed89011c62498b39801dbbb4f","7248bc1964b13fa67e601733f89ee9dc6dfa0563"]) { const out=Bun.spawnSync(["git","show",`${ref}:packages/sdk/js/src/v2/gen/client/client.gen.ts`],{cwd:process.cwd()}); assert.equal(out.exitCode,0); const js=new Bun.Transpiler({loader:"ts"}).transformSync(out.stdout.toString()).replace(/import\s[\s\S]*?from\s"[^"]+";?/g, "").replace("export const createClient", "const createClient"); const createClient=new Function(...Object.keys(deps),`${js};return createClient;`)(...Object.values(deps)); const client=createClient({baseUrl:"http://review.invalid",throwOnError:true,fetch:async()=>Response.json({name:"NotFoundError",data:{message:"session missing"}},{status:404})}); client.interceptors.error.use(wrapClientError); const err=await client.get({url:"/session/ses_missing"}).catch(e=>e); console.log(JSON.stringify({ref,errorInstance:err instanceof Error,message:err.message??null})); assert.equal(err instanceof Error,ref==="bf1cf502a3c511e9daf6a43244568ae4e83473a8" || ref==="7248bc1964b13fa67e601733f89ee9dc6dfa0563"); }'
|
||||
```
|
||||
|
||||
Workspace historical control command (real extracted logic/SQL; controlled adapter and sync boundary, minimal in-memory table):
|
||||
|
||||
```sh
|
||||
bun -e 'import assert from "node:assert/strict"; const dir=`${process.cwd()}/packages/core`; const {Effect}=await import(Bun.resolveSync("effect",dir)); const {SqliteClient}=await import(Bun.resolveSync("@effect/sql-sqlite-bun",dir)); const {EffectDrizzleSqlite}=await import("./packages/effect-drizzle-sqlite/src/index.ts"); const {eq}=await import(Bun.resolveSync("drizzle-orm",dir)); const {sqliteTable,text,integer}=await import(Bun.resolveSync("drizzle-orm/sqlite-core",dir)); const WorkspaceTable=sqliteTable("workspace",{id:text().primaryKey(),type:text(),name:text(),branch:text(),directory:text(),extra:text({mode:"json"}),project_id:text(),time_used:integer()}); for(const ref of ["HEAD","bf1cf502a3c511e9daf6a43244568ae4e83473a8","62998965e9fb0d9ed89011c62498b39801dbbb4f","7248bc1964b13fa67e601733f89ee9dc6dfa0563"]){const out=Bun.spawnSync(["git","show",`${ref}:packages/opencode/src/control-plane/workspace.ts`],{cwd:process.cwd()});assert.equal(out.exitCode,0); const source=out.stdout.toString(); const segment=source.slice(source.indexOf(" const list = Effect.fn(\"Workspace.list\")"),source.indexOf(" const get = Effect.fn(\"Workspace.get\")")); const row=source.slice(source.indexOf("function fromRow("),source.indexOf("export const CreateInput")); const js=new Bun.Transpiler({loader:"ts"}).transformSync(row+segment); for(const enabled of [false,true]) await Effect.runPromise(Effect.gen(function*(){const db=yield* EffectDrizzleSqlite.makeWithDefaults();yield* db.run("CREATE TABLE workspace (id TEXT PRIMARY KEY,type TEXT,name TEXT,branch TEXT,directory TEXT,extra TEXT,project_id TEXT,time_used INTEGER)");yield* db.insert(WorkspaceTable).values({id:"wrk_existing",type:"test",name:"same",project_id:"project",time_used:0}).run();const starts=[];let seq=0;const deps={Effect,eq,WorkspaceTable,db,flags:{experimentalWorkspaces:enabled},registeredAdapters:()=>[["test",{}]],WorkspaceAdapterRuntime:{list:()=>Effect.succeed([{name:"same",type:"test",projectID:"project",directory:"/fixture",branch:null,extra:null}])},WorkspaceV2:{ID:{ascending:()=>`wrk_new_${++seq}`}},startSync:(info)=>Effect.sync(()=>{starts.push(info.id)})};const api=new Function(...Object.keys(deps),`${js};return {list,syncList};`)(...Object.values(deps));yield* api.syncList({id:"project"});yield* api.syncList({id:"project"});const rows=yield* db.select().from(WorkspaceTable).all();const visible=yield* api.list({id:"project"});const changed=!enabled&&(ref==="HEAD"||ref.startsWith("7248"));assert.equal(rows.length,changed?3:1);assert.equal(starts.length,changed?2:0);console.log(JSON.stringify({ref,enabled,rows:rows.length,attemptedSync:starts.length,visible:visible.length}));}).pipe(Effect.provide(SqliteClient.layer({filename:":memory:",disableWAL:true})),Effect.scoped));}'
|
||||
```
|
||||
|
||||
Cloudflare production-loader capture command:
|
||||
|
||||
```sh
|
||||
bun -e 'import assert from "node:assert/strict"; import os from "node:os"; const dir=`${process.cwd()}/packages/opencode`; const {Effect}=await import(Bun.resolveSync("effect",dir)); const real=globalThis.fetch; try { for(const ref of ["HEAD","bf1cf502a3c511e9daf6a43244568ae4e83473a8","7248bc1964b13fa67e601733f89ee9dc6dfa0563"]){ const out=Bun.spawnSync(["git","show",`${ref}:packages/opencode/src/provider/provider.ts`],{cwd:process.cwd()});assert.equal(out.exitCode,0); const source=out.stdout.toString();const start=source.indexOf(" \"cloudflare-ai-gateway\": Effect.fnUntraced");const end=source.indexOf(" cerebras:",start);assert.ok(start>=0&&end>start);const code=`const loaders={${source.slice(start,end)}};return loaders["cloudflare-ai-gateway"];`;const js=new Bun.Transpiler({loader:"ts"}).transformSync(code).replace(/import\("(ai-gateway-provider[^\"]*)"\)/g,(_,name)=>`import(${JSON.stringify(Bun.resolveSync(name,dir))})`);const loader=new Function("Effect","dep","iife","InstallationVersion","os",js)(Effect,{auth:()=>Effect.succeed({type:"api",key:"cf-test-secret",metadata:{accountId:"account",gatewayId:"gateway"}}),env:()=>Effect.succeed({})},fn=>fn(),"review",os);const item=await Effect.runPromise(loader({id:"cloudflare-ai-gateway",options:{metadata:{chain:"review"}}}));for(const id of ["openai/gpt-5.4","anthropic/claude-sonnet-4-6","workers-ai/@cf/test","@cf/test"]){let capture;globalThis.fetch=async(input,init)=>{capture={url:String(input),headers:Object.fromEntries(new Headers(init.headers)),body:JSON.parse(init.body)};throw new Error("STOP_AFTER_CAPTURE")};const model=await item.getModel({},id,{});await model.doGenerate({prompt:[{role:"user",content:[{type:"text",text:"hi"}]}],maxOutputTokens:64}).catch(()=>undefined);assert.ok(capture);assert.equal(capture.headers["cf-aig-authorization"],"Bearer cf-test-secret");const step=capture.body[0];const workers=id.startsWith("workers-ai/")||id.startsWith("@cf/");assert.equal(JSON.stringify(step).includes("cf-test-secret"),workers||ref.startsWith("bf1"));assert.equal(step.query.model,ref.startsWith("bf1")||workers?id:id.slice(id.indexOf("/")+1));console.log(JSON.stringify({ref,id,provider:step.provider,endpoint:step.endpoint,secretInUpstream:JSON.stringify(step).includes("cf-test-secret"),outerAuthCorrect:true}));}}}finally{globalThis.fetch=real}'
|
||||
```
|
||||
|
||||
Verbatim HEAD output; base/pristine behavior is summarized above:
|
||||
|
||||
```text
|
||||
{"ref":"HEAD","id":"openai/gpt-5.4","provider":"openai","endpoint":"v1/responses","secretInUpstream":false,"outerAuthCorrect":true}
|
||||
{"ref":"HEAD","id":"anthropic/claude-sonnet-4-6","provider":"anthropic","endpoint":"v1/messages","secretInUpstream":false,"outerAuthCorrect":true}
|
||||
{"ref":"HEAD","id":"workers-ai/@cf/test","provider":"compat","endpoint":"chat/completions","secretInUpstream":true,"outerAuthCorrect":true}
|
||||
{"ref":"HEAD","id":"@cf/test","provider":"compat","endpoint":"chat/completions","secretInUpstream":true,"outerAuthCorrect":true}
|
||||
```
|
||||
|
||||
This executed the exact production custom-loader block with installed `ai-gateway-provider` 3.2.0 held fixed, synthetic auth/env and a fetch boundary that captured then deliberately threw `STOP_AFTER_CAPTURE`. Assertions verified model-ID stripping and token scoping; the output records the selected endpoints. It did not create diagnostic files or contact Cloudflare. Thus it proves request construction, not successful live response parsing or billing.
|
||||
|
||||
### Static-follow-up control commands
|
||||
|
||||
Inventory command rechecked the denominator using `git diff --name-only <actual-base> HEAD`, then counted literal marker-bearing lines in each `git show HEAD:<path>` result. Output:
|
||||
|
||||
```text
|
||||
{"files":29,"productionFiles":17,"testFiles":12,"markerLines":737}
|
||||
```
|
||||
|
||||
Suggestion dispatcher/history control (outputs are in the pre-existing finding):
|
||||
|
||||
```sh
|
||||
bun -e 'import assert from "node:assert/strict"; for(const ref of ["HEAD","bf1cf502a3c511e9daf6a43244568ae4e83473a8","62998965e9fb0d9ed89011c62498b39801dbbb4f"]){const out=Bun.spawnSync(["git","show",`${ref}:packages/tui/src/routes/session/index.tsx`],{cwd:process.cwd()});assert.equal(out.exitCode,0);const src=out.stdout.toString();const start=src.indexOf("const toolDisplays = new Set(");const end=src.indexOf("function recordValue(",start);const js=new Bun.Transpiler({loader:"ts"}).transformSync(src.slice(start,end)).replace("export function toolDisplay","function toolDisplay");const display=new Function(`${js};return toolDisplay;`)();const refs=[...src.matchAll(/\bSuggest\b/g)].length;assert.equal(display("suggest"),"generic");assert.equal(refs,1);assert.equal(/<Suggest\b/.test(src),false);console.log(JSON.stringify({ref,suggestDisplay:display("suggest"),SuggestReferences:refs,renderedSuggest:false}));}'
|
||||
```
|
||||
|
||||
Keyed lifecycle control using real Solid, exact historical cleanup source and a recorded SDK boundary (outputs are in the pre-existing finding):
|
||||
|
||||
```sh
|
||||
bun -e 'import assert from "node:assert/strict"; const solid=await import(Bun.resolveSync("solid-js/dist/solid.js",`${process.cwd()}/packages/opencode`));for(const ref of ["HEAD","bf1cf502a3c511e9daf6a43244568ae4e83473a8","62998965e9fb0d9ed89011c62498b39801dbbb4f"]){const read=file=>{const out=Bun.spawnSync(["git","show",`${ref}:${file}`],{cwd:process.cwd()});assert.equal(out.exitCode,0);return out.stdout.toString()};const source=read("packages/tui/src/routes/session/index.tsx");const start=source.indexOf(" function processGroup(");const end=source.indexOf(" // kilocode_change end",start);const js=new Bun.Transpiler({loader:"ts"}).transformSync(source.slice(start,end));assert.match(read("packages/tui/src/app.tsx"),/route.data.sessionID : undefined} keyed/);for(const keyed of [false,true]){const calls=[];const entries=[{id:"parent"},{id:"child",parentID:"parent"}];let set,dispose;solid.createRoot(d=>{dispose=d;const [id,write]=solid.createSignal("parent");set=write;const deps={createEffect:solid.createEffect,onCleanup:solid.onCleanup,route:{get sessionID(){return id()}},sync:{session:{get:id=>entries.find(e=>e.id===id)},data:{session:entries}},project:{workspace:{current:()=>undefined}},sdk:{client:{backgroundProcess:{stopSession:async arg=>{calls.push(arg.sessionID)}}}}};const mount=new Function(...Object.keys(deps),js);const view=solid.createComponent(solid.Show,{get when(){return id()},keyed,children:(_)=>{mount(...Object.values(deps));return "mounted"}});solid.createRenderEffect(()=>view());});assert.deepEqual(calls,[]);set("child");await Promise.resolve();assert.deepEqual([...new Set(calls)].sort(),keyed?["child","parent"]:[]);console.log(JSON.stringify({ref,keyed,navigation:"parent -> child",stopRequests:calls.slice()}));dispose();}}'
|
||||
```
|
||||
|
||||
No package suites were rerun during this static follow-up. These inline controls do not spawn real background processes, initialize project config or write diagnostic source files.
|
||||
|
||||
Harness attempts that did not count as passing tests:
|
||||
|
||||
- Initial `bun --cwd packages/opencode test ...` selected the package test script and printed `No test files found`; corrected to `bun test --cwd packages/opencode ...`, with non-zero tests/assertions shown above.
|
||||
- Initial historical SDK `data:` module import failed with `NameTooLong`; replaced by in-memory `Function` evaluation of transpiled source.
|
||||
- Initial workspace inline imports failed with `Cannot find package 'effect'` / `Cannot find module './packages/core/node_modules/effect'`; corrected using `Bun.resolveSync` from the package directory. No installation was attempted.
|
||||
- The first Solid cleanup harness used a zero-argument `Show` child function, which Solid does not invoke as the render callback; it returned no stop requests and failed its assertion. The corrected control uses the actual app's one-argument callback shape, asserts the keyed boundary is present in each historical app source, and includes an unkeyed negative control. Only the corrected results support the finding.
|
||||
|
||||
### First-pass remote and checkout checks (parent owns final metadata)
|
||||
|
||||
```sh
|
||||
gh pr view 13513 --repo Kilo-Org/kilocode --json headRefOid,baseRefName,baseRefOid,mergeable,mergeStateStatus
|
||||
```
|
||||
|
||||
```text
|
||||
{"baseRefName":"johnnyeric/kilo-opencode-v1.18.18","baseRefOid":"bf1cf502a3c511e9daf6a43244568ae4e83473a8","headRefOid":"6a7d6bc002319ac2987bcde3d6c63efcafc07021","mergeStateStatus":"CLEAN","mergeable":"MERGEABLE"}
|
||||
```
|
||||
|
||||
`gh pr checks 13513 --repo Kilo-Org/kilocode` reported all listed substantive checks passing, including HttpApi exerciser, platform unit jobs, JS/JetBrains typechecks, annotations and visual regressions; `[code]smith` was `skipping`. These checks do not cover the two counterexamples above.
|
||||
|
||||
Tracked working-tree and index diffs were empty before report creation and remained empty after it. Other reviewers' untracked reports appeared during the review and were left untouched. Only this report is authored by this reviewer. Final local HEAD remained `6a7d6bc002319ac2987bcde3d6c63efcafc07021`.
|
||||
|
||||
Report-format check:
|
||||
|
||||
```sh
|
||||
bun run script/check-md-table-padding.ts BROKEN_PIPELINE_CHAINS.md
|
||||
```
|
||||
|
||||
```text
|
||||
check-md-table-padding: 1 file(s) checked, no padded tables found.
|
||||
```
|
||||
|
||||
## Limitations / human verification
|
||||
|
||||
- Static source coverage is complete for the 737 marker-bearing lines/29 changed files within the bounded in-repo definition above. Runtime execution is not exhaustive. The specific unresolved external activation/serialization contract is HV-1 (`provider/transform.ts:1705–1709` → `kilocode/provider-options.ts:19–22`); it is not counted as a verified bug.
|
||||
- Bun 1.3.14 was used instead of pinned 1.4.0. No runtime-specific claim is made for the Bun-1.4 stream-cancellation marker at `provider/provider.ts:67`.
|
||||
- Disposable XDG/database setup does not establish config-read isolation. This lens did not trace every package-test config read; the parent's separately reported primary-checkout leakage must be reconciled by the config lens. Earlier package test results are scoped evidence, not a hermeticity guarantee.
|
||||
- No full-process reproduction of the disabled-workspace duplicate via the HTTP endpoint; static production route composition plus real in-memory service-function/SQL execution establishes the write-side defect. Mixed client/server flag UX should be checked manually when fixing it.
|
||||
- No real Cloudflare/Codex/Cerebras inference, live token-refresh settlement against provider accounts, enterprise residency enforcement, or real billing. Provider catalog discovery occurred in existing tests.
|
||||
- No manual TUI/VS Code/JetBrains launch, visual reasoning test, Windows/Linux execution, full permission/sandbox penetration test, or real released-client round trip. Relevant synthetic compatibility and permission tests ran.
|
||||
- No SDK/OpenAPI regeneration: source/generated writes were prohibited. Routes and generated artifacts were inspected and representative runtime SDK tests ran, but second-generation cleanliness is delegated to the parent/CI.
|
||||
- Historical controls execute selected historical implementations with current dependencies and controlled boundaries, not fully installed historical checkouts. This holds helpers fixed to isolate the observed changes but is not a complete released-environment comparison.
|
||||
- Two broader-suite timeouts passed alone; no base-suite timing control was run. Two explicitly skipped tests remain unverified.
|
||||
- Exact rerere/mergiraf/manual conflict counts were not reconstructed by this lens. Local authoritative upstream refs were verified; no independent remote tag fetch was performed.
|
||||
- No source edits, source diagnostic files, repository config changes, branch changes, commits, pushes or GitHub mutations. No real user database or login credential was intentionally accessed.
|
||||
@@ -0,0 +1,117 @@
|
||||
# Configuration regression review — PR #13513
|
||||
|
||||
## Scope and method
|
||||
|
||||
Reviewer 6/7; configuration-discovery lens only. **Verdict: safe to merge for this lens; no PR-introduced configuration-discovery regression found.** One pre-existing managed-configuration policy question is separated below and does not block this incremental merge.
|
||||
|
||||
Reviewed the exact isolated checkout at `/Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports`. Read root `AGENTS.md`, `REVIEW.md`, CLI/package test instructions, merge-minimization guidance, and upstream merge documentation; loaded `kilo-steer`, `kilo-config`, and merge-review guidance. Traced actual runtime sources rather than classifying every `opencode` string as a configuration read.
|
||||
|
||||
Compared actual base → HEAD, pinned main → HEAD, pristine upstream v1.18.18 → v1.18.20, upstream v1.18.20 → HEAD, and both HEAD parents → HEAD. Checked config selectors, global/project/home/worktree lookup, commands/agents/modes/plugins/skills, instructions, environment flags, TUI migration, the config-source listing, and built-in skill registration. Used real Config/Skill services with disposable files for bounded runtime checks; auth/account/NPM were fixture dependencies, and host MDM preference reads were disabled in a diagnostic preload.
|
||||
|
||||
| Reference | Verified commit |
|
||||
|---|---|
|
||||
| HEAD | `6a7d6bc002319ac2987bcde3d6c63efcafc07021` |
|
||||
| Actual base / merge base | `bf1cf502a3c511e9daf6a43244568ae4e83473a8` |
|
||||
| Pinned main control | `62998965e9fb0d9ed89011c62498b39801dbbb4f` |
|
||||
| Upstream v1.18.18 | `31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d` |
|
||||
| Upstream v1.18.19 | `2b72179c663cadcb54f54d9f19221b3fb3d11fb6` |
|
||||
| Upstream v1.18.20 | `7248bc1964b13fa67e601733f89ee9dc6dfa0563` |
|
||||
| HEAD parent 1 | `91ca95bad927436131ea4783a470885a381ce6ad` |
|
||||
| HEAD parent 2, transformed upstream | `9563af96a012effc25df5a11eaa1f7633161a742` |
|
||||
|
||||
The record merge `91ca95bad9` has parents actual base and pristine v1.18.20. The PR has 95 commits, two first-parent commits, and 59 changed files. Config/skill/instruction loaders did not change from the actual base or HEAD's first parent. Differences against the transformed second parent preserve Kilo's selectors and registration exclusions rather than restoring upstream candidates. Main-relative config warning/persistence changes were already in the actual base and are not changes introduced by this PR.
|
||||
|
||||
## Findings
|
||||
|
||||
No confirmed finding introduced by this upstream range or its Kilo conflict resolution.
|
||||
|
||||
### HV-1 — P2 conditional / human policy verification: existing OpenCode MDM domain remains active
|
||||
|
||||
- **Location:** `packages/opencode/src/config/managed.ts:8`, `packages/opencode/src/config/managed.ts:53`, invoked by `packages/opencode/src/config/config.ts:945`. The source-list mirror is `packages/opencode/src/kilocode/config/sources.ts:258`.
|
||||
- **Verification class:** static reachability verified; real managed-profile behavior deliberately not exercised. Whether the existing compatibility behavior violates the intended product policy requires human verification.
|
||||
- **Provenance:** pre-existing Kilo retention of an upstream behavior, not PR-introduced. `managed.ts` is byte-identical in actual base, HEAD, and pinned main: blob `d52f7657c7820217d7ef60842463165742165737`.
|
||||
- **Invariant/control:** automatic `.opencode` directory fallback is excluded, but that is not currently a blanket prohibition on every OpenCode-named config source. On macOS, the loader still checks `/Library/Managed Preferences/<user>/ai.opencode.managed.plist` and the system equivalent, converts an existing plist with `plutil`, and merges it after other config sources. Pinned main is the same-behavior provenance control; the directory-exclusion runtime tests are the distinct negative control.
|
||||
- **Potential impact:** a machine with an OpenCode-managed profile can have its Kilo settings overridden by that profile. This is conditional on such a profile existing, not an observed host failure or a claimed merge regression.
|
||||
- **Fix direction:** confirm whether enterprise OpenCode-profile compatibility is intentional. If the policy forbids it, move the loader and source-list mirror to an agreed Kilo-owned managed domain in a separate change with disposable-profile tests. Do not treat this PR as having reintroduced the domain.
|
||||
|
||||
## Notable non-findings
|
||||
|
||||
### Directory fallback removal and `.kilo` priority survive
|
||||
|
||||
`packages/opencode/src/config/paths.ts:23` selects only the Kilo XDG root, `.kilocode`/`.kilo` project and home directories, and explicit `KILO_CONFIG_DIR`. It never adds `.opencode`. This file is byte-identical in actual base, HEAD, and pinned main: blob `5517f82a949207005b0e451a1d9370ac62e788e2`.
|
||||
|
||||
`packages/core/src/config.ts:143` and `packages/core/src/config.ts:182` independently retain Kilo directory discovery for the v2 service. `.kilo`/`.kilocode` target order is reversed when applied at `packages/core/src/config.ts:189`, preserving canonical `.kilo` precedence. This entire file is identical across the same three Kilo controls: blob `6c9b06d3e5990f2fd833c14457548b0388fb6b63`.
|
||||
|
||||
The CLI passes those directories to command, agent, mode, and file-plugin loaders at `packages/opencode/src/config/config.ts:827`, `:833`, `:840`, and `:848`. The changed plugin registry still gets external plugin origins from that merged config at `packages/opencode/src/plugin/index.ts:162` and `:191`; its only registration addition is the Cerebras chat-parameter hook, not another discovery root. Bounded runtime checks confirmed `.opencode` contributes no config, agent, command, plugin origin, or skill, while both Kilo directory spellings still work.
|
||||
|
||||
### Primary-worktree fallback remains Kilo-only
|
||||
|
||||
`packages/opencode/src/config/config.ts:766` requests only `[".kilocode", ".kilo"]` from `primaryPaths`, inserts primary results before active-worktree directories, and marks them local. `packages/opencode/src/kilocode/primary-worktree.ts:23` walks the mirrored directory to the primary root using only the supplied names. Skills use the same Kilo names at `packages/opencode/src/skill/index.ts:245`; `.agents` and `.claude` are separate, intentionally supported external skill roots at `:213`. None of these sources changed against actual base or pinned main. Real linked-worktree fixture tests were not run because they create commits and edit fixture Git configuration; this portion is static evidence only.
|
||||
|
||||
### Legacy filenames are active, but were not restored here
|
||||
|
||||
The broader assertion that Kilo reads no OpenCode-named configuration files is not true of actual base or pinned main:
|
||||
|
||||
- `packages/opencode/src/config/config.ts:434` and `:438` merge `opencode.json`/`opencode.jsonc` **inside the Kilo global directory**.
|
||||
- `packages/opencode/src/config/config.ts:744` retains root-level `opencode.json[c]` alongside `kilo.json[c]`.
|
||||
- `packages/opencode/src/kilocode/config/config.ts:41` retains these filenames inside allowed Kilo directories and managed directories.
|
||||
- `packages/opencode/src/kilocode/skills/kilo-config.md:338`–`:346` explicitly documents legacy filenames while excluding `.opencode` directories.
|
||||
|
||||
A bounded runtime control confirmed root `opencode.json` still overrides root `kilo.json` for the same model field. This is existing lookup order, not a newly reordered `.kilo` lookup. A separate poisoned sibling XDG `opencode/opencode.json` was ignored. No filename-compatibility removal is proposed as a merge fix.
|
||||
|
||||
### Changed `customize-opencode` documentation is not a registered Kilo builtin
|
||||
|
||||
The only change at `packages/core/src/plugin/skill/customize-opencode.md:43` adds `~/.config/opencode/opencode.jsonc` to an upstream documentation row. Upstream history contains `62387f39d4` (`fix(skills): Update global config path in documentation (#42337)`) and generated adjustment `ab7cbc808f` for this file.
|
||||
|
||||
The body is imported by `packages/core/src/plugin/skill.ts:9`, but Kilo's production registration deliberately omits that plugin at `packages/core/src/plugin/internal.ts:112`. Repository-wide caller search found only the direct core unit test invoking `SkillPlugin.Plugin`; the config-skill plugin is a different module. The production exclusion is unchanged from actual base and pinned main.
|
||||
|
||||
The CLI seeds `kilo-config` from `packages/opencode/src/kilocode/skills/builtin.ts:14` through `packages/opencode/src/skill/index.ts:301`. A bounded real Skill-service check returned `kilo-config` and not `customize-opencode`. System skill context uses this registry at `packages/opencode/src/session/system.ts:159`; its environment context still directs creation into `.kilo`, explicitly not `.kilocode` or `.opencode`, at `packages/opencode/src/kilocode/system-prompt.ts:29`. No built-in documentation regression was demonstrated.
|
||||
|
||||
### Instructions, flags, migration, and path-like names
|
||||
|
||||
- `packages/opencode/src/session/instruction.ts:62` still prefers explicit `KILO_CONFIG_DIR/AGENTS.md`, then the Kilo global root, with the separately supported Claude fallback. Project `AGENTS.md`/`CLAUDE.md`/`CONTEXT.md` handling and provenance restrictions are unchanged. Three instruction-profile precedence tests passed.
|
||||
- `packages/core/src/flag/flag.ts:45`, `:131`, and `:140` use `KILO_CONFIG`, `KILO_DISABLE_PROJECT_CONFIG`, and `KILO_CONFIG_DIR`; no `OPENCODE_CONFIG` alias was added. The bounded disable-project check passed.
|
||||
- `packages/opencode/src/config/tui.ts:192`/`:223` reuse/filter Kilo directories. Despite its old helper name, `packages/opencode/src/config/tui-migrate.ts:126` discovers `kilo.json[c]`, not implicit OpenCode config files. Neither path changed in this PR.
|
||||
- `packages/opencode/src/kilocode/config/config.ts:643` checks existence of leftover OpenCode directories for a migration notice; it does not parse or merge their content. A bounded test distinguished detection from loading.
|
||||
- The changed `opencode-go` provider identifier in `packages/opencode/src/tool/registry.ts:87`, the account console URL, provider SDK imports, and `.opencode-version` download-cache/version markers are not restored local config discovery. Existing well-known remote config is auth-selected, not a filesystem fallback.
|
||||
|
||||
## Command outputs and validation
|
||||
|
||||
All terminal commands had the designated review checkout or its `packages/opencode` directory as `workdir`. No dependency installation was performed by this reviewer.
|
||||
|
||||
| Command/check | Result |
|
||||
|---|---|
|
||||
| `git rev-parse HEAD` and `git merge-base bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD` | Exact supplied HEAD and actual base |
|
||||
| `git diff --stat bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD` | 59 files, 1524 insertions, 647 deletions |
|
||||
| `git rev-list --count bf1cf502a3c511e9daf6a43244568ae4e83473a8..HEAD` | `95` |
|
||||
| `git log --first-parent --oneline bf1cf502a3c511e9daf6a43244568ae4e83473a8..HEAD` | `6a7d6bc002 resolve merge conflicts`; `91ca95bad9 merge: record upstream v1.18.20` |
|
||||
| Scoped upstream .18 → .20 loader diff | No changes to config selectors, core global/flags/FS walker, instruction loader, or builtin registration |
|
||||
| Scoped actual-base/first-parent → HEAD loader diff | Empty |
|
||||
| `git diff --check` | Passed; no tracked source changes |
|
||||
|
||||
Final test runs used Bun `1.3.14`, empty inherited environment except PATH, checkout-local HOME/TMPDIR, canonical `GIT_CEILING_DIRECTORIES`, `GIT_CONFIG_NOSYSTEM=1`, and `KILO_DISABLE_MODELS_FETCH=1 KILO_DISABLE_DEFAULT_PLUGINS=1 KILO_DISABLE_LSP_DOWNLOAD=1`. The package test preload supplied disposable XDG roots and an in-memory database. A diagnostic preload replaced only host `readManagedPreferences()` with an empty result.
|
||||
|
||||
From `packages/opencode`, with that environment:
|
||||
|
||||
```text
|
||||
bun test --preload ../../.review-config-r6/preload.ts ../../.review-config-r6/bounded.test.ts
|
||||
6 pass, 0 fail, 23 expect() calls; 1.384s
|
||||
|
||||
bun test --preload ../../.review-config-r6/preload.ts ./test/kilocode/instruction.test.ts ./test/kilocode/config/variable.test.ts
|
||||
16 pass, 2 skip, 0 fail, 19 expect() calls; 2.39s
|
||||
```
|
||||
|
||||
The six bounded checks exercised actual runtime code with an explicit fixture-local `InstanceRef.worktree`: project config/agent/command/plugin/skill exclusions and positive Kilo controls; home directory exclusion plus explicit-profile ordering; legacy root filename compatibility; global OpenCode sibling exclusion; project-config disable; and migration detection without loading. Auth/account and NPM used existing test fixtures, avoiding credentials and package installation. Diagnostic files were removed after verification.
|
||||
|
||||
Two earlier attempts are not counted as successful isolated validation:
|
||||
|
||||
1. Existing `test/kilocode/config/config.test.ts`, filtered to `project config directory precedence|opencode config migration notice`: **3 pass, 3 fail, 46 filtered out**. With TMPDIR inside this checkout, notice tests walking without a worktree stop detected the checkout's own `.opencode` directory. More importantly, the noncanonical Git-ceiling value did not isolate the fixture: loader logs showed attempted config reads under the primary checkout at `/Users/johnnyamancio/Workspace/kilo_workspace/kilocode/`. This run is discarded as contaminated. It is not evidence that the PR reintroduced OpenCode config loading.
|
||||
2. First bounded-harness invocation: **0 pass, 1 import error** because the diagnostic used a relative Effect package-directory import. Corrected to its existing `dist/index.js`; the subsequent four-check run and final six-check run passed.
|
||||
|
||||
## Limitations and cleanup
|
||||
|
||||
- Configuration-discovery verdict only, not the overall seven-lens PR verdict. No live model calls, provider credential integration, Windows execution, real enterprise MDM profiles, or full linked-worktree integration run. The two skipped substitution tests are Linux-specific `/proc` protections.
|
||||
- Pinned local refs and parent graph were verified; this reviewer did not refetch tags, inspect live GitHub checks, or re-query the remote PR. HEAD remained `6a7d6bc002319ac2987bcde3d6c63efcafc07021` at completion.
|
||||
- The first fixture run violated the intended runtime isolation boundary by attempting primary-checkout config reads. No credential values were printed, but this report does **not** claim that the entire review avoided all external config access. No subsequent external inspection/repair was attempted; corrected bounded runs constrained the config context and asserted fixture-local discovery paths. The initial run's possible incidental config-setup effects outside the checkout were not independently audited.
|
||||
- No source edits, commits, pushes, GitHub changes, branch switches, or Git configuration commands were made by this reviewer. No fixture tests that deliberately create commits or edit Git configuration were run. Temporary `.review-config-r6` diagnostics and test state were removed; final tracked `git diff` was empty. Other reviewers' untracked reports were left untouched.
|
||||
- Lint/typecheck and broad suites were not duplicated for this report-only review; semantic targeted checks are reported above. Main-relative warning/persistence changes and the existing MDM policy question require their own provenance/scope decisions, not fixes attributed to this incremental merge.
|
||||
@@ -0,0 +1,245 @@
|
||||
# Infrastructure change review — PR #13513
|
||||
|
||||
## Summary, scope, and methodology
|
||||
|
||||
**Infrastructure verdict: safe after specific fixes.** One P2 SDK-generation compatibility regression is present relative to the actual PR base, although it also exists on comparison main. Release-note coverage needs a P3 follow-up. The remaining infrastructure changes require explicit human verification rather than being classified as defects.
|
||||
|
||||
Reviewer 2 audited the actual PR delta for workflows/actions, CI scheduling, release/deployment, containers/builds, manifests/workspaces/toolchain pins, lockfiles, repository automation, issue templates, changesets/changelogs, and SDK generation. Comparisons covered actual base → HEAD, pristine upstream v1.18.18 → v1.18.20, pristine upstream → HEAD, both merge parents → result, and comparison main → HEAD. Static caller tracing was supplemented with read-only guards, SDK tests/typecheck/lint, two disposable generator runs, and real loopback HTTP probes against complete base/HEAD/main SDK snapshots.
|
||||
|
||||
All shell commands ran from `/Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports`. No caller checkout access, source edits, dependency installation, branch switching, Git configuration changes, commits, pushes, or GitHub mutations were performed. Disposable output and control snapshots were outside the checkout and removed by the probes.
|
||||
|
||||
- Actual base / merge base: `bf1cf502a3c511e9daf6a43244568ae4e83473a8` (`johnnyeric/kilo-opencode-v1.18.18`).
|
||||
- Reviewed HEAD: `6a7d6bc002319ac2987bcde3d6c63efcafc07021`; 59 changed files, 3 added / 56 modified, 95 reachable commits, two first-parent merges.
|
||||
- Comparison main: `62998965e9fb0d9ed89011c62498b39801dbbb4f`.
|
||||
- Local authoritative upstream refs resolve to `.18 = 31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d`, `.19 = 2b72179c663cadcb54f54d9f19221b3fb3d11fb6`, `.20 = 7248bc1964b13fa67e601733f89ee9dc6dfa0563`.
|
||||
- `91ca95bad927436131ea4783a470885a381ce6ad` has actual base and pristine `.20` as parents and is tree-identical to the base. HEAD has `91ca95bad9` and transformed `9563af96a012effc25df5a11eaa1f7633161a742` as parents. Thus the effective tree changes occur at the final merge, not the ancestry-recording merge.
|
||||
|
||||
## Findings
|
||||
|
||||
### F1 — P2: generator upgrade loses error wrapping for client-level `throwOnError`
|
||||
|
||||
**Location:** `packages/sdk/js/src/v2/gen/client/client.gen.ts:201`; activating dependency change at `packages/sdk/js/package.json:26`. Kilo consumer: `packages/sdk/js/src/error-interceptor.ts:19`, registered by `packages/sdk/js/src/v2/client.ts:100`.
|
||||
|
||||
**Invariant:** `createKiloClient({ throwOnError: true })` should preserve the same structured `Error` behavior as setting `{ throwOnError: true }` on an individual operation. Kilo's interceptor deliberately gives thrown server errors a usable `.message` and `.cause.status`.
|
||||
|
||||
The new generated client computes the effective throw policy from `options.throwOnError ?? _config.throwOnError` at line 74, but passes the **unresolved operation options** into error interceptors at line 201. The previous generated client passed resolved `opts`, including client defaults. Consequently `wrapClientError` sees no `throwOnError`, returns the decoded error object unchanged, and the client throws that object instead of an `Error`. A consumer reading `err.message`, `err.cause.status`, or formatting `String(err)` loses the useful message/status and can display `[object Object]`.
|
||||
|
||||
**Proof / controls:** a real local HTTP server returned status 400 and `{ name: "BadRequestError", data: { message: "review proof: denied" } }`. Calling `client.global.health()` with client-level `throwOnError: true` rejected with an `Error` on actual base, but a plain object on HEAD. Explicit per-call `throwOnError: true` still produced an `Error` on both. The nonthrowing result tuple remained unchanged. Complete SDK source snapshots, not a rewritten error algorithm, were used for the controls; exact output is below.
|
||||
|
||||
**Provenance:** introduced into this PR's actual-base delta by the Kilo-side `@hey-api/openapi-ts` upgrade to `0.97.3`, not by pristine upstream `.18 → .20` (which retains `0.90.10` and unchanged generated transport). The same generator and transport already exist on comparison main, and main reproduces the failure. This is **not a newly discovered main-only regression caused by this upstream release**; it is a demonstrated compatibility loss when adopting that generator into this stack.
|
||||
|
||||
**Fix direction:** preserve effective client defaults at the Kilo-owned error-interceptor registration boundary, or make generation preserve resolved options when invoking interceptors. Add a regression covering client default, per-call override, and nonthrowing tuple behavior. Do not hand-edit generated output without a reproducible generation fix.
|
||||
|
||||
### F2 — P3: the newly adopted release range has no changeset
|
||||
|
||||
**Location:** `.opencode-version:1` advances to `v1.18.20`; release-note consumption is in `script/publish.ts:18-29`.
|
||||
|
||||
The actual PR adds or updates no `.changeset` file and changes no changelog. The existing `.changeset/opencode-v1-18-16-to-v1-18-18.md:6` explicitly ends at v1.18.18; there is no `.19`/`.20` coverage in current changeset text. The PR contains user-facing provider/session fixes, so consuming the existing changesets will not describe this newly adopted range. This violates the documented release-note requirement, not the runtime or publishing chain.
|
||||
|
||||
**Provenance / control:** omission in this actual merge delta, not the older changeset deletions or package version differences visible only against main. Prior range changesets demonstrate the intended mechanism. Do not recreate already-consumed main security changesets merely because this stack is behind main.
|
||||
|
||||
**Fix direction:** add one concise user-facing patch changeset for the newly adopted range, covering the CLI/extension fixed release group, or explicitly document where equivalent release-note coverage is supplied before publication.
|
||||
|
||||
## Infrastructure changes requiring explicit human verification
|
||||
|
||||
These are **policy/product approval items, not additional severity-graded defects**. They account for all infrastructure-related changes in the actual PR delta, grouped by purpose rather than by every file.
|
||||
|
||||
### HV1 — Kilo SDK generator upgrade and compatibility policy
|
||||
|
||||
- `packages/sdk/js/package.json:26`: `@hey-api/openapi-ts` changes `0.90.10 → 0.97.3`; corresponding toolchain dependency graph changes are in `bun.lock:1578` onward. This is **Kilo adaptation matching comparison main**, not adoption of an upstream `.19/.20` generator upgrade.
|
||||
- `packages/sdk/js/script/build.ts:97-109` now accepts an already-correct SSE return signature, instead of requiring the old buggy signature to be replaced. This is necessary for `0.97.3`; the checked output still requires `ServerSentEventsResult<TData>`.
|
||||
- `packages/sdk/js/script/build.ts:111-122` adds a Kilo-specific patch restoring required `request: Request` / `response: Response` fields in the nonthrowing result type. That preserves the base's declared contract, but deliberately declines the generator's more accurate optional fields. Network errors already lacked a response on base; the broader new catch also allows pre-request failures to return with no request. This is a conscious compatibility tradeoff requiring approval, not proof that those fields always exist at runtime.
|
||||
- Ten generated V2 files change as a consequence. This is not purely formatting: the fetch catch now covers construction/validation/interception/parsing, interceptor error transformations compose, `buildUrl` includes client config, parameter maps use null prototypes, required flattened request fields become required, SSE callback typing changes, and colliding generated class identifiers gain suffixes. F1 is the verified integration failure among these changes. The underlying committed OpenAPI document is unchanged by this PR.
|
||||
- The caller chain remains `.github/workflows/generate.yml:36 → script/generate.ts:5 → SDK build`, and `.github/workflows/publish.yml:476 → script/publish.ts:71 → SDK build → script/publish.ts:120 → SDK publish`. `packages/sdk/js/script/publish.ts:23-25` still maps source exports to built `dist` JavaScript/declarations. The newly changed generator therefore affects both generated source and shipped SDK artifacts; it is not dormant developer tooling.
|
||||
|
||||
**Human verification:** explicitly approve the generator/security-pin reconciliation, generated API/transport changes, and result-type compatibility policy. Two disposable runs of the actual generation/postprocessing section matched all 15 tracked V2 files exactly after filename-aware formatting. The pre-existing history numeric-query and duplicate-schema guards still pass.
|
||||
|
||||
### HV2 — upstream runtime provider pins and regenerated dependency graph
|
||||
|
||||
- `packages/core/package.json:95,108` and `packages/opencode/package.json:82,130`: `@ai-sdk/google-vertex 4.0.128 → 4.0.181` and `ai-gateway-provider 3.1.2 → 3.2.0`.
|
||||
- These two pin changes **are present in pristine upstream `.18 → .20`**. Comparison main still has the older provider pins. They are legitimate upstream code/dependency adoption, but still require Kilo infrastructure approval.
|
||||
- `bun.lock:1154,2830` and nested package records change the resolved Vertex and gateway provider trees. The lock now carries newer nested Anthropic/Google/OpenAI-compatible/provider-utils versions and additional optional gateway-provider SDK copies, including their `undici` dependencies. This changes the shipped dependency graph even though root AI SDK pins remain fixed.
|
||||
|
||||
**Human verification:** approve the runtime dependency/bundle changes together with the upstream provider behavior. No independent provider API or cross-platform CLI build certification is claimed by this infrastructure lens.
|
||||
|
||||
### HV3 — Kilo minimatch pin reconciliation and lockfile hygiene
|
||||
|
||||
- `packages/core/package.json:80`: `minimatch 10.2.5 → 10.2.6`.
|
||||
- `packages/opencode/package.json:183`: `minimatch 10.0.3 → 10.2.6`.
|
||||
- Both match comparison main, while pristine `.20` retains the older differing pins. This is **Kilo-side reconciliation**, not an upstream release requirement.
|
||||
- `bun.lock:3954` changes the root resolution to `minimatch@10.2.6`, drops the separate old core resolution and unneeded `@isaacs` matcher dependencies, and uses the `brace-expansion` dependency selected by the newer matcher.
|
||||
- Across all SDK/provider/minimatch changes, the lock has **40 added, 15 changed, and 11 removed package records**. Exactly three workspace records change, with seven direct pin updates matching their manifests. Lockfile/config versions, workspace membership, catalog, overrides, trusted dependencies, and patch metadata are preserved.
|
||||
|
||||
**Human verification:** approve this dependency reconciliation and the complete lockfile delta; do not misclassify the root Bun/version differences against main as new changes in this PR.
|
||||
|
||||
### HV4 — upstream automation baseline advances
|
||||
|
||||
`.opencode-version:1` changes `v1.18.18 → v1.18.20`. This influences future upstream-marker/reset tooling through the documented baseline selection; it is not a Kilo package version bump. The final merge records pristine `.20` ancestry, and the resolved local `.20` ref matches the supplied authoritative SHA.
|
||||
|
||||
**Human verification:** approve `.20` as the next automation baseline together with the range release notes. Upstream `.19` is a release-commit sibling rather than an ancestor of `.20`; their common ancestor is `.19`'s parent `f4a89683da2fb5fd1b37995402100ca7a24a8484`. This is consistent with release-only tag commits and is not a wrong-target finding.
|
||||
|
||||
## Notable non-findings and inherited limitations
|
||||
|
||||
- **No workflow/action, CI configuration, release/deploy script, Docker/container definition, Nix file, root package/workspace configuration, issue/PR template, changelog, or changeset content changes occur in the actual PR delta.** The only changed handwritten build script is the SDK generator discussed above. The broader differences against main (including Bun `1.3.14 → 1.4.0`, package versions `7.5.5` versus `7.5.0`, older changelog/changeset state, Nix/container adjustments, and upstream automation edits) belong to the existing stack, not this PR's effective change set.
|
||||
- Pristine upstream deletes `.github/workflows/beta.yml` and removes preview-CLI publishing from `script/publish.ts`. Kilo retains its own beta workflow and its existing publishing script unchanged. Kilo already did not publish the preview CLI. No upstream-hosted application, deployment, or desktop publishing infrastructure is reintroduced.
|
||||
- CLI/core `test:ci` commands and all scripts in the three changed manifests are preserved. The workflow allowlist and package scheduling guard pass. No new no-op release/test chain was introduced.
|
||||
- **Pre-existing CI coverage gap:** `@kilocode/sdk` has no `test:ci` on base, HEAD, or comparison main. Turbo's dry run reports `@kilocode/sdk#test:ci` as `<NONEXISTENT>`. `script/check-test-ci.ts:19,34` collapses the nested test path to `packages/sdk`, which has no package manifest, and skips it. Thus the passing guard does not establish SDK test scheduling. This is unchanged Kilo infrastructure, not a new PR finding; the seven existing SDK tests were run manually here.
|
||||
- **Pre-existing generation coverage limitation:** `generate.yml:3-6` runs on pushes to `dev`, not as a PR SDK-regeneration gate. `check-kilo-generated-artifacts.yml:6-11` does not cover SDK source generation. Neither is treated as proof that F1 or generator behavior was tested by PR CI.
|
||||
- No generated SDK drift remains in the disposable fixture check. An initial one-character generic trailing-comma difference came from the review harness omitting Prettier's `filepath`; rerunning with the actual `.ts` filename produced an exact match. It was a harness artifact, not a repository finding.
|
||||
|
||||
## Exact command outputs and evidence
|
||||
|
||||
All commands below used the separate review checkout as their working directory. Long diff and lint outputs are represented by explicitly labeled decisive excerpts.
|
||||
|
||||
### Read-only guards and existing SDK checks
|
||||
|
||||
```text
|
||||
$ bun --version
|
||||
1.3.14
|
||||
|
||||
$ bun run script/check-workflows.ts
|
||||
check-workflows: ok (29 workflows).
|
||||
|
||||
$ bun run script/check-test-ci.ts
|
||||
check-test-ci: ok (25 test-bearing package(s), 11 root script test file(s))
|
||||
|
||||
$ git diff --check bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD
|
||||
[no output; exit 0]
|
||||
|
||||
$ bun test ./packages/sdk/js/test/session-history.test.ts ./packages/sdk/js/test/server.test.ts
|
||||
bun test v1.3.14 (0d9b296a)
|
||||
|
||||
7 pass
|
||||
0 fail
|
||||
11 expect() calls
|
||||
Ran 7 tests across 2 files. [37.00ms]
|
||||
|
||||
$ bun run --cwd packages/sdk/js typecheck --incremental false --composite false
|
||||
$ tsgo --noEmit --incremental false --composite false
|
||||
[exit 0]
|
||||
|
||||
$ bun run lint packages/sdk/js/script/build.ts packages/sdk/js/src/v2/gen
|
||||
[summary excerpt; exit 0]
|
||||
Found 320 warnings and 0 errors.
|
||||
Finished in 1.3s on 15 files with 130 rules using 18 threads.
|
||||
```
|
||||
|
||||
Lint warnings are not promoted to review findings; they include generated-code type/style warnings. The complete lint output is retained at `/Users/johnnyamancio/.local/share/kilo/tool-output/tool_0439358b20015p0VrBqf4hgcU2`.
|
||||
|
||||
### Disposable generation
|
||||
|
||||
```text
|
||||
$ bun /var/folders/pd/_rh0zzyx19ncnzldhjlt3mtc0000gp/T/kilo/pr13513-infra-r2-generate.ts
|
||||
[decisive stdout; exit 0]
|
||||
{"round":1,"generatedFiles":15,"drift":[]}
|
||||
{"round":2,"generatedFiles":15,"drift":[]}
|
||||
SDK fixture generation: two identical rounds; numeric history, duplicate-schema, SSE and result-contract patches passed
|
||||
```
|
||||
|
||||
The harness executes the actual `build.ts` section from `const document` through the new result-contract patch, redirecting only file I/O and generator output to a disposable directory. It supplies the committed OpenAPI document, uses the installed `@hey-api/openapi-ts@0.97.3`, and formats with the repository's settings and actual filenames. Both generations were byte-identical to each other and to tracked V2 output. The generator also emitted its nonfatal `instance` deprecation notice; changing that option is not required to establish this PR's correctness.
|
||||
|
||||
### Real HTTP error-regression proof
|
||||
|
||||
```text
|
||||
$ bun /var/folders/pd/_rh0zzyx19ncnzldhjlt3mtc0000gp/T/kilo/pr13513-infra-r2-error.ts
|
||||
{"ref":"base","mode":"config","rejected":true,"isError":true,"message":"review proof: denied"}
|
||||
{"ref":"base","mode":"call","rejected":true,"isError":true,"message":"review proof: denied"}
|
||||
{"ref":"base","mode":"tuple","rejected":false,"isError":false,"error":{"name":"BadRequestError","data":{"message":"review proof: denied"}}}
|
||||
{"ref":"HEAD","mode":"config","rejected":true,"isError":false,"error":{"name":"BadRequestError","data":{"message":"review proof: denied"}}}
|
||||
{"ref":"HEAD","mode":"call","rejected":true,"isError":true,"message":"review proof: denied"}
|
||||
{"ref":"HEAD","mode":"tuple","rejected":false,"isError":false,"error":{"name":"BadRequestError","data":{"message":"review proof: denied"}}}
|
||||
{"ref":"main","mode":"config","rejected":true,"isError":false,"error":{"name":"BadRequestError","data":{"message":"review proof: denied"}}}
|
||||
{"ref":"main","mode":"call","rejected":true,"isError":true,"message":"review proof: denied"}
|
||||
{"ref":"main","mode":"tuple","rejected":false,"isError":false,"error":{"name":"BadRequestError","data":{"message":"review proof: denied"}}}
|
||||
```
|
||||
|
||||
The local server is stopped and snapshot copies are removed in `finally`. The probe uses `createKiloClient` and actual generated clients, not duplicated transport/error logic.
|
||||
|
||||
### Lockfile and CI-task verification
|
||||
|
||||
```text
|
||||
$ bun /var/folders/pd/_rh0zzyx19ncnzldhjlt3mtc0000gp/T/kilo/pr13513-infra-r2-lock.ts
|
||||
[final summary excerpt; all seven manifest/lock pin assertions passed; exit 0]
|
||||
{"added":40,"changed":15,"removed":11}
|
||||
```
|
||||
|
||||
The asserted preserved metadata keys were `lockfileVersion`, `configVersion`, `trustedDependencies`, `patchedDependencies`, `overrides`, and `catalog`. Changed workspaces were exactly `packages/core`, `packages/opencode`, and `packages/sdk/js`.
|
||||
|
||||
Turbo command executed by the read-only JSON-summary wrapper:
|
||||
|
||||
```text
|
||||
bun turbo run test:ci --filter=@kilocode/sdk --dry=json
|
||||
```
|
||||
|
||||
Exact wrapper output:
|
||||
|
||||
```text
|
||||
exit=0
|
||||
{
|
||||
"packages": [
|
||||
"@kilocode/sdk"
|
||||
],
|
||||
"tasks": [
|
||||
{
|
||||
"taskId": "@kilocode/sdk#test:ci",
|
||||
"command": "<NONEXISTENT>"
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
### Control and integrity checks
|
||||
|
||||
These commands all exited 0 with no output:
|
||||
|
||||
```sh
|
||||
git diff --quiet bf1cf502a3c511e9daf6a43244568ae4e83473a8 91ca95bad927436131ea4783a470885a381ce6ad
|
||||
git diff --quiet bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD -- .github script infra nix package.json bunfig.toml turbo.json .changeset '**/CHANGELOG*' '**/Dockerfile*'
|
||||
git diff --quiet refs/review/pr-13513/upstream-v1.18.18 refs/review/pr-13513/upstream-v1.18.20 -- packages/sdk/js/src/v2/gen/client/client.gen.ts
|
||||
git diff --quiet bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD -- packages/sdk/js/src/error-interceptor.ts
|
||||
git diff --quiet 62998965e9fb0d9ed89011c62498b39801dbbb4f HEAD -- packages/sdk/js/src/v2/gen/client/client.gen.ts
|
||||
git diff --exit-code
|
||||
git diff --cached --exit-code
|
||||
```
|
||||
|
||||
Latest exact-head check before report creation:
|
||||
|
||||
```text
|
||||
$ git rev-parse HEAD
|
||||
6a7d6bc002319ac2987bcde3d6c63efcafc07021
|
||||
```
|
||||
|
||||
The initial checkout was clean. Later untracked reports/temporary review files belonged to other parallel reviewers; they were neither edited nor removed. This review adds only `INFRASTRUCTURE_CHANGE.md` in the checkout. Diagnostic harnesses are retained under the approved temporary directory for reproducibility; generated output and snapshot trees have been cleaned up.
|
||||
|
||||
## Limitations
|
||||
|
||||
- Local Bun is `1.3.14`; the unchanged stack pin is `bun@1.4.0`. Parent-owned frozen installation with lifecycle scripts disabled was not rerun by this reviewer. Lockfile assertions and installed generator execution are not a substitute for a fresh install on every supported platform.
|
||||
- Disposable generation validates the committed OpenAPI fixture → V2 SDK path, not live server → OpenAPI freshness. No tracked regeneration, complete publish build, `tsc` declaration emission, npm packaging, Docker/Nix build, release execution, or deployment was attempted. Existing history/type tests and source typecheck do not certify every downstream consumer.
|
||||
- No broad provider/model, CLI, VS Code, or JetBrains tests were run by this lens; those belong to the other reviewers/parent. F1 proves the supported public SDK configuration path, not a claim that all current product calls use client-level defaults.
|
||||
- GitHub CI/mergeability and remote head freshness were not independently queried by this worker; exact supplied local refs were used and local HEAD was rechecked. The parent should reconcile the final PR-wide verdict and any main/base movement.
|
||||
- This infrastructure lens accessed no real user credentials, configuration, databases, or model endpoints. Its only network test was a temporary loopback HTTP server. This statement does not cover other reviewers: `CONFIG_REGRESSION.md` discloses an initial fixture run that attempted primary-checkout config reads, discarded results, corrected isolation, and unaudited possible incidental setup effects.
|
||||
|
||||
## Coordinator validation and cross-report reconciliation
|
||||
|
||||
All seven assigned reports were completed by their respective reviewers; the marker-chain reviewer completed a follow-up static trace covering all 737 marker-bearing lines in 29 changed files. Overall reviewed-head verdict: **safe after specific fixes**, based on the two distinct merge-relative P2 findings in `BROKEN_PIPELINE_CHAINS.md`. This report's SDK finding is the same SDK finding, not a third defect. Existing main/base problems and human policy checks are explicitly separated from new regressions.
|
||||
|
||||
Coordinator checks, performed after the report reviews:
|
||||
|
||||
```text
|
||||
$ bun run script/check-md-table-padding.ts KILOCODE_CHANGE_MARKERS.md INFRASTRUCTURE_CHANGE.md OPENCODE_MENTIONS.md UNNECESSARY_MARKERS.md BROKEN_PIPELINE_CHAINS.md CONFIG_REGRESSION.md TESTS.md
|
||||
check-md-table-padding: 7 file(s) checked, no padded tables found.
|
||||
|
||||
$ bun run lint
|
||||
Found 9489 warnings and 0 errors.
|
||||
Finished in 18.0s on 5461 files with 130 rules using 18 threads.
|
||||
|
||||
$ bun run typecheck --incremental false --composite false
|
||||
$ tsgo --noEmit --incremental false --composite false
|
||||
[run separately from packages/core and packages/tui; both exit 0]
|
||||
```
|
||||
|
||||
The CLI and SDK typechecks passed in the pipeline lens; this is not a claim that the coordinator ran the full root Turbo/JetBrains typecheck. The coordinator independently reran the real HTTP SDK harness recorded above and reproduced all nine base/HEAD/main outcomes. All local runtime checks still used Bun 1.3.14, not pinned 1.4.0.
|
||||
|
||||
Read-only `gh pr view 13513` recheck returned the same head/base pins, `MERGEABLE` / `CLEAN`, and all 29 non-skipped checks successful; `[code]smith` was skipped. Independent tag verification corrected an initial marker-report summary: both release-only `.18` and `.19` tag commits are siblings, not ancestors, of `.20`; their common ancestors with `.20` are their respective parents. The exact `.20` tag is an ancestor of the reviewed head, so the target remains valid.
|
||||
|
||||
Before staging, the isolated checkout contained only the seven untracked report files and no tracked-source diff. The original user checkout remained clean on `johnnyeric/kilo-opencode-v1.18.20` at `ff7f6654fd2692013ed80f78516de4bd6c21267e`; no report branch was checked out there. Publication is limited to these reports in the separate branch, with the reviewed PR branch as the draft PR base. No source fixes or edits to the reviewed PR are part of this work.
|
||||
@@ -0,0 +1,143 @@
|
||||
# Kilo change-marker review — PR #13513
|
||||
|
||||
## Scope and methodology
|
||||
|
||||
Reviewed **all 59 files** in the actual PR comparison: **3 added, 56 modified**, including all **10 generated SDK files**. This is the marker-preservation/fork-delta lens, not an overall PR approval. File-by-file comparisons were performed internally; this report intentionally omits an exhaustive checklist.
|
||||
|
||||
All commands ran with `/Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports` as the tool working directory. Historical files were read from Git objects, without checking out another revision. Read root and applicable CLI, LLM-adapter, HTTP-handler, and test instructions, plus the upstream merge documentation; loaded `kilo-steer` and `kilocode-merge-minimizer`.
|
||||
|
||||
Pinned comparisons:
|
||||
|
||||
| Revision | SHA |
|
||||
|---|---|
|
||||
| Actual PR base and merge base | `bf1cf502a3c511e9daf6a43244568ae4e83473a8` |
|
||||
| Reviewed HEAD | `6a7d6bc002319ac2987bcde3d6c63efcafc07021` |
|
||||
| Main control | `62998965e9fb0d9ed89011c62498b39801dbbb4f` |
|
||||
| Pristine upstream v1.18.18 | `31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d` |
|
||||
| Pristine upstream v1.18.19 | `2b72179c663cadcb54f54d9f19221b3fb3d11fb6` |
|
||||
| Pristine upstream v1.18.20 | `7248bc1964b13fa67e601733f89ee9dc6dfa0563` |
|
||||
| Recorded upstream merge | `91ca95bad927436131ea4783a470885a381ce6ad` |
|
||||
| Transformed upstream parent | `9563af96a012effc25df5a11eaa1f7633161a742` |
|
||||
|
||||
The range contains **95 reachable commits and two first-parent merges**. The recorded merge has the base and pristine v1.18.20 as parents; its tree equals the base tree. HEAD merges that recorded commit with the transformed parent. Thus the first-parent recording commit itself did not remove source behavior.
|
||||
|
||||
For every changed file, compared base, main, merged HEAD, pristine upstream, and merge-parent blobs. In addition to raw diffs and blob equality, compared the Kilo residual against upstream before and after the merge, ignoring marker-only/whitespace changes for that secondary comparison. Inspected every actual marker removal/move separately and traced material adaptations through their implementations and tests. Generated files and the lockfile were compared as such, rather than treated as handwritten missing-marker violations.
|
||||
|
||||
## Lens verdict
|
||||
|
||||
**Safe to merge from the marker-preservation lens, with two P3 annotation follow-ups.** No demonstrated runtime behavior loss resulted from the marker removals. One inherited provider-limit decision warrants human confirmation; it is not an established PR-introduced defect.
|
||||
|
||||
## Findings
|
||||
|
||||
### P3 — Newly adapted ripgrep assertion lacks its Kilo marker
|
||||
|
||||
- **Location:** `packages/core/test/ripgrep.test.ts:94`.
|
||||
- **Evidence/control:** Pristine v1.18.20 and transformed parent assert `matches[0]?.text`; HEAD correctly asserts `matches.items[0]?.text`. Kilo's real implementation returns the metadata-bearing object at `packages/core/src/ripgrep.ts:301`. The existing equivalent adaptations at test lines 59–60 have inline markers; line 94 is outside the block ending at line 78 and has none.
|
||||
- **Provenance:** Introduced by this PR's adaptation of a new upstream test. The test is absent from both the actual base and pinned main.
|
||||
- **Impact:** Test behavior is correct today. The necessary fork-specific result-shape adaptation is invisible to marker-oriented merge review, making an upstream reset liable to restore the wrong assertion. The actual-base guard skips this PR, so its successful exit does not detect the omission.
|
||||
- **Minimal action:** Add a single inline marker to line 94, matching the existing assertions. Do not mark the entire upstream test or move it merely to avoid this one-line adaptation.
|
||||
|
||||
### P3 — New SDK compatibility postprocessing is unmarked in shared handwritten code
|
||||
|
||||
- **Location:** `packages/sdk/js/script/build.ts:107` and `:111–122`.
|
||||
- **Evidence/control:** The base and transformed parent require the old SSE replacement to change the generated text. HEAD intentionally accepts an already-correct signature and adds a postprocessor restoring required `request`/`response` fields. The latter block is absent from pristine upstream, the transformed parent, the actual base, and pinned main. It is not generated code and has no `kilocode_change` annotation. The resulting contract is visible at `packages/sdk/js/src/v2/gen/client/types.gen.ts:121`.
|
||||
- **Provenance:** The compatibility adaptation is introduced in this PR; other unmarked customizations elsewhere in this build script predate it and are not charged to this PR.
|
||||
- **Impact:** A still-required Kilo postprocessor can be mistaken for upstream code during a later SDK build-script reconciliation. Restoring the upstream guard would reject the newer generator's already-correct SSE output; losing the result-field postprocessor would remove this PR's intentional source-compatibility contract. The annotation checker does not include the SDK package in its scopes.
|
||||
- **Minimal action:** Narrowly annotate the changed acceptance condition and the new compatibility block in the handwritten build script. Do not hand-annotate generated SDK files. The in-memory control below confirms the actual patch accepts base/main/HEAD fixtures and is idempotent; this is not a claim that the SDK generator is broken now.
|
||||
|
||||
## Human verification — not an established defect
|
||||
|
||||
### HV1 — Confirm the retained GPT-5.6 OAuth context-limit override
|
||||
|
||||
- **Location:** `packages/opencode/src/plugin/openai/codex.ts:433–437`.
|
||||
- **Severity:** Unscored pending confirmation; potentially P2 if the backend limit is lower than the advertised Kilo limit.
|
||||
- **Evidence/control:** Upstream v1.18.18 used 500,000 context / 372,000 input for GPT-5.6; v1.18.20 deliberately reduces these to 400,000 / 272,000. Kilo retains its marked 1,050,000 / 922,000 override, byte-for-byte from both base and pinned main. The model filter permits suffixed GPT-5.6 IDs even though it rejects the exact `gpt-5.6` alias. Kilo's tests deliberately expect the larger limits.
|
||||
- **Provenance:** Pre-existing Kilo/main behavior retained across a new upstream limit change, not a newly introduced override or accidentally deleted marker.
|
||||
- **Possible impact/action:** If upstream's smaller limit also applies to Kilo's eligible OAuth models, compaction could start too late. Confirm the model-specific backend limits and retain or adjust the existing marked override accordingly. No live provider-limit probe was performed, so this is a product/API verification concern, not proof of a regression.
|
||||
|
||||
## Notable non-findings
|
||||
|
||||
- **All actual removals accounted for:** The base-to-HEAD diff removes four marker-bearing lines across two production files. Three are in `packages/opencode/src/server/routes/instance/httpapi/handlers/provider.ts`: an adjacent end/start pair is consolidated, and a redundant inline marker is removed. The surviving block at lines 62–87 still covers prompt-training filtering, failed-provider retention, provider metadata, default selection, and the adapted connected-provider expression. None of those behaviors disappeared.
|
||||
- **Websearch marker moved, not lost:** The fourth removal is the old marked return in `packages/opencode/src/tool/registry.ts:85–90`. Its replacement marks the Kilo-specific provider predicate at line 86. The `opencode-go` branch comes from pristine upstream; Kilo still replaces the ordinary OpenCode default provider and retains explicit search flags. The newly worded test title is a cosmetic branding difference, not a lost feature.
|
||||
- **Compatibility resets deliberately preserved:** Upstream removes the context-epoch import and two reset calls. HEAD retains and newly annotates them in `packages/core/src/session/projector.ts:16`, `:276`, and `:475`. Kilo still has the table, runner initialization/prepare calls, and history reads; retaining the resets is coherent with that surviving implementation, not an obsolete-markers defect.
|
||||
- **Task failure handling reconciled:** `packages/opencode/src/tool/task.ts:271–280` adopts upstream terminal-tool-error detection while preserving Kilo's resumable `task_id` error hint. The corresponding tests retain Kilo-specific assertions. Background-process finalization and cost propagation remain wired. Some newly upstream-equivalent lines remain inside existing broader blocks; that is not missing behavior.
|
||||
- **Main-only Task fix is not removed by this PR:** The synthetic/ignored/empty-text filtering present on pinned main is already absent from the actual base. Both the implementation and its regression-test difference are inherited stack/main drift, not marker removal in this 59-file PR. This is not a claim that the missing main fix is unnecessary; it belongs in later main reconciliation.
|
||||
- **Retry improvement reaches the Kilo-owned replacement:** Upstream adds a broad fallback to its shared provider-error parser. Kilo instead retains its existing marked `KiloError.fallback` hook at `packages/opencode/src/provider/error.ts:170` and extends capacity/temporary-unavailability matching in `packages/opencode/src/kilocode/provider/error.ts:56–57`. The new upstream regression test exercises that public parser. The shared file being absent from the changed-file list is not evidence that the improvement was silently dropped.
|
||||
- **Cloudflare deletion is an upstream replacement:** Removing the old output-token-cap hook is paired with native OpenAI/Anthropic passthrough routing and native SDK selection. This is upstream behavior adoption, not deletion of a Kilo-marked feature.
|
||||
- **TUI routed-model integration survives:** Opaque reasoning rendering is adopted while the marked part-ID propagation and `RoutedModelMeta.View` remain in `packages/tui/src/routes/session/index.tsx:1832–1834` and `:1891–1893`.
|
||||
- **SDK regeneration is not wholesale Kilo loss:** Seven of the ten changed generated files are byte-identical to pinned main. The remaining main differences were inspected: required result fields are intentional compatibility postprocessing, and the missing snapshot-removal endpoint is already absent from the actual base. The new generator's SSE correction makes the old mandatory-change assumption obsolete, not the final SSE contract.
|
||||
|
||||
## Commands and results
|
||||
|
||||
The following commands were run with the review-root working directory specified above. No dependency installation or source regeneration was performed by this reviewer.
|
||||
|
||||
```sh
|
||||
git status --short
|
||||
git rev-parse HEAD
|
||||
git merge-base bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD
|
||||
git diff --name-status bf1cf502a3c511e9daf6a43244568ae4e83473a8...HEAD
|
||||
git diff --stat bf1cf502a3c511e9daf6a43244568ae4e83473a8...HEAD
|
||||
git log --first-parent --format='%H %P %s' bf1cf502a3c511e9daf6a43244568ae4e83473a8..HEAD
|
||||
git rev-list --count bf1cf502a3c511e9daf6a43244568ae4e83473a8..HEAD
|
||||
git diff --quiet bf1cf502a3c511e9daf6a43244568ae4e83473a8 91ca95bad927436131ea4783a470885a381ce6ad
|
||||
git diff --check bf1cf502a3c511e9daf6a43244568ae4e83473a8...HEAD
|
||||
```
|
||||
|
||||
Results: initially clean; expected HEAD/base; 59 files, 1,524 insertions, 647 deletions; 95 reachable commits; two recorded first-parent merges; recording-tree equality and diff whitespace check exit 0.
|
||||
|
||||
```sh
|
||||
git rev-parse refs/review/pr-13513/upstream-v1.18.18 refs/review/pr-13513/upstream-v1.18.19 refs/review/pr-13513/upstream-v1.18.20
|
||||
git cat-file -t refs/review/pr-13513/upstream-v1.18.18
|
||||
git cat-file -t refs/review/pr-13513/upstream-v1.18.19
|
||||
git cat-file -t refs/review/pr-13513/upstream-v1.18.20
|
||||
git merge-base --is-ancestor 31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d 7248bc1964b13fa67e601733f89ee9dc6dfa0563
|
||||
git merge-base --is-ancestor 2b72179c663cadcb54f54d9f19221b3fb3d11fb6 7248bc1964b13fa67e601733f89ee9dc6dfa0563
|
||||
```
|
||||
|
||||
Results: exact pinned upstream SHAs above; each ref resolves directly to `commit`. Coordinator verification corrected the initial ancestry summary: **both `.18 → .20` and `.19 → .20` ancestry checks exit 1**, not 0. The `.18` and `.19` tags are release-only sibling commits; their parents/common ancestors with `.20` are `14b37df39168eaf6a6faf862ec4a7bbe9c825bbd` and `f4a89683da2fb5fd1b37995402100ca7a24a8484`, respectively. Pristine `.20` itself is a verified ancestor of the reviewed HEAD through the recording merge. This is not a wrong-target finding.
|
||||
|
||||
```sh
|
||||
bun run /Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports/script/check-opencode-annotations.ts --base bf1cf502a3c511e9daf6a43244568ae4e83473a8
|
||||
```
|
||||
|
||||
Exit 0, exact output:
|
||||
|
||||
```text
|
||||
Skipping shared upstream annotation check — upstream merge detected.
|
||||
```
|
||||
|
||||
**This is a skipped check, not an annotation pass.** The implementation at `/Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports/script/check-opencode-annotations.ts:257–259` skips merge ranges; it also exempts whole files with marker removals at line 274. Manual upstream/parent comparisons therefore supplied the substantive coverage. The read-only in-memory scan of new HEAD lines against both base and transformed upstream identified the ripgrep assertion, cosmetic websearch title, and SDK build postprocessor as unmarked fork-specific additions; each was inspected manually.
|
||||
|
||||
The exact in-memory SDK implementation control was:
|
||||
|
||||
```sh
|
||||
node -e 'const {execFileSync}=require("node:child_process"); const vm=require("node:vm"); const assert=require("node:assert/strict"); const cwd="/Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports"; const show=(ref,file)=>execFileSync("git",["show",`${ref}:${file}`],{cwd,encoding:"utf8"}); const build=show("6a7d6bc002319ac2987bcde3d6c63efcafc07021","packages/sdk/js/script/build.ts"); const block=build.slice(build.indexOf("const sseTypesPatched ="),build.indexOf("await Bun.write(sseTypesPath, compatible)")); const apply=sseTypesSource=>vm.runInNewContext(block+"; compatible",{sseTypesSource,sseTypesPath:"generated fixture"}); const path="packages/sdk/js/src/v2/gen/client/types.gen.ts"; for(const [label,ref] of [["base","bf1cf502a3c511e9daf6a43244568ae4e83473a8"],["main","62998965e9fb0d9ed89011c62498b39801dbbb4f"],["head","6a7d6bc002319ac2987bcde3d6c63efcafc07021"]]) {const input=show(ref,path);const output=apply(input);assert(output.includes("=> Promise<ServerSentEventsResult<TData>>"));assert(/request: Request\s+response: Response/.test(output));assert.equal(apply(output),output);console.log(label+": current build patch accepts historical blob, preserves required fields, and is idempotent");} assert.throws(()=>apply("unexpected generator output"),/SseFn patch did not apply/); console.log("malformed control: rejects changed generator signature");'
|
||||
```
|
||||
|
||||
Exit 0, exact output:
|
||||
|
||||
```text
|
||||
base: current build patch accepts historical blob, preserves required fields, and is idempotent
|
||||
main: current build patch accepts historical blob, preserves required fields, and is idempotent
|
||||
head: current build patch accepts historical blob, preserves required fields, and is idempotent
|
||||
malformed control: rejects changed generator signature
|
||||
```
|
||||
|
||||
The script evaluates the actual build-script patch block, not a reimplementation, and performs no filesystem writes.
|
||||
|
||||
```sh
|
||||
gh pr view 13513 --repo Kilo-Org/kilocode --json headRefOid,baseRefOid,baseRefName,url
|
||||
git diff --quiet
|
||||
git diff --cached --quiet
|
||||
git rev-parse HEAD
|
||||
```
|
||||
|
||||
Results: remote HEAD/base still match the reviewed pins, base branch remains `johnnyeric/kilo-opencode-v1.18.18`; both tracked-cleanliness checks exit 0; local HEAD unchanged.
|
||||
|
||||
## Limitations
|
||||
|
||||
- Static marker/behavior-preservation review plus the read-only SDK patch control; no full runtime, UI, database compatibility, live-provider, lint, typecheck, or package-test run is claimed by this lens. Reviewed tests are evidence of intended coverage, not reported as executed.
|
||||
- No SDK regeneration, dependency install, marker-rewriter execution, source edits, commits, pushes, branch switching, Git configuration changes, or GitHub mutations. The upstream refs supplied for this review were verified locally, not fetched again by this reviewer.
|
||||
- Exact rerere/mergiraf/manual-resolution attribution is not recoverable from the inspected committed trees alone; no automation-count claim is made.
|
||||
- Main comparisons distinguish inherited stack drift from changes in this PR; they do not certify that this branch already contains every later main fix.
|
||||
- The shared review checkout acquired other reviewers' untracked report/diagnostic files during review. They were not edited or removed. Tracked sources stayed clean; this reviewer's only written artifact is `/Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports/KILOCODE_CHANGE_MARKERS.md`.
|
||||
@@ -0,0 +1,144 @@
|
||||
# OpenCode mentions — PR #13513
|
||||
|
||||
## Scope and method
|
||||
|
||||
Reviewer 3/7, branding and OpenCode-web-property lens only. Reviewed all 59 changed paths, including complete handwritten production diffs, changed prompt/documentation text, package/lock metadata, test literals, generated SDK files, and unchanged context in the changed files. Followed suspect strings into CLI and builtin-skill registration, OAuth rendering, web-search descriptions/transports, provider listing, retry-action UI, and committed OpenAPI. Read root/package guidance, `REVIEW.md`, `kilo-steer`, merge-review guidance, merge-minimizer guidance, and `script/upstream/README.md`.
|
||||
|
||||
Compared the actual base to HEAD, pristine upstream `.18` to `.20`, pristine/transformed upstream to HEAD, and both final merge parents to HEAD; used pinned main as an additional provenance control, not as this stacked PR's base. Performed read-only Git-blob assertions rather than initializing the application or installing dependencies.
|
||||
|
||||
**Verdict: safe to merge for this lens. No verified merge-introduced user-facing OpenCode identity or OpenCode-link regression.** This is not a verdict on the other six review lenses. Existing residual branding and a non-blocking product-policy ambiguity are separated below.
|
||||
|
||||
Pins verified locally and against read-only PR metadata:
|
||||
|
||||
| Reference | SHA |
|
||||
|---|---|
|
||||
| Actual base / merge base, `johnnyeric/kilo-opencode-v1.18.18` | `bf1cf502a3c511e9daf6a43244568ae4e83473a8` |
|
||||
| Reviewed HEAD | `6a7d6bc002319ac2987bcde3d6c63efcafc07021` |
|
||||
| Pinned main control | `62998965e9fb0d9ed89011c62498b39801dbbb4f` |
|
||||
| Pristine upstream v1.18.18 | `31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d` |
|
||||
| Pristine upstream v1.18.19 | `2b72179c663cadcb54f54d9f19221b3fb3d11fb6` |
|
||||
| Pristine upstream v1.18.20 | `7248bc1964b13fa67e601733f89ee9dc6dfa0563` |
|
||||
| First-parent recording merge | `91ca95bad927436131ea4783a470885a381ce6ad` |
|
||||
| Transformed upstream / final second parent | `9563af96a012effc25df5a11eaa1f7633161a742` |
|
||||
|
||||
The recording merge has actual base and pristine `.20` as parents. HEAD has the recording merge and transformed upstream as parents. Scope is 95 reachable commits, two first-parent merges, and 59 files / 1,524 insertions / 647 deletions. The parent supplied independently fetched upstream refs; this reviewer verified their local object IDs without fetching or changing refs.
|
||||
|
||||
## Findings
|
||||
|
||||
None introduced by this merge in the assigned branding/link lens. In particular, the two newly changed OpenCode-facing literals are not registered product paths, and the new `opencode-go` literal identifies a third-party provider rather than Kilo itself.
|
||||
|
||||
## Notable non-findings and human verification
|
||||
|
||||
### Changed OpenCode console URL is dormant, not the Kilo Console destination
|
||||
|
||||
- **Location:** `packages/opencode/src/cli/cmd/account.ts:18`, consumed by its local `LoginCommand` at `:177-189`.
|
||||
- **Literal change:** `https://console.opencode.ai` → `https://opencode.ai/console`.
|
||||
- **Exposure proof:** Kilo's entrypoint omits the upstream account import and registration at `packages/opencode/src/index.ts:3` and `:108`. `KiloCli.register` instead registers `KiloConsoleCommand` at `packages/opencode/src/kilocode/cli/setup.ts:46`; its lazy loader points to the Kilo-owned command at `packages/opencode/src/kilocode/cli/lazy-kilo-commands.ts:3-6`. Repository reference searches found no production consumer that reinstates the upstream account command. The account unit test imports the URL directly, which does not prove CLI registration.
|
||||
- **Control/provenance:** Actual base, pinned main, and the recording merge contain the old URL and the same Kilo registration exclusions. Pristine `.20`, transformed upstream, and HEAD contain the new URL. Upstream origin is `2cba7e227d68a7e7e4a2aa9c85b808e8ecb14daf` (`fix(cli): update default console URL (#43043)`). The entire Kilo entrypoint and registration setup are byte-identical base→HEAD.
|
||||
- **Classification:** Statically verified dormant upstream maintenance; no demonstrated shipped user redirection. No correction required for this merge. Preserve the registration exclusion rather than blindly substituting a Kilo website into an incompatible upstream account protocol.
|
||||
|
||||
### Updated `customize-opencode.md` remains unregistered
|
||||
|
||||
- **Location:** `packages/core/src/plugin/skill/customize-opencode.md:43`.
|
||||
- The file still contains OpenCode config names and `https://opencode.ai/config.json`, but file presence is not runtime exposure. Its only embedding module, `packages/core/src/plugin/skill.ts:9-25`, defines the upstream plugin; production registration at `packages/core/src/plugin/internal.ts:107-120` deliberately omits that plugin. The direct upstream unit test explicitly invokes `SkillPlugin.Plugin.effect`, not the production registration graph.
|
||||
- Kilo's loader seeds `BUILTIN_SKILLS` at `packages/opencode/src/skill/index.ts:301-309`. That registry contains one builtin, `kilo-config`, with Kilo-specific description/content at `packages/opencode/src/kilocode/skills/builtin.ts:14-20`. The registry, loader, registered Kilo content, and core registration file are unchanged from actual base.
|
||||
- **Control/provenance:** Base and pinned main already retained the dormant document and disabled registration. Pristine `.18` and `.20` register it; transformed upstream still registers it; HEAD preserves Kilo's exclusion. The document's changed line comes from upstream `62387f39d4ccbe8672eb57a9a69d26e0ffa42b54` (`fix(skills): Update global config path in documentation (#42337)`).
|
||||
- **Classification:** Statically verified non-exposure in shipped registration; not a runtime branding finding. No correction required. Exact changed text is recorded below rather than claiming there were no prompt-file changes.
|
||||
|
||||
### `opencode-go` enables the existing search tool; it does not rename Kilo
|
||||
|
||||
- **Location:** `packages/opencode/src/tool/registry.ts:81-90`, used by the actual model tool filter at `:364-370`.
|
||||
- The added `providerID === ProviderV2.ID.make("opencode-go")` enables `websearch` for an explicitly selected OpenCode Go provider without requiring the Exa/Parallel enable flags. `ProviderV2.ID.kilo` remains enabled, and the ordinary `opencode` provider is not newly enabled. The changed test records precisely this distinction at `packages/opencode/test/tool/websearch.test.ts:33-39`.
|
||||
- The newly available description is the unchanged, provider-neutral `packages/opencode/src/tool/websearch.txt:1-14`. Display labels are `Parallel Web Search`, `Exa Web Search`, and `Web Search` at `packages/opencode/src/tool/websearch.ts:49-52`. The transports remain Kilo REST or the existing Exa/Parallel endpoints; see `:143-165`, `:188-201`, and `packages/opencode/src/tool/mcp-websearch.ts:7-11`. No OpenCode web property is added as a search destination.
|
||||
- **Control/provenance:** Base/main/recording merge enable Kilo or explicit flags only. Pristine `.20` and transformed upstream add Go beside upstream's own provider; HEAD retains Kilo in the first arm and adopts only the Go addition. Upstream origin: `4643e65ad6334de3e4e68dedc201d5fbb828c9fe` (`fix(opencode): enable web search for Go (#42630)`).
|
||||
- **Classification:** Verified provider-compatibility/tool-availability change, not product-identity regression. **Human verification, non-blocking:** product owners may confirm whether Go should receive automatic search eligibility while other third-party providers still require configuration/flags. Intent beyond the code is not proven. If that asymmetry is unwanted, the minimal correction is removing only the Go eligibility arm and its expectation, not renaming the provider or changing Kilo branding. No severity-bearing defect is asserted on that policy question.
|
||||
|
||||
### OAuth identity and retry links survive the touched paths
|
||||
|
||||
- Codex success/error pages still say `Kilo - Codex Authorization Successful`, `You can close this window and return to Kilo.`, and `Kilo - Codex Authorization Failed` at `packages/opencode/src/plugin/openai/codex.ts:176`, `:209`, and `:222`. The callback serves this retained page at `:328`; the comment mentioning an OpenCode-branded *shared* page is not the HTML being served. The authorize request keeps `originator: "kilo"` at `:116`. These Kilo overrides exist in base/main and survive against pristine/transformed upstream controls.
|
||||
- The new `https://api.openai.com/auth` occurrence at `packages/opencode/src/plugin/openai/codex.ts:100` is a JWT claim namespace, not an OpenCode property or a new browser destination.
|
||||
- Existing `opencode` user-agent/originator headers in Codex (`:591`, `:615`, `:674-675`) and Cloudflare (`packages/opencode/src/provider/provider.ts:793`, `:862`; `packages/core/src/plugin/provider/cloudflare-ai-gateway.ts:80`) are transport identity, not newly added UI copy. They predate this PR. Whether every such header should eventually identify Kilo requires provider/protocol compatibility verification; this review does not assume a blind rename is safe.
|
||||
- The unchanged TUI Go-upsell identifiers at `packages/tui/src/routes/session/index.tsx:104-127` do not establish a newly exposed upsell. The consumer requires `status.action` at `:463-475`, while Kilo's `packages/opencode/src/session/retry.ts:85-120` still returns messages without producing upstream Go actions. Its retry-regex changes do not restore the upstream upsell builders. The existing retry dialog's special pricing URL remains `https://kilo.ai/pricing` at `packages/tui/src/component/dialog-retry-action.tsx:10`. The changed reasoning header at `packages/tui/src/routes/session/index.tsx:1876-1880` renders `Thought`, not OpenCode branding.
|
||||
|
||||
### Existing Snowflake instruction is a real residual, but not introduced here
|
||||
|
||||
- **Location:** `packages/opencode/src/plugin/snowflake-cortex.ts:478`:
|
||||
|
||||
> Complete Snowflake sign-in in your browser. OpenCode will capture the OAuth callback and store the bearer token automatically.
|
||||
|
||||
- **Exposure/effect:** The Snowflake plugin is in the existing internal plugin list (`packages/opencode/src/plugin/index.ts:91`); provider login prints automatic authorization instructions at `packages/opencode/src/cli/cmd/providers.ts:103-105`. Thus this text can identify the Kilo client as OpenCode during Snowflake sign-in. It is not merely a provider brand or a dormant documentation mention.
|
||||
- **Control:** The exact instruction exists at the same line in actual base, pinned main, pristine `.18`, pristine `.20`, the recording merge, transformed upstream, and HEAD. The whole Kilo Snowflake file is byte-identical base→HEAD; this PR adds Cerebras to the plugin list, not Snowflake registration.
|
||||
- **Classification:** Static verification; **pre-existing Kilo branding residual inherited from upstream**, low/P3 follow-up, explicitly excluded from this merge's findings/verdict. If addressed separately, the minimal correction is changing only `OpenCode will capture` to `Kilo will capture`. No new OAuth behavior or credential failure is claimed.
|
||||
|
||||
### Public package, SDK, OpenAPI, and help identity are preserved
|
||||
|
||||
- Package identity metadata (`name`, description, version, private flag, bin, repository, homepage, bugs, keywords) is unchanged in all three edited package manifests. Public CLI and SDK remain `@kilocode/cli` / `kilo` (`packages/opencode/package.json:4`, `:20`) and `@kilocode/sdk` with the Kilo repository (`packages/sdk/js/package.json:3`, `:39`). Core's `@opencode-ai/core` name and `opencode` development bin are pre-existing private-package metadata (`packages/core/package.json:4`, `:7`, `:17`), not a new public package rename.
|
||||
- Scanning every changed generated SDK file found identical branding/URL line multisets before and after. The generated API still uses `KiloClient`; its missing-client error explicitly recommends `new KiloClient()` (`packages/sdk/js/src/v2/gen/sdk.gen.ts:666`). Representative descriptions still say `Kilo system`, `Kilo server`, and `Kilo configuration` (`:781`, `:886`, `:1512`). No OpenCode product name or OpenCode web URL was found in the v2 generated tree by the targeted identity scan.
|
||||
- `packages/sdk/openapi.json` is byte-identical to actual base. Config/OpenAPI descriptions were not silently replaced during the SDK generator upgrade. Regeneration freshness and SDK source compatibility are other reviewers' concerns, not inferred from this branding check.
|
||||
- The executable help entrypoint remains `.scriptName("kilo")` (`packages/opencode/src/index.ts:58`). No public docs/help file is changed besides the dormant upstream skill document. Provider IDs, protocol/service identifiers, `@opencode-ai/*` imports, and existing third-party auth-package names were not misclassified as Kilo product names.
|
||||
|
||||
### Exact prompt text and meaningful behavior exposure
|
||||
|
||||
The sole changed prompt/skill-document literal is the global-config table cell at `packages/core/src/plugin/skill/customize-opencode.md:43` (table padding omitted, text preserved exactly):
|
||||
|
||||
```text
|
||||
Before: `~/.config/opencode/opencode.json` (NOT `~/.opencode/`)
|
||||
After: `~/.config/opencode/opencode.json` or `~/.config/opencode/opencode.jsonc` (NOT `~/.opencode/`)
|
||||
```
|
||||
|
||||
This document remains unregistered as established above. No active system-prompt or tool-description wording was changed. That does **not** mean model-visible behavior is identical:
|
||||
|
||||
- OpenCode Go sessions gain the existing `websearch` description/schema through the provider gate; Kilo sessions retain their existing eligibility.
|
||||
- `packages/opencode/src/tool/task.ts:276-278` now propagates the last failed child tool as a task error, rather than returning only final child text when no assistant-level error exists. The new output template is exactly `` `${failed.state.error}\n${resumeHint(nextSession.id)}` ``. Its already-existing hint (`packages/opencode/src/kilocode/task-resume.ts:3-4`) is exactly `This subagent session can be resumed: call the task tool again with task_id="${sessionID}" and a prompt describing how to continue or recover. Its prior context is preserved.` This is newly exposed error/recovery context, not an OpenCode identity instruction.
|
||||
- Core runner affinity headers and compaction HTTP propagation change request metadata, not system/summary wording (`packages/core/src/session/runner/llm.ts:207-214`, `packages/core/src/session/compaction.ts:205`). Retry changes and the new `Provider finish_reason: network_error` diagnostic (`packages/opencode/src/session/llm/ai-sdk.ts:94`) alter recovery/error exposure, not product branding. Provider-returned arbitrary text is not treated as an authored Kilo→OpenCode replacement.
|
||||
|
||||
## Command outputs and verification
|
||||
|
||||
All commands ran with workdir `/Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports`.
|
||||
|
||||
```text
|
||||
git status --short # initial: no output
|
||||
git rev-parse HEAD
|
||||
6a7d6bc002319ac2987bcde3d6c63efcafc07021
|
||||
|
||||
git merge-base bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD
|
||||
bf1cf502a3c511e9daf6a43244568ae4e83473a8
|
||||
|
||||
git diff --stat bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD
|
||||
59 files changed, 1524 insertions(+), 647 deletions(-)
|
||||
|
||||
git rev-list --count bf1cf502a3c511e9daf6a43244568ae4e83473a8..HEAD
|
||||
95
|
||||
|
||||
git log --first-parent --merges --format='%H %P %s' bf1cf502a3c511e9daf6a43244568ae4e83473a8..HEAD
|
||||
6a7d6bc002319ac2987bcde3d6c63efcafc07021 91ca95bad927436131ea4783a470885a381ce6ad 9563af96a012effc25df5a11eaa1f7633161a742 resolve merge conflicts
|
||||
91ca95bad927436131ea4783a470885a381ce6ad bf1cf502a3c511e9daf6a43244568ae4e83473a8 7248bc1964b13fa67e601733f89ee9dc6dfa0563 merge: record upstream v1.18.20
|
||||
```
|
||||
|
||||
Read-only Python scanners consumed `git diff --unified=0` and `git show HEAD:<path>` for every changed path. Added/deleted-line scan for `opencode|anomalyco|sst.dev|https?://|You are|instructions|description`: **33 matching lines**, including tests and removed lines. Full changed-file OpenCode scan: **451 matching lines**, of which the mechanical import/dependency filter identified 327 and the remaining test/fixture filter identified 35; the other 89 contextual matches were inspected for exposure. These are line counts, not defect counts.
|
||||
|
||||
The read-only Git-blob assertion run exited 0. Selected stdout (the omitted lines are individual unchanged-file confirmations):
|
||||
|
||||
```text
|
||||
PASS: all 59 changed paths enumerated and scanned
|
||||
PASS: account console and customize-opencode absent from production registration; kilo-config retained
|
||||
PASS: committed OpenAPI byte-identical to actual base
|
||||
PASS: package identity metadata unchanged: packages/core/package.json
|
||||
PASS: package identity metadata unchanged: packages/opencode/package.json
|
||||
PASS: package identity metadata unchanged: packages/sdk/js/package.json
|
||||
PASS: branding/URL line multisets unchanged in all 10 changed generated SDK files
|
||||
PASS: Snowflake instruction is pre-existing in actual base and pinned main
|
||||
Static checks only; no runtime initialization or filesystem writes performed
|
||||
```
|
||||
|
||||
Final `git diff --exit-code` and `git diff --cached --exit-code` both exited 0. Status contained only untracked `.review-config-r6/` (other parallel work) and this report.
|
||||
|
||||
`gh pr view 13513 --repo Kilo-Org/kilocode --json number,headRefOid,baseRefName,baseRefOid,mergeable,mergeStateStatus` returned the exact reviewed head/base, `MERGEABLE`, and `CLEAN`. The final local HEAD recheck also remained `6a7d6bc002319ac2987bcde3d6c63efcafc07021`.
|
||||
|
||||
## Limitations and integrity
|
||||
|
||||
- Static branding/registration review only: no live OAuth, browser/TUI smoke, model request, dependency install, SDK generation, lint, typecheck, or application test suite was run. Only this report was authored; running application/test initialization could create state outside the report, contrary to this reviewer's write restriction. These checks establish source reachability and provenance, not full runtime correctness.
|
||||
- No live models.dev/provider catalog capture, external service redirect check, arbitrary user-plugin evaluation, or independent remote-tag refetch was performed. External provider names/instructions can remain visible intentionally. Provider/header product-policy ambiguity is explicitly left for human verification, not promoted to a regression.
|
||||
- No exhaustive repo-wide pre-existing-branding cleanup or full-PR correctness verdict is claimed. Other reviewers own configuration, pipeline, marker, and infrastructure lenses. CI suite results and resolution-tool/rerere accounting were not independently audited by this lens.
|
||||
- The checkout was initially clean. A later status check showed an externally created untracked `.review-config-r6/` during parallel work; it was not created, read, edited, or removed by this reviewer. No tracked source changes were made. The only reviewer-authored file is `OPENCODE_MENTIONS.md`.
|
||||
- No caller-checkout access/modification, source edits, commits, pushes, branch/ref changes, Git configuration edits, GitHub mutations, real user-state access, or credential access. No diagnostic files were created or required cleanup.
|
||||
@@ -0,0 +1,199 @@
|
||||
# Tests review — PR #13513
|
||||
|
||||
## Scope and method
|
||||
|
||||
Reviewer 7 of 7; this report covers removal, weakening, or disconnection of Kilo-specific test coverage, not the overall merge verdict. All commands ran inside `/Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports` or its package directories. No commands ran in the caller worktree.
|
||||
|
||||
Reviewed exact HEAD `6a7d6bc002319ac2987bcde3d6c63efcafc07021` against actual stacked base / merge base `bf1cf502a3c511e9daf6a43244568ae4e83473a8` (`johnnyeric/kilo-opencode-v1.18.18`). Controls were pinned main `62998965e9fb0d9ed89011c62498b39801dbbb4f`, pristine upstream v1.18.18 `31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d`, v1.18.19 `2b72179c663cadcb54f54d9f19221b3fb3d11fb6`, and v1.18.20 `7248bc1964b13fa67e601733f89ee9dc6dfa0563`.
|
||||
|
||||
Read root `AGENTS.md`, `REVIEW.md`, `TESTING.md`, the upstream-review command, CLI/package/test/server-test instructions, test preloads and runner selection, and the requested skills. Obtained full changed/deleted/renamed path inventories; inspected all 18 changed test-file diffs, including every removed line in seven files; compared upstream and both merge parents; enumerated Kilo-named test/fixture blobs; and checked shared tests containing Kilo identifiers, fixtures, imports, or markers. Inspected main-only missing tests separately so unrelated main development was not attributed to this stacked PR. Ran focused existing tests using the parent-installed dependencies, without installing or editing source.
|
||||
|
||||
Verified inventory:
|
||||
|
||||
- Entire PR: **59 files = 3 added / 56 modified; zero deleted or renamed**. **95 reachable commits; two first-parent merges**.
|
||||
- Test delta: **18 files = 2 added / 16 modified; 902 inserted / 140 deleted lines**. The two additions are Cerebras and provider-stream-error tests.
|
||||
- **1,243 tracked Kilo-named test/fixture paths from the actual base remain byte-identical at HEAD**. This is a broad path inventory, including fixtures and support files, not a count of executable tests.
|
||||
- Thirteen modified shared test files contain Kilo identifiers in their base contents; those were not excluded merely because their paths are upstream-owned.
|
||||
- Upstream merge `91ca95bad927436131ea4783a470885a381ce6ad` has parents actual base and pristine v1.18.20. HEAD has parents that merge and transformed upstream `9563af96a012effc25df5a11eaa1f7633161a742`. All 18 final test changes are present in the first-parent-to-HEAD adaptation delta.
|
||||
|
||||
## Findings and scoped verdict
|
||||
|
||||
**No confirmed Kilo-specific test removal, assertion weakening, or scheduling disconnection introduced by this PR. Safe to merge for this review lens.** No severity-ranked remediation is required by this audit; other reviewers own product correctness and the overall merge verdict.
|
||||
|
||||
## Notable non-findings
|
||||
|
||||
### 1. The upstream “remove flaky subagent test” commit does not remove a test from the final PR delta
|
||||
|
||||
`62cb3f77bd2b4eb3721f286022066de1abe04432` removes the 44-line `answers requested permissions from subagents` case from `packages/opencode/test/cli/run/run-process.test.ts`. That case was first added by upstream `08faeb3893` inside the same v1.18.18 → v1.18.20 range. It is absent from both pristine endpoints and from both Kilo endpoints; the pristine file blobs match each other, and the Kilo base/HEAD blobs match each other.
|
||||
|
||||
The removed intermediate case has no Kilo-specific assertions or fixtures. Kilo's existing auto-reject/nonzero-exit, dangerous-flag, and explicit-deny coverage remains at `packages/opencode/test/cli/run/run-process.test.ts:327-361`, unchanged. **Provenance: temporary upstream-only addition and removal; not a final-diff Kilo coverage regression.** This does not claim the surviving parent-permission test is equivalent to the removed subagent subprocess test; that heavier end-to-end path was not run here.
|
||||
|
||||
### 2. Cloudflare assertion removals follow an upstream API replacement, not removal of Kilo coverage
|
||||
|
||||
The four old max-output-token hook tests at base `packages/opencode/test/plugin/cloudflare.test.ts:43-68` are replaced by auth registration and explicit absence-of-hook assertions at HEAD `packages/opencode/test/plugin/cloudflare.test.ts:16-25`. The production `chat.params` hook itself is removed: OpenAI uses native Responses passthrough rather than the unified chat-completions workaround (`packages/opencode/src/provider/provider.ts:874-889`).
|
||||
|
||||
The old wire assertions are replaced rather than silently dropped: Responses reasoning effort/summary are asserted at `packages/opencode/test/provider/cf-ai-gateway-e2e.test.ts:208-237`; the compatibility-route `reasoning_effort` assertion and wrong-key negative control remain for Workers AI at lines 258-273. Native Anthropic and token-scoping checks are added at lines 240-255 and 277-306. All 12 tests executed, with 32 assertions.
|
||||
|
||||
**Provenance control:** both test files are byte-identical across base/main/pristine v1.18.18, and their HEAD versions are byte-identical to pristine v1.18.20. No Kilo overlay was removed. The helper still reconstructs gateway routing rather than calling production `Provider.getModel`; see limitations below.
|
||||
|
||||
### 3. Shared Kilo Codex, websearch, task, and credential assertions survive
|
||||
|
||||
- `packages/opencode/test/plugin/codex.test.ts:138-214`: the Kilo OAuth model-filter block changes only formatting/trailing commas. All **nine** filter assertions and their model fixtures are preserved. The refresh assertion at line 502 replaces a literal access token with a generated JWT and adds residency checks; it does not stop checking the refreshed token or request authorization. The model-filter and surrounding plugin tests executed successfully.
|
||||
- `packages/opencode/test/tool/websearch.test.ts:33-40`: the test is renamed and an `opencode-go` case is added, but the Kilo-enabled and OpenCode-disabled assertions remain exactly as before. No skip or eligibility gate was added. The full file executed successfully.
|
||||
- `packages/opencode/test/tool/task.test.ts:103-126`: the Kilo prompt stub still persists assistant cost through the real session service. New optional error fixtures do not replace the existing cost branch. Forked-session resume, platform attribution, failure resume hints, background/extended costs, delta-only costs, and partial-abort costs remain. The two new cases at lines 398-489 retain upstream child-error checks while asserting Kilo's resumable `task_id` message. Thirteen selected tests executed, including the preserved Kilo coverage; 19 unrelated cases were deliberately filtered by this review command, not skipped by the PR.
|
||||
- `packages/opencode/test/server/httpapi-provider.test.ts:354-383`: the Kilo-auth fixture is unchanged and gains an assertion that persisted Google credentials appear in `connected`; existing response, nonserialized-fetch, and nonzero-cost assertions remain. Static verification only in this lens.
|
||||
|
||||
**Provenance:** existing Kilo coverage preserved; upstream additions adapted to Kilo error text, plus an additional Kilo credential assertion. No new `skip`, `only`, `todo`, timeout, environment gate, or `mock.module` change occurs in the actual PR test hunks.
|
||||
|
||||
### 4. Apparent missing main tests are not PR deletions
|
||||
|
||||
Comparing pinned main directly to HEAD produces **13 absent test/fixture paths**: 12 tests and one worktree-reference fixture. Every one is also absent from the actual stacked base. Their addition history belongs to unrelated main commits, including `62998965e9` (plan-mode ruleset stacking), `45202c0764` (snapshot cleanup/auth and provider lifecycle), `648fa0a6a7` (failed-turn retry), `13a9673d08` (worktree references/recency), and other JetBrains/VS Code UI work. They must not be described as tests removed by this PR.
|
||||
|
||||
The more subtle shared-file case is pinned main's `packages/opencode/test/tool/task.test.ts:503-568`, which checks that synthetic/ignored trailing text does not replace the real subagent answer. Its three output assertions were added by main commit `bf7848cb48` and are already absent at the actual base. Likewise, compaction's `<previous-summary>` and retry's two-argument delay assertions differ between main and the actual base before this PR; the base has alternative compaction assertions and explicit deterministic jitter arguments. **Provenance: pre-existing stack/main divergence, not a merge-caused test loss.** Reconcile unrelated main changes in their appropriate later integration, not as a purported deletion fix here.
|
||||
|
||||
### 5. The omitted new upstream projector test conflicts with Kilo's retained compatibility contract
|
||||
|
||||
Pristine v1.18.20 adds `projects moved sessions without the transitional context epoch table` at upstream `packages/core/test/session-projector.test.ts:48-79`; it explicitly drops `session_context_epoch`. Kilo's projector test file is byte-identical across actual base, pinned main, and HEAD. Kilo still retains and resets that table for released-client compatibility (`packages/core/src/session/projector.ts:16,276,475`), and existing compatibility coverage still asserts released writes at `packages/core/test/kilocode/database-migration-compat.test.ts:137-181`.
|
||||
|
||||
**Provenance: a new upstream-only test not adopted for a deliberately different Kilo schema contract; no pre-existing Kilo test removed or weakened.** Static compatibility rationale only; this report does not validate the complete migration policy.
|
||||
|
||||
### 6. Test discovery and CI eligibility are preserved
|
||||
|
||||
The actual PR changes no workflows, runner/profile/shard helpers, preloads, Bun test configuration, or package test scripts. CLI package changes are dependency versions, not scheduling changes. `packages/opencode/script/test-runner.ts:154-186` still discovers `test/**/*.test.{ts,tsx}`, selects all tests by default, and retains its pre-existing OAuth-browser exclusion only. New Cerebras/error files match discovery. The runner's unsafe-file demotion changes isolation, not whether those files run (`packages/opencode/script/test-runner.ts:343-375`).
|
||||
|
||||
`.github/workflows/test.yml:212-228` continues to use the Kilo CLI `test:ci` runner, its existing Darwin profile, and full Linux/Windows shards. No new platform/profile gate disconnects Kilo tests. This was a static scheduling audit, not a complete CI execution.
|
||||
|
||||
## Command evidence and outputs
|
||||
|
||||
### Git inventories and controls
|
||||
|
||||
Commands below ran at the isolated review root. All Git comparisons use immutable commits, not a moving local `main`.
|
||||
|
||||
```sh
|
||||
git diff --name-status --find-renames bf1cf502a3c511e9daf6a43244568ae4e83473a8 6a7d6bc002319ac2987bcde3d6c63efcafc07021
|
||||
git diff --diff-filter=DR --name-status --find-renames bf1cf502a3c511e9daf6a43244568ae4e83473a8 6a7d6bc002319ac2987bcde3d6c63efcafc07021
|
||||
git merge-base bf1cf502a3c511e9daf6a43244568ae4e83473a8 6a7d6bc002319ac2987bcde3d6c63efcafc07021
|
||||
```
|
||||
|
||||
The deletion/rename command produced **no output**; the merge-base command returned:
|
||||
|
||||
```text
|
||||
bf1cf502a3c511e9daf6a43244568ae4e83473a8
|
||||
```
|
||||
|
||||
Read-only Python wrappers over `git diff`, `git ls-tree -r`, `git show`, and `git rev-parse` produced these exact summary/control outputs:
|
||||
|
||||
```text
|
||||
All changed paths: {'M': 56, 'A': 3}
|
||||
Test paths: {'M': 16, 'A': 2}
|
||||
Reachable commits: 95
|
||||
First-parent merges: 2
|
||||
Kilo-named tracked test/fixture paths at base: 1243
|
||||
Absent at HEAD: 0
|
||||
Changed at HEAD: 0
|
||||
Main-only absent test paths: 13
|
||||
Codex Kilo model-filter assertions base/head: 9 9
|
||||
Assertions byte-identical: True
|
||||
Whole Kilo block identical ignoring whitespace/trailing commas: True
|
||||
Flaky subagent case present at base/head: [False, False]
|
||||
Retained Kilo auto-reject case present at base/head: [True, True]
|
||||
Changed test-line totals: 18 files changed, 902 insertions(+), 140 deletions(-)
|
||||
```
|
||||
|
||||
The formatting normalization above ignores whitespace and trailing commas, not assertion values or fixture identifiers. The assertion comparison is independently byte-identical.
|
||||
|
||||
```sh
|
||||
git show --stat --oneline 62cb3f77bd
|
||||
```
|
||||
|
||||
```text
|
||||
62cb3f77bd test(opencode): remove flaky subagent test (#43819)
|
||||
packages/opencode/test/cli/run/run-process.test.ts | 44 ----------------------
|
||||
1 file changed, 44 deletions(-)
|
||||
```
|
||||
|
||||
The complete removal hunk and its earlier addition were inspected. Endpoint blob control:
|
||||
|
||||
```sh
|
||||
git rev-parse bf1cf502a3c511e9daf6a43244568ae4e83473a8:packages/opencode/test/cli/run/run-process.test.ts 6a7d6bc002319ac2987bcde3d6c63efcafc07021:packages/opencode/test/cli/run/run-process.test.ts 31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d:packages/opencode/test/cli/run/run-process.test.ts 7248bc1964b13fa67e601733f89ee9dc6dfa0563:packages/opencode/test/cli/run/run-process.test.ts
|
||||
```
|
||||
|
||||
```text
|
||||
5841afa60432b9409835f944586d2ded4d654e8a
|
||||
5841afa60432b9409835f944586d2ded4d654e8a
|
||||
bd5847e2723cfca944ef6f97e6d6fe3ff986c042
|
||||
bd5847e2723cfca944ef6f97e6d6fe3ff986c042
|
||||
```
|
||||
|
||||
### Focused execution
|
||||
|
||||
All commands below ran from `packages/opencode/` in the isolated review tree, with Bun **1.3.14 (0d9b296a)**. The unchanged test preload redirects XDG/test home and enforces an in-memory database. No live provider credentials or model calls were needed.
|
||||
|
||||
```sh
|
||||
bun test ./test/tool/websearch.test.ts ./test/plugin/cloudflare.test.ts ./test/plugin/codex.test.ts
|
||||
```
|
||||
|
||||
```text
|
||||
bun test v1.3.14 (0d9b296a)
|
||||
INFO 2026-08-27T14:16:39 +892ms service=plugin.codex refreshing codex access token
|
||||
|
||||
38 pass
|
||||
0 fail
|
||||
78 expect() calls
|
||||
Ran 38 tests across 3 files. [1.51s]
|
||||
```
|
||||
|
||||
The refresh log is from the test's local fake OAuth server, not a real account refresh.
|
||||
|
||||
```sh
|
||||
bun test ./test/provider/cf-ai-gateway-e2e.test.ts
|
||||
```
|
||||
|
||||
```text
|
||||
bun test v1.3.14 (0d9b296a)
|
||||
|
||||
12 pass
|
||||
0 fail
|
||||
32 expect() calls
|
||||
Ran 12 tests across 1 file. [1358.00ms]
|
||||
```
|
||||
|
||||
```sh
|
||||
bun test ./test/tool/task.test.ts --test-name-pattern 'resumable|platform attribution|forked|cost propagation|child cost|extended run|experiment is disabled|child prompt returns assistant error'
|
||||
```
|
||||
|
||||
```text
|
||||
bun test v1.3.14 (0d9b296a)
|
||||
|
||||
13 pass
|
||||
19 filtered out
|
||||
0 fail
|
||||
34 expect() calls
|
||||
Ran 13 tests across 1 file. [4.01s]
|
||||
```
|
||||
|
||||
**Total: 63 passing tests, 144 `expect()` calls, zero failures; 19 intentionally filtered cases.** There were no failing tests requiring an isolated rerun or base failure-control execution. Historical controls were Git-object comparisons, not execution of historical checkouts.
|
||||
|
||||
### Final report validation
|
||||
|
||||
```sh
|
||||
bun run script/check-md-table-padding.ts && git diff --exit-code && git rev-parse HEAD && git status --short
|
||||
```
|
||||
|
||||
```text
|
||||
check-md-table-padding: 403 file(s) checked, no padded tables found.
|
||||
6a7d6bc002319ac2987bcde3d6c63efcafc07021
|
||||
?? .review-config-r6/
|
||||
?? OPENCODE_MENTIONS.md
|
||||
?? TESTS.md
|
||||
```
|
||||
|
||||
The tracked diff was empty. Other reviewers' untracked artifacts were left untouched.
|
||||
|
||||
## Limitations and integrity
|
||||
|
||||
- Static removal audit was primary. Did not run full package suites, subprocess permission tests, HTTP API exercisers, SDK generation/builds, lint/typecheck, or platform matrices; no source implementation was changed, and heavy pipeline verification belongs to other reviewers.
|
||||
- Cloudflare tests use real transforms and SDK serialization but reconstruct routing in `gatewayModel` / `cfNpm` and stub the network boundary. They prove the stated wire assertions execute, not that every production routing/configuration path is covered. This limitation is not evidence of a removed Kilo test.
|
||||
- Task tests use the existing prompt-operation stub while exercising the real task/session boundary. Their passing assertions do not substitute for the removed intermediate upstream subagent subprocess scenario.
|
||||
- Local pinned refs and ancestry were verified. This lens did not independently fetch authoritative tags, refresh GitHub CI, or re-resolve a moving PR head; the parent reviewer owns those checks. Conclusions apply only to the exact reviewed SHA.
|
||||
- Initial tracked and untracked status was clean. No source edits, dependency installs, commits, pushes, GitHub mutations, branch switches, or git-config changes were made by this reviewer. Only `TESTS.md` was intentionally written. A concurrent reviewer's `.review-config-r6/` directory appeared during review and was left untouched; shared-worktree untracked files are not evidence of this review modifying source.
|
||||
@@ -0,0 +1,154 @@
|
||||
# Unnecessary markers — PR #13513
|
||||
|
||||
**Verdict: safe to merge for this review lens.** No changed file is upstream-identical except for markers. There are low-priority stale annotations, including one import newly made redundant by upstream and an existing stale block expanded around new upstream code. Do not apply the bulk reset proposals blindly: two would remove intentional Kilo behavior.
|
||||
|
||||
## Scope and method
|
||||
|
||||
- Reviewer 4/7; sole output: `UNNECESSARY_MARKERS.md`.
|
||||
- Checkout: `/Users/johnnyamancio/orca/workspaces/kilocode/review-pr-13513-reports`. All commands ran there; the caller checkout was not modified.
|
||||
- HEAD: `6a7d6bc002319ac2987bcde3d6c63efcafc07021`.
|
||||
- PR base and verified merge base: `bf1cf502a3c511e9daf6a43244568ae4e83473a8` (`johnnyeric/kilo-opencode-v1.18.18`). Full PR scope: **59 files, 1,524 insertions, 647 deletions**, 95 reachable commits beyond base.
|
||||
- Main control: `62998965e9fb0d9ed89011c62498b39801dbbb4f`.
|
||||
- Authoritative upstream controls: v1.18.18 `31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d`; v1.18.19 `2b72179c663cadcb54f54d9f19221b3fb3d11fb6`; v1.18.20 `7248bc1964b13fa67e601733f89ee9dc6dfa0563`.
|
||||
- HEAD parents: Kilo `91ca95bad927436131ea4783a470885a381ce6ad`, transformed upstream `9563af96a012effc25df5a11eaa1f7633161a742`.
|
||||
- `.opencode-version` contains `v1.18.20`; the local tag resolves to the authoritative SHA, which is an ancestor of HEAD. No fallback fetch was necessary. Authoritative remote fetching was performed by the parent reviewer; this lens independently checked local refs and ancestry.
|
||||
|
||||
Read root `AGENTS.md`, `REVIEW.md`, `script/upstream/README.md`, applicable package instructions, both required skills, and the reset scripts and helpers before executing them. `find-reset-candidates.ts:386` gates writes behind `!opts.dryRun`; `utils/reset.ts:43,55,67` returns before deletion or writes on dry runs. The diagnostic scripts are unchanged by this PR.
|
||||
|
||||
The bulk output was intersected with the **entire 59-file PR list**, not just files containing newly added markers. All 29 changed marker-bearing files were then compared in memory against both (a) the scripts' freshly translated pristine upstream and (b) the actual transformed merge parent. The scan examined **184 parsed marker blocks and 338 inline markers**, followed by inspection of 24 standalone marker comments and contextual verification of equality candidates. It used order-sensitive `difflib.SequenceMatcher(..., autojunk=False)`, not the bulk tool's line-multiset heuristic. No temporary source files were created.
|
||||
|
||||
## Findings
|
||||
|
||||
### P3 — Upstream adoption leaves misleading Kilo ownership annotations
|
||||
|
||||
**Locations:** `packages/opencode/test/tool/task.test.ts:6`; `packages/opencode/src/cli/cmd/run.ts:802-807`.
|
||||
|
||||
1. **Newly obsolete import marker.** The task test retains `// kilocode_change - Cause for resume-hint coverage` on `import { Cause, Deferred, Effect, Exit, Fiber, Layer } from "effect"`. Upstream v1.18.18 did not import `Cause`; v1.18.20 and transformed parent now have exactly this complete import. Base and main contain the Kilo marker, so its redundancy is caused by this upstream range, not a newly written comment. The tests still have genuine Kilo resume-hint assertions, but that no longer makes the shared import a fork difference.
|
||||
2. **Existing stale block grows around new upstream code.** `run.ts:802` says “revert to upstream: consume native events without normalizing sync copies.” The enclosed loop header was already upstream-identical in base/main. The merge adds upstream's three-line `session.created` descendant-tracking branch inside that marker. The complete four-line body at HEAD lines 803–806 matches v1.18.20 and the transformed parent verbatim. Base/main contain only the old loop-header marker; their bodies lack the newly added descendant branch.
|
||||
|
||||
**Maintenance impact:** These annotations falsely identify upstream-owned imports and descendant tracking as Kilo behavior to preserve during subsequent merges. This creates avoidable conflict noise and makes a future upstream update look like a fork behavior change.
|
||||
|
||||
**Minimal direction:** Remove only the import suffix and the `run.ts` block's two marker comments. Preserve the import, loop, descendant tracking, and surrounding genuine Kilo auto-approval/suggestion logic. This is non-blocking maintenance, not a runtime regression.
|
||||
|
||||
### P3 — Additional pre-existing stale annotations remain in changed files
|
||||
|
||||
These are **pre-existing Kilo maintenance debt**, not regressions introduced by PR #13513. Base/main retain the same annotations, while the referenced code already exists in pristine v1.18.18 and remains in v1.18.20/transformed upstream:
|
||||
|
||||
- `packages/opencode/src/tool/task.ts:286-315`: the entire 28-line `TaskTool.injectBackgroundResult` body is upstream-identical. This is a redundant block even though the rest of the file retains substantial Kilo cost, sandbox, lifecycle, and resume-hint behavior.
|
||||
- `packages/opencode/src/plugin/openai/codex.ts:413`: the `DISALLOWED_MODELS` rejection line is identical. The adjacent optional `model.options?.reasoningMode` guard is a real difference and should remain annotated.
|
||||
- `packages/opencode/src/provider/provider.ts:1338,1342`: the `reasoningVariants` fallback and `mapValues` assignment are identical. The preceding `patchKiloModel` call is genuinely Kilo-specific and must remain.
|
||||
- `packages/opencode/src/tool/registry.ts:281`: the conditional `execute: Tool.init(codeModeTool)` registration is identical. Kilo's instance-scoped initialization and added tool registrations elsewhere still differ.
|
||||
- `packages/opencode/test/tool/task.test.ts:16`: the full `MessageID, PartID, SessionID` import already exists upstream; the “SessionID used by cost propagation tests” suffix is stale.
|
||||
- `packages/opencode/test/session/compaction.test.ts:1337`: `{ timeout: 10_000 }` already matches upstream. Preserve the neighboring Kilo snapshot-isolation changes.
|
||||
- `packages/opencode/test/session/processor-effect.test.ts:1207`: the trailing marker on `)` is redundant; as a stronger control than matching punctuation, the complete preceding provider-executed-error test at lines 1161–1206 matches pristine upstream and transformed upstream after marker removal.
|
||||
|
||||
**Evidence/control:** Exact cleaned block matching and contextual diffs against base, main, pristine v1.18.18/v1.18.20, and transformed parent. Both fully redundant blocks and all eight unchanged inline candidates were verified against the actual transformed parent; the previous finding accounts for one block and one inline candidate.
|
||||
|
||||
**Maintenance impact and direction:** Reduce false fork-ownership signals by deleting only these stale comments. Do not reset any of these whole files or remove the neighboring Kilo implementations. These inherited cleanups need not block this incremental merge.
|
||||
|
||||
## Unsafe reset proposals — not upstream-identical files
|
||||
|
||||
The bulk tool proposes five changed files, all in `small-diff`; **none is `markers-only` or `cosmetic-only`**. All five were verified with the required per-file dry run. Those commands only say “Would reset”; they do not prove safety. Comparing their proposed content against actual transformed upstream gives:
|
||||
|
||||
| Candidate | Bulk count | Verified difference and disposition |
|
||||
|---|---|---|
|
||||
| `packages/core/src/session/compaction.ts:229` | 1 | Reset deletes `include: selected.recent`, an intentional compatibility field in emitted compaction events. Preserve it and its marker. |
|
||||
| `packages/opencode/test/plugin/openai-ws.test.ts:545,596` | 3 | Reset changes the first explicit `idleTimeout: 100` back to `20` and removes the second explicit `100`. These are real test timing controls, not whitespace/markers. Preserve pending an independent timing decision. |
|
||||
| `packages/opencode/src/control-plane/workspace.ts:5` | 2 | Reset restores an unused `FetchHttpClient` import. This file has no Kilo markers and is not evidence of unnecessary annotations. |
|
||||
| `packages/sdk/js/src/v2/gen/core/types.gen.ts:65` | 2 | Only generated comment punctuation differs: `e.g.` versus `e.g.,`. No markers. Follow the generator rather than manually resetting generated output. |
|
||||
| `packages/sdk/js/src/v2/gen/core/utils.gen.ts:126` | 2 | Only generated comment punctuation differs: `i.e.` versus `i.e.,`. No markers. Same generator caveat. |
|
||||
|
||||
**Compatibility control:** Base contains `include` at compaction line 228 and main at line 222. `packages/schema/src/session-event.ts:429` explicitly preserves this optional field; `packages/core/test/kilocode/event-storage-compat.test.ts:88,105-107` covers persistence of the version-1 event's `include` field. Resetting the producer would remove a compatibility output, even though the bulk heuristic counts only one line. This lens did not execute a historical-client downgrade test and does not claim a newly reproduced data-loss scenario.
|
||||
|
||||
**Timing control:** Base/main both have the two explicit `idleTimeout: 100` settings at lines 520 and 571. The current PR adds unrelated residency/large-payload tests; it does not make the timing settings upstream-equivalent. No CI-flake reproduction was attempted, so the precise benefit of 100 ms is not independently quantified here.
|
||||
|
||||
**Provenance:** The reset heuristic and both intentional deltas predate this PR. The generator is upgraded from `@hey-api/openapi-ts` 0.90.10 to 0.97.3 in `packages/sdk/js/package.json:26`; the punctuation-only differences are part of that generated delta, not stale markers.
|
||||
|
||||
## Notable non-findings
|
||||
|
||||
- **Zero whole-file unnecessary-marker candidates in the PR.** The repository-wide bulk scan finds 40 `markers-only` files and two `cosmetic-only` files, but none of those 42 files is in this PR's 59-file delta.
|
||||
- **Six apparent inline matches are translation artifacts, not redundant against the actual merge parent.** The referer headers at `packages/opencode/src/provider/provider.ts:491,502,512,523,638,908` become `https://kilo.ai/` when the per-file translator runs. The actual transformed parent still has `https://opencode.ai/` at those sites. They were therefore excluded from the stale-marker findings: removing them solely on the translator's result would hide a real diff against the merge input.
|
||||
- **Genuine terminal-task adaptation remains.** The new task test block at `packages/opencode/test/tool/task.test.ts:398-487` intentionally asserts Kilo's resume-hint format rather than upstream's exact failure message. The corresponding runtime errors append `resumeHint`. These blocks are not wholly redundant despite containing substantial upstream test structure.
|
||||
- The 24 standalone comments include annotations whose local next line happens to match upstream but whose control flow differs: for example `packages/tui/src/routes/session/index.tsx:441` follows removal of upstream's `plan_exit` auto-switch branch, and `packages/opencode/src/provider/provider.ts:1678` follows a loop extended with Kilo custom loaders. They are not treated as marker-only code from a one-line match.
|
||||
|
||||
## Commands and observed outputs
|
||||
|
||||
All commands below ran from the isolated report checkout. The SHA aliases used in prose above were not branch substitutions in the comparisons.
|
||||
|
||||
### Baselines and scope
|
||||
|
||||
```sh
|
||||
git rev-parse HEAD
|
||||
git diff --name-status bf1cf502a3c511e9daf6a43244568ae4e83473a8 6a7d6bc002319ac2987bcde3d6c63efcafc07021
|
||||
git show HEAD:.opencode-version
|
||||
git rev-parse 'v1.18.20^{commit}' refs/review/pr-13513/upstream-v1.18.18 refs/review/pr-13513/upstream-v1.18.19 refs/review/pr-13513/upstream-v1.18.20
|
||||
git merge-base bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD
|
||||
git show -s --format='%H %P' HEAD
|
||||
git merge-base --is-ancestor 7248bc1964b13fa67e601733f89ee9dc6dfa0563 HEAD
|
||||
git diff --stat bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD
|
||||
git rev-list --count bf1cf502a3c511e9daf6a43244568ae4e83473a8..HEAD
|
||||
git diff --quiet bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD -- script/upstream
|
||||
git diff --name-only -z bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD | xargs -0 git grep -n 'kilocode_change' HEAD --
|
||||
```
|
||||
|
||||
Results: exact expected HEAD/base/parents, authoritative upstream SHAs, ancestry success, 59 changed paths, 95 commits, no PR change to the diagnostics. Marker search identified 29 changed marker-bearing files.
|
||||
|
||||
### Required bulk check
|
||||
|
||||
```sh
|
||||
bun run script/upstream/find-reset-candidates.ts --dry-run
|
||||
```
|
||||
|
||||
A complete successful invocation produced the following summary against **v1.18.20 / `7248bc19`**, scope **all shared paths**, default review limit **5**:
|
||||
|
||||
| Bucket | Repository count | Intersection with full PR delta |
|
||||
|---|---|---|
|
||||
| markers-only | 40 | 0 |
|
||||
| cosmetic-only | 2 | 0 |
|
||||
| small-diff | 221 | 5 |
|
||||
| large-diff | 541 | 40 |
|
||||
| identical | 167 | 4 |
|
||||
| upstream-missing | 384 | 1 (`.opencode-version`) |
|
||||
| local-missing | 2 | 0 |
|
||||
| Total classified/pre-bucketed | 1,357 | 50 |
|
||||
| Non-code assets skipped | 342 | 1 (`bun.lock`) |
|
||||
| Config-protected skipped | 2,187 | 0 |
|
||||
|
||||
The remaining eight PR paths are seven raw-upstream-identical paths omitted by the bulk prefilter and one excluded Kilo-owned path. The completed report's entries were intersected using a content-search expression enumerating the full changed-path set: **50 matches**, including exactly the five reset proposals listed above. The independent classifier pass over all 29 marker-bearing files returned **27 large-diff, two small-diff, zero marker-only/cosmetic-only**.
|
||||
|
||||
The successful full output is retained in the local tool artifact `/Users/johnnyamancio/.local/share/kilo/tool-output/tool_043945176001J2waYZ7KDJ9ywl` (1,438 lines; summary at lines 37–59). This report records its complete bucket counts rather than embedding the repository-wide inventory.
|
||||
|
||||
**Execution limitation:** The first invocation timed out at 120 seconds. A retry with a 300-second allowance completed and produced the output above. A later captured-output rerun, and the final resume-time rerun, timed out at 300 seconds after logging `Classified 973/973`, without reaching the summary. That progress message is not proof that every concurrent worker completed. The completed invocation—not those timed-out runs—is the source of the counts. Dependencies resolved; no installation was attempted. The intermittent hang's cause was not diagnosed.
|
||||
|
||||
### Required per-candidate checks
|
||||
|
||||
```sh
|
||||
bun run script/upstream/reset-to-upstream.ts packages/core/src/session/compaction.ts --dry-run
|
||||
bun run script/upstream/reset-to-upstream.ts packages/opencode/src/control-plane/workspace.ts --dry-run
|
||||
bun run script/upstream/reset-to-upstream.ts packages/opencode/test/plugin/openai-ws.test.ts --dry-run
|
||||
bun run script/upstream/reset-to-upstream.ts packages/sdk/js/src/v2/gen/core/types.gen.ts --dry-run
|
||||
bun run script/upstream/reset-to-upstream.ts packages/sdk/js/src/v2/gen/core/utils.gen.ts --dry-run
|
||||
```
|
||||
|
||||
All five completed successfully, including a second execution after resume. Each resolved `v1.18.20 (7248bc19)` and printed `[DRY-RUN] Would reset <file> to transformed upstream v1.18.20`. **Five proposals, zero applied resets.** Their actual content comparisons—not the dry-run status line—support the dispositions above.
|
||||
|
||||
### Context/provenance controls
|
||||
|
||||
```sh
|
||||
git diff -U2 9563af96a012effc25df5a11eaa1f7633161a742 HEAD -- packages/opencode/src/provider/provider.ts packages/opencode/src/tool/registry.ts packages/opencode/src/plugin/openai/codex.ts
|
||||
git diff -U3 31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d 7248bc1964b13fa67e601733f89ee9dc6dfa0563 -- packages/opencode/test/tool/task.test.ts
|
||||
git diff -U1 bf1cf502a3c511e9daf6a43244568ae4e83473a8 HEAD -- packages/sdk/js/script/build.ts packages/sdk/js/package.json
|
||||
git grep -n 'include: selected.recent\|idleTimeout: 100' bf1cf502a3c511e9daf6a43244568ae4e83473a8 62998965e9fb0d9ed89011c62498b39801dbbb4f -- packages/core/src/session/compaction.ts packages/opencode/test/plugin/openai-ws.test.ts
|
||||
```
|
||||
|
||||
Additional read-only `bun -e` probes used the repository's `upstream`, `translate`, and `clean` helpers to pass JSON through Python's in-memory sequence matcher. The freshly translated baseline yielded **two unchanged blocks, 14 unchanged inline sites**. Repeating against the actual transformed parent yielded **two unchanged blocks, eight unchanged inline sites**; all six removed matches were the referer-header translation artifacts. Exact-body controls independently verified the complete task injection helper and provider-error test against base/main/upstream, avoiding punctuation-only conclusions.
|
||||
|
||||
## Limitations and integrity
|
||||
|
||||
- This is the unnecessary-marker lens only, not an overall functional, security, CI, or mergeability verdict. No GitHub state was queried or changed by this reviewer.
|
||||
- The bulk classifier's “small” and “cosmetic” buckets are not semantic equivalence tests. Its multiset algorithm can hide reordering; no changed file landed in its cosmetic bucket in the completed run.
|
||||
- Parsed marker counts exclude HTML comment markers inside the Codex callback HTML strings. Their enclosing callback replacement genuinely differs from upstream's shared-page implementation; they were not counted as fully redundant blocks.
|
||||
- Equality scanning is diagnostic, not a proof that every partially broad marker is minimal. Whole-block/inline candidates were manually checked against context, with provenance separated from new merge debt.
|
||||
- No runtime tests, lint, or typecheck were run for this report-only lens. No source code or generated files were changed, so no implementation-validation claim is made. A diagnostic probe initially misused `join(clean(...))`, failed, and was corrected to `join(clean(...).text)` before its results were used.
|
||||
- No source resets, installs, commits, pushes, branch switches, Git configuration changes, or real user-state access. Other reviewers' report files were left untouched. At resume, tracked source was clean and HEAD still matched the immutable target; the six other reports were untracked. This reviewer writes only this report.
|
||||
- Final verification: `bun run script/check-md-table-padding.ts UNNECESSARY_MARKERS.md` passed (`1 file(s) checked, no padded tables found`); `git diff --exit-code` passed with no tracked-source changes; `git rev-parse HEAD` remained `6a7d6bc002319ac2987bcde3d6c63efcafc07021`. The only new file authored by this reviewer is this report.
|
||||
Reference in New Issue
Block a user