Skip to content

Commit 97a5e5a

Browse files
committed
merge(dev): sync lidge-jun#784 onto tip, keep upstream star-deps seam
Resolve star-state and sidebar-routes conflicts by taking tip's setStarDepsForTests / withStarDeps approach (supersedes this branch's Windows gh hang workaround).
2 parents de60ea5 + b79eb71 commit 97a5e5a

14 files changed

Lines changed: 391 additions & 102 deletions
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# 050 — windows-latest 플레이크 RCA
2+
3+
CI가 세 번 연속 빨간데 **매번 다른 테스트**가 죽었다. 이런 모양이면 보통 원인이
4+
테스트가 아니라 환경이다. 기록해두는 이유는, 다음에 이 색깔을 보는 사람이 같은
5+
진단을 처음부터 다시 하지 않도록 하기 위해서다.
6+
7+
## 관측
8+
9+
| 커밋 | 죽은 테스트 | 시간 |
10+
|---|---|---|
11+
| `bf9bc1ac8` | Windows tray packaging — detached tray host가 listen 소켓을 안 물고 뜨는지 ||
12+
| `f2b61ee7e` | `GET /api/github/star` ×2 | 5015ms, 5001ms |
13+
| `6e1cdb7de` | server local API auth — pool retry 인가 안 함 ×2 | 5353ms, 412ms |
14+
15+
`bf9bc1ac8`**내 푸시 이전** 커밋이다. 즉 이 라운드가 만든 문제가 아니다.
16+
셋 다 로컬에서 통과하고, 셋 다 해당 파일이 그 푸시의 diff에 없다.
17+
18+
공통점은 5초다. Bun의 테스트 예산이 5초이고, 세 건 모두 **러너가 통제하는 무언가를
19+
기다리다** 그 예산을 넘겼다. 다만 기다리는 대상은 서로 다르다 — 하나의 원인으로
20+
묶으려다 틀리는 것보다 셋을 따로 보는 게 맞았다.
21+
22+
## 원인 1 — sidebar: 진짜 `gh`를 띄운다
23+
24+
`star-state.ts:55``spawnGh()`가 사용자의 실제 `gh` 프로세스를 띄운다.
25+
`AUTH_TIMEOUT_MS = 5_000`.
26+
27+
여기서 중요한 건, **이미 한 번 고쳐진 적이 있다는 것**이다. `0af17fbfd`
28+
Windows `.cmd` shim 해석을 `commandInvocation`으로 돌렸고, 코드 주석이 그 사실을
29+
명시한다:
30+
31+
> On Windows `gh` is a `.cmd` shim ... which is how these sidebar tests turned into
32+
> 5s timeouts on windows-latest while passing everywhere else.
33+
34+
그런데 `f2b61ee7e``0af17fbfd` **이후**인데도 같은 자리에서 죽었다. 바이너리를
35+
정확히 찾아주는 것과, 부하 걸린 러너에서 그 프로세스가 빨리 끝나는 것은 다른
36+
문제다. 첫 수정은 필요했지만 충분하지 않았다.
37+
38+
진짜 문제는 **라우트 테스트가 외부 바이너리를 띄운다는 사실 자체**다. `gh`의 설치
39+
여부, 인증 헬퍼, Windows shim은 전부 라우트 계약 밖이다.
40+
41+
수정: `star-state.ts`에 이미 있던 `StarDeps`의 주입 가능한 `runGh`
42+
`setStarDepsForTests()`로 선택한다. 테스트는 자기가 실제로 주장하는 것 —
43+
라우트 도달 가능성, 응답 형태, 그리고 `gh` 출력·토큰·계정 식별자가 절대
44+
직렬화되지 않는다는 것 — 을 결정적인 fake로 검증한다. 5초가 0.25ms가 됐다.
45+
프로덕션은 진짜 러너를 그대로 쓴다.
46+
47+
## 원인 2 — server auth: 한 테스트가 하니스를 네 번 띄운다
48+
49+
malformed-detail 케이스 네 개가 각각 프록시/업스트림 하니스를 새로 세웠다.
50+
Windows에서는 그 기동 비용만으로 예산이 찼고, 마지막 요청이 아직 날아가는 중에
51+
다음 테스트가 시작돼 전역 `fetch`를 두고 경합했다. 412ms짜리 실패가 그 흔적이다.
52+
53+
수정: 하니스 하나를 네 케이스가 공유한다. 각 케이스는 여전히 자기 원본 400과
54+
`acct-pool-a` 단일 dispatch를 증명한다.
55+
56+
## 원인 3 — tray: 안 고쳤다
57+
58+
`windows-tray.test.ts:247`이 PowerShell을 띄우고, 거기서 Bun 자식을 띄우고,
59+
같은 포트에 다시 bind한다. **자식이 listen 소켓을 물려받지 않는다는 것을 증명하는
60+
게 이 테스트의 존재 이유다.**
61+
62+
결정적으로 만들려면 `src/tray/windows.ts:464`에 프로세스 기동 seam이 필요하다.
63+
테스트에서만 흉내내면 증명이 사라진다 — 소켓 상속은 진짜 프로세스를 띄워야만
64+
관측되는 성질이다. 그래서 손대지 않았다.
65+
66+
**이건 미해결이다.** 초록으로 만들 수 있었지만 그렇게 하면 테스트가 지키던 것을
67+
버리는 것이다.
68+
69+
## 하지 않은 것
70+
71+
타임아웃을 올리지 않았다. 테스트를 skip하지 않았다. assertion을 지우지 않았다.
72+
셋 다 빨간색을 없애지만 신뢰성 신호를 침묵으로 바꾼다. `020`에서 무력한 테스트
73+
세 건을 지적해놓고 여기서 같은 짓을 하면 앞뒤가 안 맞는다.
74+
75+
## 인접 작업
76+
77+
PR #801(luvs01)이 Windows PowerShell 프로세스 열거 플레이크를 다룬다 — CIM
78+
열거가 첫 시도에 빈 결과를 주면 한 번 재시도. 우리가 고친 것과 다른 지점이고
79+
충돌하지 않는다.
80+
81+
PR #805(Wibias)가 #764`--native` 잔여를 닫는다. 지난 라운드에서 내가 열어둔
82+
바로 그 절반이다.

src/adapters/anthropic.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,16 @@ function synthesizeToolUseId(): string {
266266
return `toolu_${crypto.randomUUID().replace(/-/g, "").slice(0, 24)}`;
267267
}
268268

269+
/**
270+
* A tool_use id that a client can actually echo back. `??` only catches a missing
271+
* field, so an Anthropic-compatible relay that sends `""` or `" "` produced a
272+
* call whose id round-trips as blank — the next turn then cannot pair the result
273+
* with its call. Treat blank as absent and synthesize (#765).
274+
*/
275+
function usableToolUseId(id: unknown): string {
276+
return typeof id === "string" && id.trim() ? id : synthesizeToolUseId();
277+
}
278+
269279
function toolUseArguments(input: unknown): string {
270280
if (typeof input === "string") {
271281
const trimmed = input.trim();
@@ -824,7 +834,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
824834
if (!block) break;
825835
currentBlockType = block.type;
826836
if (block.type === "tool_use") {
827-
currentToolCallId = block.id ?? synthesizeToolUseId();
837+
currentToolCallId = usableToolUseId(block.id);
828838
currentToolCallName = toolNames.fromWire(block.name ?? "");
829839
currentToolCallJson = "";
830840
yield { type: "tool_call_start", id: currentToolCallId, name: currentToolCallName };
@@ -938,7 +948,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
938948
} else if (block.type === "redacted_thinking" && typeof block.data === "string") {
939949
events.push({ type: "redacted_thinking", data: block.data });
940950
} else if (block.type === "tool_use") {
941-
const id = block.id ?? synthesizeToolUseId();
951+
const id = usableToolUseId(block.id);
942952
events.push({ type: "tool_call_start", id, name: toolNames.fromWire(block.name ?? "") });
943953
events.push({ type: "tool_call_delta", arguments: toolUseArguments(block.input) });
944954
events.push({ type: "tool_call_end" });

src/adapters/kiro-tools.ts

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
1-
import type { OcxParsedRequest } from "../types";
1+
import type { OcxParsedRequest, OcxTool } from "../types";
22
import { namespacedToolName } from "../types";
33
import { normalizeKiroModelId } from "../providers/kiro-models";
44
import { createKiroToolNameRegistry, type KiroToolNameRegistry } from "./kiro-wire";
55

66
const MAX_KIRO_TOOL_DESCRIPTION_UNVERIFIED = 1024;
77
const MAX_KIRO_TOOL_DESCRIPTION_GPT_56_SOL = 9_216;
8+
// Issue #719's successful Kiro probe used 49 outbound tools at about 108 KiB. Leave one of those
9+
// slots for Kiro's private completion tool and headroom for the enclosing request/message fields.
10+
export const MAX_KIRO_TOOL_COUNT = 48;
11+
export const MAX_KIRO_TOOL_CATALOG_BYTES = 96_000;
12+
const textEncoder = new TextEncoder();
813

914
// JSON Schema validation/annotation keywords that Kiro's runtimeservice tool-spec validator
1015
// rejects ("ValidationException: Invalid tool use format."). Codex's built-in tools omit these,
@@ -147,6 +152,17 @@ function truncateDescription(description: string, limit: number): string {
147152
return `${description.slice(0, limit - 1)}…`;
148153
}
149154

155+
function serializedToolCatalogBytes(tools: readonly unknown[]): number {
156+
return textEncoder.encode(JSON.stringify(tools)).byteLength;
157+
}
158+
159+
function omittedToolCatalogNotice(kept: number, omitted: readonly OcxTool[], registry: KiroToolNameRegistry): string {
160+
const names = omitted.slice(0, 12).map(tool => registry.alias(namespacedToolName(tool.namespace, tool.name)));
161+
const remainder = omitted.length - names.length;
162+
const summary = `${names.join(", ")}${remainder > 0 ? `, and ${remainder} more` : ""}`;
163+
return `[opencodex] Kiro's outbound catalog budget allows ${kept} of ${kept + omitted.length} client tools this turn. Omitted and unavailable this turn: ${summary}.`;
164+
}
165+
150166
export function convertKiroToolContext(
151167
parsed: OcxParsedRequest,
152168
registry: KiroToolNameRegistry = createKiroToolNameRegistry(),
@@ -156,24 +172,39 @@ export function convertKiroToolContext(
156172
// Validate every listed name even when tool_choice:none emulates a tool-free turn.
157173
for (const tool of tools) registry.alias(namespacedToolName(tool.namespace, tool.name));
158174
const effectiveTools = parsed.options.toolChoice === "none" ? [] : tools;
175+
const convertedTools: unknown[] = [];
176+
let omittedAt = effectiveTools.length;
177+
for (const [index, tool] of effectiveTools.entries()) {
178+
const description = tool.description || `Tool: ${tool.name}`;
179+
// Send the full namespaced wire name (e.g. mcp__chrome-devtools__navigate_page) so Kiro echoes
180+
// it back; the bridge's toolNsMap is keyed by this name and restores the MCP namespace Codex
181+
// routes by. Kiro's runtimeservice rejects names with spaces or >64 chars, so normalize to a
182+
// safe form and remember the mapping; the response parser restores the original wire name.
183+
const wireName = namespacedToolName(tool.namespace, tool.name);
184+
const toolName = registry.alias(wireName);
185+
const converted = {
186+
toolSpecification: {
187+
name: toolName,
188+
description: truncateDescription(description, descriptionLimit),
189+
inputSchema: { json: ensureRootObjectType(sanitizeKiroSchema(tool.parameters ?? {})) },
190+
},
191+
};
192+
// Preserve declaration order and only omit a suffix. Ranking tools would make a catalog change
193+
// silently alter which capability disappears; this deterministic policy is paired with a
194+
// model-visible omission notice so unavailable tools are explicit rather than assumed absent.
195+
if (
196+
convertedTools.length >= MAX_KIRO_TOOL_COUNT
197+
|| serializedToolCatalogBytes([...convertedTools, converted]) > MAX_KIRO_TOOL_CATALOG_BYTES
198+
) {
199+
omittedAt = index;
200+
break;
201+
}
202+
convertedTools.push(converted);
203+
}
204+
const omittedTools = effectiveTools.slice(omittedAt);
159205
return {
160-
tools: effectiveTools.map(t => {
161-
const description = t.description || `Tool: ${t.name}`;
162-
// Send the full namespaced wire name (e.g. mcp__chrome-devtools__navigate_page) so Kiro echoes
163-
// it back; the bridge's toolNsMap is keyed by this name and restores the MCP namespace Codex
164-
// routes by. Kiro's runtimeservice rejects names with spaces or >64 chars, so normalize to a
165-
// safe form and remember the mapping; the response parser restores the original wire name.
166-
const wireName = namespacedToolName(t.namespace, t.name);
167-
const toolName = registry.alias(wireName);
168-
return {
169-
toolSpecification: {
170-
name: toolName,
171-
description: truncateDescription(description, descriptionLimit),
172-
inputSchema: { json: ensureRootObjectType(sanitizeKiroSchema(t.parameters ?? {})) },
173-
},
174-
};
175-
}),
176-
systemAdditions: [],
206+
tools: convertedTools,
207+
systemAdditions: omittedTools.length > 0 ? [omittedToolCatalogNotice(convertedTools.length, omittedTools, registry)] : [],
177208
nameMap: registry.nameMap,
178209
registry,
179210
};

src/adapters/kiro.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,10 @@ export function buildKiroPayload(
429429
// Neutralize Codex's GPT-5 identity line so a routed Kiro model never misreports as GPT-5/OpenAI
430430
// and the proxy identity never leaks upstream.
431431
if (parsed.context.systemPrompt?.length) systemParts.push(neutralizeIdentity(parsed.context.systemPrompt.join("\n\n")));
432+
for (const addition of toolContext.systemAdditions) {
433+
const boundedAddition = boundedInjectedInstruction(addition, injectedChars);
434+
if (boundedAddition) systemParts.push(boundedAddition);
435+
}
432436
const toolCatalogNudge = buildNonOpenAIToolCatalogNudgeFromNames(kiroToolWireNames(kiroTools));
433437
const boundedNudge = toolCatalogNudge ? boundedInjectedInstruction(toolCatalogNudge, injectedChars) : undefined;
434438
if (boundedNudge) systemParts.push(boundedNudge);

src/adapters/openai-chat.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,8 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
697697
const pendingToolCalls: PendingToolCall[] = [];
698698
let toolCallSeq = 0;
699699
const flushToolCalls = function* (): Generator<AdapterEvent> {
700+
// Do not treat flushed tool calls as user-facing output for the finish-less EOF
701+
// fallback — incomplete tool args must stay on the truncation path.
700702
for (const call of pendingToolCalls) {
701703
if (!call.id) call.id = `call_${++toolCallSeq}`;
702704
yield { type: "tool_call_start", id: call.id, name: call.name };
@@ -711,6 +713,9 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
711713
// explicit `[DONE]` sentinel OR a chunk carrying a non-null `finish_reason` (some
712714
// OpenAI-compatible providers omit `[DONE]` but do send finish_reason).
713715
let finishReason: string | undefined;
716+
// Only answer text enables the finish-less EOF fallback. Reasoning-only streams can be
717+
// suppressed by hideThinkingSummary and must not complete as empty successful turns.
718+
let sawUserFacingOutput = false;
714719

715720
// Single per-line handler shared by the streaming loop and the EOF residual-frame flush, so
716721
// a final frame is parsed identically wherever it lands (no duplicated, drift-prone parsing).
@@ -780,6 +785,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
780785
yield { type: "reasoning_raw_delta", text: delta.reasoning_content };
781786
}
782787
if (typeof delta.content === "string" && delta.content.length > 0) {
788+
sawUserFacingOutput = true;
783789
yield { type: "text_delta", text: delta.content };
784790
}
785791

@@ -837,10 +843,8 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
837843
if (buffer.length > 0) {
838844
if ((yield* handleDataLine(buffer)) === "terminate") return;
839845
}
840-
// Reader EOF. A graceful close shows at least one terminal signal: `[DONE]` (returns above),
841-
// a non-null finish_reason (sawFinish), or a trailing usage chunk (providers emit usage only
842-
// at end-of-generation). If NONE of those were seen, the stream was cut mid-flight — fail
843-
// closed so the bridge emits a classified response.failed rather than a silent truncation.
846+
// Reader EOF. Prefer failing closed before flushing pending tool calls so the bridge
847+
// never sees a fabricated tool_call_end on a truncated mid-assembly stream.
844848
//
845849
// Checked BEFORE flushToolCalls(), because that helper emits tool_call_end and there is no
846850
// taking it back: a half-assembled argument string would reach the client as a completed
@@ -856,16 +860,19 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
856860
yield { type: "error", message: "upstream stream ended mid tool call without a terminal signal — possible truncation" };
857861
return;
858862
}
859-
yield* flushToolCalls();
860-
if (!sawFinish && pendingUsage === undefined) {
863+
// Finish-less EOF is only safe when answer text was emitted. Reasoning-only / usage-only
864+
// truncations must stay on the error path (hideThinkingSummary can suppress reasoning).
865+
// Trailing usage alone is not a terminal signal for this adapter (#735 / restore #773).
866+
if (!sawFinish && !sawUserFacingOutput) {
861867
debugProviderDiagnostic("openai-chat", "stream-truncated", {
862868
finishReason: finishReason ?? null,
863-
hadUsage: false,
869+
hadUsage: pendingUsage !== undefined,
864870
});
865871
yield { type: "error", message: "upstream stream ended without a terminal signal ([DONE] or finish_reason) — possible truncation" };
866872
return;
867873
}
868-
// Graceful close that omitted [DONE] but delivered finish_reason and/or final usage.
874+
yield* flushToolCalls();
875+
// Graceful close that omitted [DONE] but delivered finish_reason and/or answer text.
869876
const stopReason = finishReason === "length"
870877
? "max_tokens"
871878
: finishReason === "content_filter"

0 commit comments

Comments
 (0)