From 632fcc8861f6f974daf1c7438df390ed8a2652e7 Mon Sep 17 00:00:00 2001 From: Destin Date: Mon, 28 Sep 2026 04:47:20 -0700 Subject: [PATCH 01/14] fix(harness): harden turn delivery, instructions and tool lifecycle Implement the accepted native harness audit scope with regression coverage. Preserve one queued-send flow, restore applicable guidance, isolate connection generations, and keep native question answers independent. Process-group escalation remains deferred; Claude Code credential projection and duplicate-question behavior remain unchanged. Submitted via YouCoded Assistant --- .../src/main/harness/busy-message-boundary.ts | 66 +++ desktop/src/main/harness/eval/run-case.ts | 5 + desktop/src/main/harness/harness-session.ts | 388 +++++++++--------- .../main/harness/injection/fit-rule-group.ts | 48 +++ .../main/harness/injection/path-triggers.ts | Bin 8687 -> 11375 bytes .../harness/injection/project-instructions.ts | 86 ++++ .../injection/retained-rule-visibility.ts | 16 + .../main/harness/injection/rule-delivery.ts | 68 +++ desktop/src/main/harness/mcp/mcp-manager.ts | 140 ++++--- .../src/main/harness/native-session-host.ts | 256 ++++++------ desktop/src/main/harness/prompt-assembly.ts | 15 +- desktop/src/main/harness/shell-registry.ts | 21 +- desktop/src/main/harness/tool-args.ts | 17 + .../main/harness/tool-group-finalization.ts | 27 ++ .../main/harness/tools/ask-user-question.ts | 16 +- desktop/src/main/harness/tools/bash.ts | 56 ++- desktop/src/main/harness/tools/net-guard.ts | 38 +- desktop/src/main/harness/tools/read.ts | 42 +- desktop/src/main/harness/tools/types.ts | 7 +- .../src/renderer/components/InputBar.test.tsx | 53 ++- .../components/SessionContextPopup.tsx | 30 +- desktop/src/renderer/components/ToolCard.tsx | 102 ++--- .../components/ask-question-normalize.ts | 34 ++ .../components/session-context-facts.ts | 1 + .../dev/workbench/fixture-loader.test.ts | 10 + .../renderer/dev/workbench/fixture-loader.ts | 9 +- .../fixtures/bubbles/questions-cc.jsonl | 4 + .../fixtures/bubbles/questions-native.jsonl | 4 + .../src/renderer/dev/workbench/mock-shim.ts | 39 +- .../renderer/dev/workbench/screens/chat.ts | 5 + .../src/renderer/dev/workbench/seed-chat.ts | 11 +- .../src/shared/project-instruction-summary.ts | 7 + desktop/src/shared/types.ts | 6 +- .../ask-user-question-card-other.test.tsx | 94 +++++ desktop/tests/ask-user-question-tool.test.ts | 20 + desktop/tests/bash-background.test.ts | 66 ++- desktop/tests/buddy-helper-states.test.tsx | 4 +- .../tests/harness-accepted-history.test.ts | 37 ++ desktop/tests/harness-compaction.test.ts | 13 + desktop/tests/harness-history-rebuild.test.ts | 50 +++ desktop/tests/harness-session-loop.test.ts | 264 ++++++++++++ desktop/tests/mcp-manager.test.ts | 225 ++++++++++ desktop/tests/mock-shim.test.ts | 32 ++ desktop/tests/native-clear-barrier.test.ts | 10 + desktop/tests/native-compact.test.ts | 14 + .../tests/native-image-attachments.test.ts | 37 ++ .../tests/native-permission-broker.test.ts | 12 + desktop/tests/native-session-host.test.ts | 258 +++++++++++- desktop/tests/native-tools-polish.test.ts | 93 ++++- desktop/tests/net-guard.test.ts | 123 ++++++ desktop/tests/path-triggers.test.ts | 90 ++++ desktop/tests/prompt-assembly.test.ts | 99 +++++ desktop/tests/rule-injection.test.ts | 378 +++++++++++++++++ desktop/tests/session-context-panel.test.tsx | 29 ++ desktop/tests/session-context.test.ts | 15 + desktop/tests/session-store.test.ts | 17 + desktop/tests/shell-registry.test.ts | 26 +- .../specialist-child-permissions.test.ts | 26 ++ desktop/tests/web-fetch-tool.test.ts | 19 + docs/native-runtime.md | 6 +- 60 files changed, 3133 insertions(+), 551 deletions(-) create mode 100644 desktop/src/main/harness/busy-message-boundary.ts create mode 100644 desktop/src/main/harness/injection/fit-rule-group.ts create mode 100644 desktop/src/main/harness/injection/project-instructions.ts create mode 100644 desktop/src/main/harness/injection/retained-rule-visibility.ts create mode 100644 desktop/src/main/harness/injection/rule-delivery.ts create mode 100644 desktop/src/main/harness/tool-args.ts create mode 100644 desktop/src/main/harness/tool-group-finalization.ts create mode 100644 desktop/src/renderer/components/ask-question-normalize.ts create mode 100644 desktop/src/renderer/dev/workbench/fixtures/bubbles/questions-cc.jsonl create mode 100644 desktop/src/renderer/dev/workbench/fixtures/bubbles/questions-native.jsonl create mode 100644 desktop/src/shared/project-instruction-summary.ts diff --git a/desktop/src/main/harness/busy-message-boundary.ts b/desktop/src/main/harness/busy-message-boundary.ts new file mode 100644 index 000000000..614d62bca --- /dev/null +++ b/desktop/src/main/harness/busy-message-boundary.ts @@ -0,0 +1,66 @@ +import type { ModelMessage } from 'ai'; +import { finalizeRemainingCalls } from './tool-group-finalization'; + +export interface BusyMessage { + id: string; + text: string; + attachments: string[]; + restore?: () => void; +} + +/** WHY: the claim and acceptance are one synchronous boundary. An exception + * returns the unaccepted head to its host, rather than losing a queued send. */ +export function claimBusyMessage( + take: (() => BusyMessage | undefined) | undefined, + accept: (item: BusyMessage) => void, + beforeAccept?: () => void, +): boolean { + const item = take?.(); + if (!item) return false; + try { + beforeAccept?.(); + accept(item); + } catch (err) { + item.restore?.(); + throw err; + } + return true; +} + +/** WHY: the emitted event is the authority for both live and rebuilt history; + * no synthetic steer or app-generated marker is used for human input. */ +export function appendUserHistory( + text: string, attachments: string[], emit: () => string, appGenerated: boolean, + imageParts: (paths: string[]) => Array<{ type: 'file'; mediaType: string; data: Buffer }>, + markAppGenerated: (message: ModelMessage) => ModelMessage, + history: ModelMessage[], origins: Array, record: (uuid: string) => void, +): void { + const images = imageParts(attachments); + const uuid = emit(); + const message = (images.length + ? { role: 'user', content: [{ type: 'text', text }, ...images] } as ModelMessage + : { role: 'user', content: text } as ModelMessage); + history.push(appGenerated ? markAppGenerated(message) : message); + origins.push([uuid]); + record(uuid); +} + +/** Pair every unstarted call before adding new human input. Already completed + * results stay at the front of the same tool message. */ +export function supersedeToolGroup( + calls: readonly TCall[], from: number, completed: TPart[], completedOrigins: string[], + emit: (data: { toolUseId: string; toolName: string; toolResult: string; isError: true }) => string, + part: (call: TCall, text: string) => TPart, + record: (uuid: string) => void, commit: (parts: TPart[], origins: string[]) => void, + injectCompleted: (calls: TCall[]) => void, + reason = 'Not run: new user input arrived before this action started.', + afterCompleted?: () => void, +): void { + const remaining = finalizeRemainingCalls(calls, from, + () => reason, + (call, text) => emit({ toolUseId: call.toolCallId, toolName: call.toolName, toolResult: text, isError: true }), part); + remaining.origins.forEach(record); + commit([...completed, ...remaining.parts], [...completedOrigins, ...remaining.origins]); + injectCompleted(calls.slice(0, from)); + afterCompleted?.(); +} diff --git a/desktop/src/main/harness/eval/run-case.ts b/desktop/src/main/harness/eval/run-case.ts index fea8c4f68..085709833 100644 --- a/desktop/src/main/harness/eval/run-case.ts +++ b/desktop/src/main/harness/eval/run-case.ts @@ -9,6 +9,7 @@ import { ASSISTANT_PRESET } from '../../../shared/harness-manifest'; import { CORE_TOOLS } from '../tools'; import { seedFixtureWorkspace } from './fixture-workspace'; import { assembleSystemPrompt } from '../prompt-assembly'; +import { prepareProjectInstructions } from '../injection/project-instructions'; import { resolvePreset } from '../preset-registry'; import { BATTERY_PROMPT } from './battery'; import type { TranscriptEvent } from '../../../shared/types'; @@ -411,6 +412,9 @@ export async function runCase(opts: RunCaseOpts): Promise { const wrapUpPrompt = opts.wrapUpPrompt ?? WRAP_UP_PROMPT; const fixtureRoot = seedFixtureWorkspace(opts.instructions); + // WHY: evaluator uses the production async inventory, but fixture experiments + // cannot silently inherit real instructions above their disposable root. + const projectInstructionFiles = await prepareProjectInstructions(fixtureRoot, 20_000, fixtureRoot); const events: TranscriptEvent[] = []; let toolCalls = 0; let asks = 0; @@ -486,6 +490,7 @@ export async function runCase(opts: RunCaseOpts): Promise { presetBody: resolvePreset('assistant').body, cwd: fixtureRoot, appVersion: 'eval', + projectInstructionFiles, }), // Load-bearing (2026-08-11): omitted, HarnessSession takes fitToContext's // own 32_768 default, which against this harness's output ceiling left a diff --git a/desktop/src/main/harness/harness-session.ts b/desktop/src/main/harness/harness-session.ts index dc7974b5c..ce2aacf31 100644 --- a/desktop/src/main/harness/harness-session.ts +++ b/desktop/src/main/harness/harness-session.ts @@ -171,6 +171,9 @@ export function rememberedRuleFor( } import { formatAnswers } from './tools/ask-user-question'; import { formatArgErrors } from './tools/arg-errors'; +import { parseToolArgs } from './tool-args'; +import { PermissionCallbackFailure, permissionCallback, finalizeRemainingCalls } from './tool-group-finalization'; +import { appendUserHistory, claimBusyMessage, supersedeToolGroup } from './busy-message-boundary'; import type { AskRequest, AskDecision } from './permission-broker'; import { CLOUD_DEFAULT, type CapabilityProfile } from './capability-profile'; import { adaptForWire } from './wire-adapter'; @@ -184,7 +187,11 @@ import { BUILTIN_ROSTER, type SpecialistRoster } from './specialists/registry'; import type { ShellRegistry } from './shell-registry'; import { createSkillCatalog, type SkillCatalog } from './skills/skill-catalog'; import { fitInjection } from './injection/injection-budget'; +import { pendingRules, deliverRules } from './injection/rule-delivery'; +import { ruleFitRefusal } from './injection/fit-rule-group'; +import type { PathTrigger } from './injection/path-triggers'; import type { TriggerIndex } from './injection/path-triggers'; +import { retainedRuleMessages } from './injection/retained-rule-visibility'; import { mcpToolsFor, estimateToolSchemaTokens } from './mcp/mcp-tools'; import type { ReadyServer } from './mcp/mcp-manager'; import { @@ -217,6 +224,9 @@ export interface HarnessSessionOpts { /** The tool set this session may call. Absent/[] = v0 chat behavior (the * Chat-preset path: plain text, no tool plumbing invoked). */ tools?: NativeTool[]; + /** Root-only synchronous FIFO claim. restore returns an unaccepted claim to the host + * if an event append fails; no second turn or await occurs during a claim. */ + takeReadyBusyMessage?: () => ({ id: string; text: string; attachments: string[]; restore?: () => void } | undefined); /** Pure permission decision for (tool, subject) — the configured layers * (preset/mode/deny-list/remembered). Absent → every gated tool asks. */ decide?: (tool: string, subject: string | undefined) => Promise; @@ -792,8 +802,8 @@ export class HarnessSession extends EventEmitter { private toolByName: Map; private readRegistry = new Map(); // canonical path → content fingerprint at last Read (tools/file-fingerprint.ts) /** G-11 (2026-08-26 tools investigation) — what Read has already served this - * session (`path|offset|limit` → mtime + which call). Read answers a repeat - * of an unchanged slice with "the content you already have is current" + * session (`path|offset|limit` → fingerprint + which call). Only after + * verifying current bytes does Read say "the content you already have is current" * instead of the content. That sentence is only true while the earlier * result is still in the model's view, so this is cleared at EVERY site that * discards or shrinks history: seedHistory (resume), clearHistory (/clear), @@ -827,9 +837,14 @@ export class HarnessSession extends EventEmitter { // Resolved capability profile (Task 5). Drives the doom-loop window + tool // attachment; re-assigned by setBinding on a mid-session model swap. private profile: CapabilityProfile; - /** Trigger ids already injected in this session. Survives across turns on - * purpose — a rule is a standing instruction, not a per-turn reminder. */ + /** Rule dedupe survives turns, but is reconciled after history rewrites. */ private readonly injectedTriggerIds = new Set(); + private retainedTriggerMessages = new Set(); + + private reconcileTriggerVisibility(): void { + this.injectedTriggerIds.clear(); + this.retainedTriggerMessages = retainedRuleMessages(this.history); + } /** Delivered-image dedupe: canonical path → { mtimeMs at delivery, the tool * call whose result carried the image }. A model that * re-Reads the SAME unchanged file gets "already visible" text, not a second @@ -977,9 +992,8 @@ export class HarnessSession extends EventEmitter { continuationBinding?: string; messageOrigins?: Array; }): void { this.history = messages; - // WHY: a flat accepted seed cannot locate a tail cut. Only the event-by- - // event rebuild supplies aligned origins; reject a mismatched or invented - // map instead of guessing a portable resume point. + this.reconcileTriggerVisibility(); + // WHY: only event-by-event origins prove a portable resume cut. const known = new Set(seed?.eventUuids ?? []); this.historyOrigins = seed?.messageOrigins?.length === messages.length && seed.messageOrigins.every(part => part === null || part.every(uuid => known.has(uuid))) @@ -1351,49 +1365,29 @@ export class HarnessSession extends EventEmitter { } } - /** Append any project rules / nested project instructions that the paths this - * step touched activate (M3 item 3). - * - * As a MESSAGE, never a system-prompt edit. prompt-assembly.ts is byte-stable - * by construction and a mid-session change would discard the KV cache prefix - * every local model reuses, turning a cheap follow-up turn into a full - * re-prefill of the entire conversation. - * - * ONCE per trigger per session. A rule re-sent after every Read of a matching - * file would dominate the conversation and blow the window it was sized - * against — and repetition does not make a model follow a rule harder. - * - * Bash is skipped: its permission subject is a command string, not a path, - * so feeding it to a path matcher would be a category error. - */ - private injectPathTriggers(calls: ToolCall[]): void { - const index = this.opts.triggers; - if (!index) return; - for (const call of calls) { - // Same reasoning as the path guard: matching a rule glob against a bash - // command or a skill id is a category error, not a near-miss. - if (NON_PATH_SUBJECT_TOOLS.has(call.toolName)) continue; - const tool = this.toolByName.get(call.toolName); - const subject = tool?.permissionSubject(call.input as any); - if (!subject) continue; - for (const t of index.match(subject)) { - if (this.injectedTriggerIds.has(t.id)) continue; - this.injectedTriggerIds.add(t.id); - // t.source is the rule's path relative to cwd — inside the project, so the - // model can actually read it when the notice tells it to. - const fitted = fitInjection(t.body, this.profile.injectionBudgetTokens, t.source); + /** Project guidance is history, never a system-prompt edit (KV-cache prefix). + * Selection, fit and retained-context dedupe are shared with the pre-write + * barrier in rule-delivery.ts; Bash/Task/Skill/web have no file subjects. */ + private pendingPathTriggers(calls: ToolCall[]): PathTrigger[] { + return pendingRules(calls, this.opts.triggers, this.toolByName, + this.profile.injectionBudgetTokens, NON_PATH_SUBJECT_TOOLS, + this.injectedTriggerIds, this.retainedTriggerMessages); + } + + private appendPathTriggers(triggers: readonly PathTrigger[], bounded = false): void { + deliverRules(triggers, this.profile.injectionBudgetTokens, bounded, + this.injectedTriggerIds, this.retainedTriggerMessages, content => { this.historyOrigins.push(null); - this.history.push(markAppGenerated({ - role: 'user', - content: `\n${fitted.text}\n`, - })); - this.capture.mutated(); // injected rule text, backed by no event - } - } + this.history.push(markAppGenerated({ role: 'user', content })); + this.capture.mutated(); + }); + } + + private injectPathTriggers(calls: ToolCall[]): void { + this.appendPathTriggers(this.pendingPathTriggers(calls)); } /** Add or remove the Skill tool to match the CURRENT profile (M3 item 1). - * * Skill is the one conditional tool: its description lists every offered * skill's id and one-liner, and that rides the schema on every turn, so a * small window cannot afford it. Those sessions still reach skills through @@ -1930,6 +1924,7 @@ export class HarnessSession extends EventEmitter { // automatically at the trigger instead of only on an explicit /clear. this.shownImages.clear(); this.history = replacement; + this.reconcileTriggerVisibility(); this.historyOrigins = [null, ...this.historyOrigins.slice(cut)]; this.reprojectContextUsed(estimateBefore); // Existing frozen event (no new type). `summary` is the canonical field the @@ -1957,9 +1952,7 @@ export class HarnessSession extends EventEmitter { return true; } - /** Make a pruned copy of the history the live history, with every - * bookkeeping consequence a prune has. Shared by the 'prune' decision and - * the summary commit above. */ + /** Adopt a pruned history and its bookkeeping consequences. */ private commitPrune(pruned: ModelMessage[]): void { const before = this.history; // Prune can collapse an image 'content' output to text (compaction.ts) @@ -1978,7 +1971,10 @@ export class HarnessSession extends EventEmitter { // tagging those as a transformation would bump the revision and invalidate a // published checkpoint for a history that never moved. Only a changed // message means "tool output no longer equals its event's text". - if (pruned.some((m, i) => m !== before[i])) { this.capture.markPruned(); this.prefixMoved = true; } + if (pruned.some((m, i) => m !== before[i]) || pruned.length !== before.length) { + this.reconcileTriggerVisibility(); + this.capture.markPruned(); this.prefixMoved = true; + } if (countImageOutputs(pruned) < imagesBefore) this.shownImages.clear(); // G-11: prune may have sliced an old Read result down to 2,000 chars (and // a summary discards it outright) — forget what was served so Read never @@ -2160,6 +2156,7 @@ export class HarnessSession extends EventEmitter { this.shownImages.clear(); this.servedReads.clear(); this.servedSkills.clear(); // same reason as maybeCompact this.history = replacement; + this.reconcileTriggerVisibility(); this.historyOrigins = [null, ...this.historyOrigins.slice(cut)]; this.reprojectContextUsed(estimateBeforeSummary); this.emit('transcript-event', committed); @@ -2191,6 +2188,7 @@ export class HarnessSession extends EventEmitter { if (this.abort) return { ok: false, reason: 'turn-in-flight' }; const estimateBefore = this.rewriteBaseline(); this.history = []; + this.reconcileTriggerVisibility(); this.historyOrigins = []; // What the model still holds after the barrier: the system prompt and the // tool schemas, not the conversation. Without this the status bar kept @@ -2387,22 +2385,10 @@ export class HarnessSession extends EventEmitter { }), [], true); } - /** `attachments` are absolute paths to files the user attached in the composer. - * Image ones become image parts on the user message when the model can see - * them; everything else is ignored here and reaches the model as the path - * text the composer already put in `text`. - * - * WHY images ride ALONGSIDE the text rather than replacing it: `text` is the - * dedup key. The renderer's optimistic bubble is confirmed by an EXACT match - * against the `user-message` event's text (see native-send.ts), so the string - * must stay byte-identical to what the composer built. The paths therefore - * remain in the text AND the pixels are attached — the model gets both, and - * the bubble still resolves. */ + /** WHY: keep composer text byte-identical for optimistic bubble dedup; + * image parts ride alongside it, and paths persist for replay. */ async send(text: string, attachments: string[] = []): Promise { - // Attachments ride the persisted event (paths only — events carry no binary) - // so rebuildHistory can restore the pixels on resume. Emitted only when - // present to keep the no-attachment event byte-identical to before (#290 - // follow-up fix 2). + // Persist paths for image replay; keep no-attachment event shape unchanged. return this.beginTurn(text, () => this.emitEvent('user-message', attachments.length ? { text, attachments } : { text }), attachments); } @@ -2546,34 +2532,29 @@ export class HarnessSession extends EventEmitter { return { text, images }; } - /** The turn driver. `emit` names how this turn ENTERED the conversation — a - * typed message, or a skill invocation — which is the only thing that differs - * between send() and runSkill(). Everything downstream is identical. */ + /** WHY: busy input shares the opening send's durable event/image/capture path. */ + private acceptUserMessage(text: string, attachments: string[], emit: () => string, appGenerated = false): void { + appendUserHistory(text, attachments, emit, appGenerated, paths => this.imagePartsFor(paths), + markAppGenerated, this.history, this.historyOrigins, uuid => this.capture.recordEvent(uuid)); + } + + /** Restore an unaccepted ready head before the host's next drain. */ + private acceptReadyBusyMessage(): boolean { + return claimBusyMessage(this.opts.takeReadyBusyMessage, item => this.acceptUserMessage( + item.text, item.attachments, () => this.emitEvent('user-message', + item.attachments.length ? { text: item.text, attachments: item.attachments } : { text: item.text }))); + } + + /** `emit` distinguishes user sends from skill invocation; the driver is shared. */ private async beginTurn(text: string, emit: () => string, attachments: string[] = [], appGenerated = false): Promise { - // Re-entrancy guard: a non-null abort means a turn is already streaming. - // Throw loudly rather than corrupt the single-slot turn state (see the - // class-level CONCURRENCY PRECONDITION note). + // Never overwrite the active turn's single-slot state. if (this.abort) { throw new Error('HarnessSession: a turn is already in flight — callers must serialize send()/runSkill() per session.'); } this.interrupted = false; this._currentUsageProgress = null; this.bashOutputReadsThisTurn = 0; // G-1 (D7): the cap is per TURN, notice turns included - // The uuid of the user-message / skill-invoked event this turn entered on — - // recorded at the history push below, which is the mutation it accounts for. - const enteringEventUuid = emit(); - // A plain string when there are no image parts — that is the byte-identical - // shape every existing test and rebuildHistory() already assert on, so the - // no-attachment path must not become a one-element parts array. - const imageParts = this.imagePartsFor(attachments); - const enteringMessage = (imageParts.length - ? { role: 'user', content: [{ type: 'text', text }, ...imageParts] } as ModelMessage - : { role: 'user', content: text } as ModelMessage); - this.history.push(appGenerated ? markAppGenerated(enteringMessage) : enteringMessage); - this.historyOrigins.push([enteringEventUuid]); - // Both shapes descend from the SAME event — its text, and (for the image - // parts) the attachment paths it carries. - this.capture.recordEvent(enteringEventUuid); + this.acceptUserMessage(text, attachments, emit, appGenerated); this.abort = new AbortController(); this.turnEverParked = false; // cleared at the start of every turn — see field WHY this.lastStepPromptTokens = 0; // a new turn always begins with a full prefill @@ -2754,8 +2735,8 @@ export class HarnessSession extends EventEmitter { this.overflowOutputStarted = false; while (true) { try { - step = await withChatGptRequest(this.opts.sessionId, this.opts.isSpecialistChild ? 'specialist' : 'chat', () => this.withRetry(() => - this.consumeStep(model, aiTools, (t) => { partialAssistantText = t; }), + step = await withChatGptRequest(this.opts.sessionId, this.opts.isSpecialistChild ? 'specialist' : 'chat', () => this.withRetry((registerRetraction) => + this.consumeStep(model, aiTools, (t) => { partialAssistantText = t; }, registerRetraction), )); break; } catch (err) { @@ -2949,18 +2930,20 @@ export class HarnessSession extends EventEmitter { }); continue turnLoop; } - // Second consecutive empty step: an orderly completion with an honest - // reason. Set HERE, not in mapStopReason — 'empty_response' is a - // loop-level judgment about two steps, not a mapping of one - // provider finishReason. + // WHY: second empty is a final boundary; new input re-arms recovery. + if (this.acceptReadyBusyMessage()) { + consecutiveEmptySteps = 0; + continue turnLoop; + } stopReason = 'empty_response'; break; } consecutiveEmptySteps = 0; // any real step re-arms the single retry if (step.toolCalls.length === 0) { - // Natural stop. finishReason 'length' (truncated output, including a - // truncated tool-call) collapses to 'max_tokens' via mapStopReason. + // WHY: no await before completion; late heads use the host drain. + if (this.acceptReadyBusyMessage()) continue turnLoop; + // Natural stop; mapStopReason handles truncated output as max_tokens. stopReason = mapStopReason(step.finishReason); break; } @@ -3001,51 +2984,87 @@ export class HarnessSession extends EventEmitter { const resultOrigins: string[] = []; const recordResult = (uuid: string): void => { this.capture.recordEvent(uuid); resultOrigins.push(uuid); }; for (let i = 0; i < step.toolCalls.length; i++) { + // WHY: check before each call; announced siblings need not run. + if (!this.interrupted && !this.abort.signal.aborted && claimBusyMessage( + this.opts.takeReadyBusyMessage, + item => this.acceptUserMessage(item.text, item.attachments, () => this.emitEvent('user-message', + item.attachments.length ? { text: item.text, attachments: item.attachments } : { text: item.text })), + () => supersedeToolGroup(step.toolCalls, i, resultParts, resultOrigins, + data => this.emitEvent('tool-result', data), (rem, text) => this.toolResultPart(rem, text), + uuid => this.capture.recordEvent(uuid), (parts, origins) => { + this.history.push({ role: 'tool', content: parts }); + this.historyOrigins.push(origins); + }, calls => this.injectPathTriggers(calls)))) continue turnLoop; const call = step.toolCalls[i]; - const payload = await this.runOneTool(call, recentCalls); // NEVER throws - if (payload === 'interrupted') { - // Interrupt during a permission ask. Back-fill canceled tool-results - // for THIS call AND every remaining un-executed call in the step - // (earlier calls already have real results in resultParts + emitted - // events). Every call's tool-use event was already emitted up front, - // so each still gets a matching tool-result event here. Without this, - // the assistant(tool-call) message has no matching tool message — a - // dangling tool_call that provider APIs hard-reject (HTTP 400) on the - // NEXT send, bricking the session (the bad message persists in history - // across sends). CC does the same canceled back-fill. The synthesized - // tool-result events keep the persisted transcript in agreement with - // the model-facing history. - for (let j = i; j < step.toolCalls.length; j++) { - const rem = step.toolCalls[j]; - recordResult( - this.emitEvent('tool-result', { toolUseId: rem.toolCallId, toolName: rem.toolName, toolResult: CANCELED_TOOL_TEXT, isError: true })); - resultParts.push(this.toolResultPart(rem, CANCELED_TOOL_TEXT)); + if ((call.toolName === 'Write' || call.toolName === 'Edit') && + !this.interrupted && !this.abort.signal.aborted) { + const newlyRelevant = this.pendingPathTriggers([call]); + if (newlyRelevant.length) { + // WHY: an omission notice is not guidance. A blocked group ends + // this turn rather than automatically retrying unseen instructions. + const refusal = ruleFitRefusal(newlyRelevant, this.profile.injectionBudgetTokens); + supersedeToolGroup(step.toolCalls, i, resultParts, resultOrigins, + data => this.emitEvent('tool-result', data), (rem, text) => this.toolResultPart(rem, text), + uuid => this.capture.recordEvent(uuid), (parts, origins) => { + this.history.push({ role: 'tool', content: parts }); + this.historyOrigins.push(origins); + }, calls => this.injectPathTriggers(calls), + refusal ?? 'Not run: newly applicable project instructions need review before changing this file.', + () => { + if (!refusal && !this.interrupted && !this.abort?.signal.aborted) + this.appendPathTriggers(newlyRelevant.filter(t => !this.injectedTriggerIds.has(t.id)), true); + }); + if (this.interrupted || this.abort.signal.aborted) { + this.emitEvent('user-interrupt', abandonedTurnUsage()); + return; + } + if (refusal) { stopReason = 'end_turn'; break turnLoop; } + this.acceptReadyBusyMessage(); // ready human correction precedes replan + continue turnLoop; } + } + let payload: ToolResultPayload | 'interrupted' | EndTurnResult; + try { + payload = await this.runOneTool(call, recentCalls); + } catch (err) { + if (!(err instanceof PermissionCallbackFailure)) throw err; + // WHY: the assistant already announced every call. A failed permission + // lookup is neither approval nor a human refusal; complete the accepted + // group before the outer error path settles the turn. + const failure = `Permission check failed: ${describeProviderError(err.cause)}`; + const remaining = finalizeRemainingCalls(step.toolCalls, i, + j => j === i ? failure : 'Not run: an earlier permission check failed.', + (rem, text) => this.emitEvent('tool-result', { + toolUseId: rem.toolCallId, toolName: rem.toolName, toolResult: text, isError: true, + }), (rem, text) => this.toolResultPart(rem, text)); + for (const origin of remaining.origins) recordResult(origin); + resultParts.push(...remaining.parts); + this.history.push({ role: 'tool', content: resultParts }); + this.historyOrigins.push([...resultOrigins]); + throw err; + } + if (payload === 'interrupted') { + // WHY: uses were emitted up front; back-fill canceled results for + // this and all unstarted siblings or the next send gets a provider 400. + const canceled = finalizeRemainingCalls(step.toolCalls, i, () => CANCELED_TOOL_TEXT, + (rem, text) => this.emitEvent('tool-result', { toolUseId: rem.toolCallId, toolName: rem.toolName, toolResult: text, isError: true }), + (rem, text) => this.toolResultPart(rem, text)); + canceled.origins.forEach(recordResult); + resultParts.push(...canceled.parts); this.history.push({ role: 'tool', content: resultParts }); this.historyOrigins.push([...resultOrigins]); this.emitEvent('user-interrupt', abandonedTurnUsage()); return; } - // The user dismissed a question → end the turn ORDERLY. Record THIS - // call's real result, then mark every remaining un-executed call in the - // step as not-run (same dangling-tool_call hazard the interrupt branch - // above guards against). `turn-complete` rather than `user-interrupt` - // on purpose: usage should be reported, and anything the user queued - // while the turn ran should drain — typing during the turn IS taking - // over. The max_steps gate below is the existing precedent for a - // driver-decided orderly stop. + // WHY: a dismissed question ends the turn orderly with its real result + // and not-run siblings; turn-complete reports usage and drains the queue. if ('kind' in payload) { - recordResult(this.emitEvent('tool-result', { - toolUseId: call.toolCallId, toolName: call.toolName, - toolResult: payload.payload.text, isError: true, - })); - resultParts.push(this.toolResultPart(call, payload.payload.text)); - for (let j = i + 1; j < step.toolCalls.length; j++) { - const rem = step.toolCalls[j]; - recordResult( - this.emitEvent('tool-result', { toolUseId: rem.toolCallId, toolName: rem.toolName, toolResult: NOT_RUN_TOOL_TEXT, isError: true })); - resultParts.push(this.toolResultPart(rem, NOT_RUN_TOOL_TEXT)); - } + const dismissed = finalizeRemainingCalls(step.toolCalls, i, + j => j === i ? payload.payload.text : NOT_RUN_TOOL_TEXT, + (rem, text) => this.emitEvent('tool-result', { toolUseId: rem.toolCallId, toolName: rem.toolName, toolResult: text, isError: true }), + (rem, text) => this.toolResultPart(rem, text)); + dismissed.origins.forEach(recordResult); + resultParts.push(...dismissed.parts); this.history.push({ role: 'tool', content: resultParts }); this.historyOrigins.push([...resultOrigins]); stopReason = DISMISSED_STOP_REASON; @@ -3217,9 +3236,7 @@ export class HarnessSession extends EventEmitter { }, }); } catch (err: any) { - // v0's catch, unchanged: push any in-flight partial, then split - // interrupt vs error. withRetry has already exhausted retries for a - // transient provider error before it lands here. + // Push in-flight partials; provider retries have already exhausted. // The attempt that produced this partial threw, so the loop never saw its // StepResult — this is the one acceptance decision made outside it. // trim(): the same emptiness rule as every other assistant push (and as @@ -3232,12 +3249,14 @@ export class HarnessSession extends EventEmitter { } else if (this.lastAttempt !== undefined) { this.capture.abandonAttempt(this.lastAttempt); } - if (this.interrupted || err?.name === 'AbortError' || this.abort?.signal.aborted) { + // WHY: a permission callback's own AbortError is not proof the user stopped + // the turn. Keep the tag until here while reporting its original detail. + const failure = err instanceof PermissionCallbackFailure ? err.cause : err; + if (this.interrupted || this.abort?.signal.aborted || (!(err instanceof PermissionCallbackFailure) && err?.name === 'AbortError')) { this.emitEvent('user-interrupt', abandonedTurnUsage()); } else { - // An errored turn spent the same real tokens an interrupted one did. - const errorCode = classifyProviderError(err); - this.emitEvent('session-error', { text: describeProviderError(err), ...(errorCode ? { errorCode } : {}), ...abandonedTurnUsage() }); + const errorCode = classifyProviderError(failure); + this.emitEvent('session-error', { text: describeProviderError(failure), ...(errorCode ? { errorCode } : {}), ...abandonedTurnUsage() }); } } finally { this._currentUsageProgress = null; @@ -3254,6 +3273,7 @@ export class HarnessSession extends EventEmitter { model: LanguageModel, aiTools: Record, reportPartial: (text: string) => void, + registerRetraction: (retract: () => void) => void, ): Promise { // Attempt 0 stalls with nothing streamed → runStreamOnce returns // STALL_RETRY → we re-run. That AUTOMATIC retry is available once per step: @@ -3270,7 +3290,7 @@ export class HarnessSession extends EventEmitter { // The loop is no longer bounded at two iterations — a MANUAL Retry also // returns STALL_RETRY, and the user may press it as often as they like. for (let attempt = 0; ; attempt++) { - const outcome = await this.runStreamOnce(model, aiTools, reportPartial, attempt === 0); + const outcome = await this.runStreamOnce(model, aiTools, reportPartial, attempt === 0, registerRetraction); if (outcome !== STALL_RETRY) return outcome; // Re-running after EITHER a silent stall (nothing streamed) or a manual // Retry (content streamed, then erased via dropPart): clear the on-screen @@ -3289,6 +3309,7 @@ export class HarnessSession extends EventEmitter { aiTools: Record, reportPartial: (text: string) => void, isFirstAttempt: boolean, + registerRetraction: (retract: () => void) => void, ): Promise { // One capture attempt per stream attempt. Its delta uuids stay provisional // until the turn loop (or send()'s catch) says what became of the step. @@ -3525,6 +3546,22 @@ export class HarnessSession extends EventEmitter { // rather than appended to. const emittedPartIds = new Set(); const textPartIds = new Set(); + // WHY: only the retry decision may erase an attempt. Register its LOCAL + // output before consuming; an exhausted/non-transient error keeps its honest + // partial, while a replay withdraws all four surfaces before the next request. + const retractAttempt = () => { + reportPartial(''); + for (const [prepId, entry] of preparing) { + this.emitEvent('assistant-thinking', { + toolPreparing: { toolCallId: prepId, toolName: entry.toolName, chars: entry.chars, cleared: true }, + }); + } + if (emittedPartIds.size > 0) { + this.emitEvent('assistant-thinking', { dropPart: { partIds: [...emittedPartIds] } }); + } + this.capture.abandonAttempt(attempt); + }; + registerRetraction(retractAttempt); try { while (true) { @@ -3548,28 +3585,7 @@ export class HarnessSession extends EventEmitter { void Promise.resolve(result.usage).catch(() => {}); void Promise.resolve(result.finishReason).catch(() => {}); this.resolveRetry = null; - // Retract the erased text from the model's own memory, not just the - // screen: partialAssistantText is reset per STEP (not per attempt), so - // without this, a re-run that throws before emitting anything would - // leave send()'s catch pushing the ABANDONED half-sentence — text the - // user just watched get erased via dropPart — silently back into - // this.history as an assistant message. - reportPartial(''); - // Withdraw any preparing card: the step re-runs INSIDE the same turn, - // so endTurn's reaping never fires and the card would spin forever - // beside the one the re-run mints. (Same reason as the auto-retry path.) - for (const [prepId, entry] of preparing) { - this.emitEvent('assistant-thinking', { - toolPreparing: { toolCallId: prepId, toolName: entry.toolName, chars: entry.chars, cleared: true }, - }); - } - // Erase what the abandoned attempt put on screen BEFORE re-running. - if (emittedPartIds.size > 0) { - this.emitEvent('assistant-thinking', { dropPart: { partIds: [...emittedPartIds] } }); - } - // The fourth place Retry erases: the model's memory, the screen and - // the store already forget this text, so its provenance must too. - this.capture.abandonAttempt(attempt); + retractAttempt(); return STALL_RETRY; } if (chunk === 'stall') { @@ -3582,15 +3598,7 @@ export class HarnessSession extends EventEmitter { void Promise.resolve(result.usage).catch(() => {}); void Promise.resolve(result.finishReason).catch(() => {}); if (!emittedAny && isFirstAttempt) { - // The step re-runs INSIDE the same turn, so endTurn's reaping never - // fires. Withdraw any preparing card explicitly or it spins for the - // rest of the turn while the retry mints a second card beside it. - for (const [prepId, entry] of preparing) { - this.emitEvent('assistant-thinking', { - toolPreparing: { toolCallId: prepId, toolName: entry.toolName, chars: entry.chars, cleared: true }, - }); - } - this.capture.abandonAttempt(attempt); // the re-run starts this step over + retractAttempt(); // the re-run starts this step over return STALL_RETRY; } // Name the phase honestly: a model that never STARTED (prefill) has not @@ -3851,8 +3859,9 @@ export class HarnessSession extends EventEmitter { } /** Run one tool call through the EXACT permission sequence (spec §2.1/§2.4): - * validate → doom-loop → guards → decide → (ask) → execute. NEVER throws — - * every failure mode is a tool RESULT the model can repair from, except a + * validate → doom-loop → guards → decide → (ask) → execute. Permission + * callbacks throw a tagged failure for the driver to finalize the group; + * ordinary tool failures are RESULTS the model can repair from, except a * user cancel which returns the 'interrupted' sentinel, and a dismissed * question which returns EndTurnResult — both let the loop unwind. */ private async runOneTool(call: ToolCall, recentCalls: string[]): Promise { @@ -3861,24 +3870,7 @@ export class HarnessSession extends EventEmitter { // 1. Validate (zod) — invalid args are a RESULT the model repairs from, not // a crash, and precede permissions (never ask about garbage). - let parsed = tool.inputSchema.safeParse(call.input); - if (!parsed.success && typeof call.input === 'string') { - // Weak-model hardening (Task 12, spec §3): the ai@7 SDK already parses a - // provider tool-call's stringified args into an object for us (see - // harness-sdk-toolcall-contract.test.ts), but a weak local model - // sometimes puts its WHOLE args object as a STRING one level further in - // — e.g. it emits `"{\"prompt\": ...}"` where a real object belongs. If - // the raw string itself JSON.parses to an object, give it ONE recovery - // attempt before falling back to the normal arg error — never a general - // coercion layer (YAGNI: one attempt, then the ordinary failure path). - try { - const recovered: unknown = JSON.parse(call.input); - if (recovered && typeof recovered === 'object') { - const reparsed = tool.inputSchema.safeParse(recovered); - if (reparsed.success) parsed = reparsed; - } - } catch { /* not JSON — fall through to the normal arg error below */ } - } + const parsed = parseToolArgs(tool, call.input); if (!parsed.success) { // Worded in arg-errors.ts (ledger D-2): names the unknown / missing / // mistyped parameter and, for an unknown one, the parameters that exist — @@ -3910,7 +3902,7 @@ export class HarnessSession extends EventEmitter { recentCalls.push(sig); if (recentCalls.length > threshold) recentCalls.shift(); if (recentCalls.length === threshold && recentCalls.every((s) => s === sig)) { - const d = await this.opts.askUser?.({ sessionId: this.opts.sessionId, toolName: 'doom_loop', toolInput: { repeated: call.toolName }, denyListed: false }); + const d = this.opts.askUser ? await permissionCallback(() => this.opts.askUser!({ sessionId: this.opts.sessionId, toolName: 'doom_loop', toolInput: { repeated: call.toolName }, denyListed: false })) : undefined; if (d?.behavior === 'canceled') return 'interrupted'; // Threshold-accurate: the doom-loop window length varies by profile (2 for // small local models, 3 for cloud), so quote the ACTUAL threshold, not a @@ -3928,7 +3920,7 @@ export class HarnessSession extends EventEmitter { // three times IS a doom loop and should still trip. if (tool.interactive) { if (!this.opts.askUser) return { text: `No user-interaction handler is wired for this session; ${call.toolName} cannot run. This is a configuration error.`, isError: true }; - const d = await this.opts.askUser({ sessionId: this.opts.sessionId, toolName: call.toolName, toolInput: call.input as any, denyListed: false }); + const d = await permissionCallback(() => this.opts.askUser!({ sessionId: this.opts.sessionId, toolName: call.toolName, toolInput: call.input as any, denyListed: false })); if (d.behavior === 'canceled') return 'interrupted'; // A HUMAN dismissal ENDS THE TURN: closing the card is the user taking the // turn back, not permission to guess. Still a real tool result so @@ -4006,7 +3998,7 @@ export class HarnessSession extends EventEmitter { // of rules; otherwise consult decide() (default: ask — never silent-allow). const configured: PermissionDecision = externalAsk ? { action: 'ask', denyListed: false } - : await (this.opts.decide?.(call.toolName, subject) ?? Promise.resolve({ action: 'ask', denyListed: false })); + : await permissionCallback(() => this.opts.decide?.(call.toolName, subject) ?? Promise.resolve({ action: 'ask', denyListed: false })); // denyListed: true so Full auto shows its stop band (worded per floorStop — // deny-list-copy.ts) like any deny-list stop, instead of silently running. const decision: PermissionDecision = floorStop && configured.action !== 'deny' @@ -4033,7 +4025,7 @@ export class HarnessSession extends EventEmitter { // as `pattern` — threaded through so a routed CHILD ask (child-ask- // router.ts, which has no other way to reach it) can persist the exact // same rule a root session's own remember-rule listener would. - const d = await this.opts.askUser({ sessionId: this.opts.sessionId, toolName: call.toolName, toolInput: call.input as any, denyListed: decision.denyListed, external: externalAsk, ...(floorStop ? { floorStop } : {}), subject }); + const d = await permissionCallback(() => this.opts.askUser!({ sessionId: this.opts.sessionId, toolName: call.toolName, toolInput: call.input as any, denyListed: decision.denyListed, external: externalAsk, ...(floorStop ? { floorStop } : {}), subject })); if (d.behavior === 'canceled') return 'interrupted'; // Task 8: d.message carries specific copy for a deny that ISN'T a real // user decline — e.g. child-ask-router's outside-the-folder refusal for @@ -4102,15 +4094,19 @@ export class HarnessSession extends EventEmitter { /** Exponential backoff for transient provider errors (429/5xx/network), * honoring retry-after. Layers ON TOP of the SDK's internal retry (this is * step-level resilience). Exhaustion rethrows → the session-error path. */ - private async withRetry(fn: () => Promise): Promise { + private async withRetry(fn: (registerRetraction: (retract: () => void) => void) => Promise): Promise { const delays = this.retryDelays; for (let attempt = 0; ; attempt++) { + let retractAttempt: (() => void) | undefined; try { - return await fn(); + return await fn((retract) => { retractAttempt = retract; }); } catch (err: any) { const status = err?.statusCode ?? err?.status; const retryable = status === 429 || (status >= 500 && status < 600) || err?.code === 'ECONNRESET'; if (!retryable || attempt >= delays.length || this.abort?.signal.aborted) throw err; + // WHY: retrying the whole step must first erase ONLY its failed attempt; + // previously a late provider error left streamed text on disk and screen. + retractAttempt?.(); const ra = Number(err?.responseHeaders?.['retry-after']) * 1000; await new Promise((r) => setTimeout(r, Number.isFinite(ra) && ra > 0 ? ra : delays[attempt])); } diff --git a/desktop/src/main/harness/injection/fit-rule-group.ts b/desktop/src/main/harness/injection/fit-rule-group.ts new file mode 100644 index 000000000..758ab00ba --- /dev/null +++ b/desktop/src/main/harness/injection/fit-rule-group.ts @@ -0,0 +1,48 @@ +import { fitInjection } from './injection-budget'; +import type { PathTrigger } from './path-triggers'; + +/** WHY: one pre-write boundary can activate many rules; granting each the + * entire injection budget would silently multiply the model's context cost. */ +export function fitRuleGroupDelivery(rules: readonly PathTrigger[], budgetTokens: number): { contents: string[]; omitted: string[] } { + const budget = Math.max(0, Math.floor(budgetTokens * 4)); + const wrap = (source: string, body: string) => `\n${body}\n`; + const floor = rules.map(r => `[Read ${r.source}: rule shortened or omitted.]`); + const overhead = rules.reduce((n, r) => n + wrap(r.source, '').length, 0); + const separators = Math.max(0, rules.length - 1) * 2; + if (overhead + floor.reduce((n, text) => n + text.length, 0) + separators > budget) { + // Every source must remain identifiable when space permits. Never claim a + // body was supplied if the budget cannot even hold its labelled wrapper. + const sources = rules.map(r => r.source).join(', '); + const notice = `[Project rules omitted to fit context. Read: ${sources}]`; + return { contents: [notice.length <= budget ? notice : '[Project rules omitted.]'.slice(0, budget)], omitted: rules.map(r => r.id) }; + } + let remaining = budget - overhead - separators; + const omitted: string[] = []; + const contents = rules.map((rule, i) => { + const reserve = floor.slice(i + 1).reduce((n, s) => n + s.length, 0); + const room = Math.max(floor[i].length, Math.floor((remaining - reserve) / (rules.length - i))); + const fitted = fitInjection(rule.body, Math.floor(room / 4), rule.source); + const body = fitted.text.length <= room ? fitted.text : floor[i]; + // fitInjection can itself return only a truncation notice. A label plus a + // notice is NOT delivered guidance, even if its wrapper technically fits. + if (body === floor[i] || (fitted.truncated && body.trimStart().startsWith('[...truncated'))) omitted.push(rule.id); + remaining -= body.length; + return wrap(rule.source, body); + }); + return { contents, omitted }; +} + +/** No rule body may authorize an edit through an omission-only notice. */ +export function ruleFitRefusal(rules: readonly PathTrigger[], budgetTokens: number): string | null { + const missing = fitRuleGroupDelivery(rules, budgetTokens).omitted; + if (!missing.length) return null; + const sources = rules.filter(t => missing.includes(t.id)); + const detail = sources.slice(0, 3).map(t => t.source).join(', '); + return `Not run: project rules could not fit this model's context (${detail}${sources.length > 3 ? ` and ${sources.length - 3} more` : ''}). This file change was not run. Use a model with a larger context window before retrying this change.`; +} + +/** Keep the original formatter interface for callers that display fitted text; + * execution authorization uses fitRuleGroupDelivery's explicit omission IDs. */ +export function fitRuleGroup(rules: readonly PathTrigger[], budgetTokens: number): string[] { + return fitRuleGroupDelivery(rules, budgetTokens).contents; +} diff --git a/desktop/src/main/harness/injection/path-triggers.ts b/desktop/src/main/harness/injection/path-triggers.ts index 60bd74f2d046f352768fc81d0b95930ddb2d819b..519b5d0c86e89be91eb4fee12731f0b5dc3732a8 100644 GIT binary patch delta 4359 zcmaJ^&u<&Y6-LnnMUA3q93+G9q7@85m!(c?B1+L5h#))`fd>@IC@Q(qUhnn+8( zlSY#^G|@(0ply^O`bG^YSC9_T#XP+oHPFkH-x?ER9Kd%PTCawMbZ(Qy~=}vdQyMJ)d z$(*z%#~4MlF{%?spH<)~Z;=OotmqU|zQ&YL|W!|p)9$oxZX)k$X032^|Z z3~R}(p2(!6y?1D%WWSR#TY0^`4X>|VqmnlE_Nc{Q({%AIj!oo5!)76;bYGOWLXq3l z(>j-;cqd6${v9i;9Ap!3OQINIcx@XxpXuGBKKCQ4;mEh;qanK zT`r4}8>dnB&~E}v&SqgJXp^X(w;zvn)q$7v*^Fm>J`zPcbOVX-_(=}w7TRZ}jqBj$ z?j;IBEStvYmL^_2lg9KlDKaMrYUM}$*H?e?vd>`s!Nniff4O*R5qzfT7uWKU9Nj%B zlV?x2=H}#d>scBw>Rsy|9(a75_usn~6rFnQz92Rfq|K_M-x1(gVChr*x!7PUXnd

%=l%ztW-R5(Iue#_U*wPsbYtX5o?P#X61^k` zFw^Nku#jM%PbT2AMH)BV?NYoa>hE21Z8SefB|tF)QJ;rEDCZvW22|wOd3>7 zo!~Fq*I>XIGbU5vj2!w5td-~WlS^-JGKQ6tlY2mV;hNT>=PT&ff4Fq(2m5wAxxM>l zww>PIs{e6`-dLRhQkHE}|NBx_f4TO?y*Xn35RR>ygIUR`qLMc$6x*{SXE;MzCy57h z=P3-m8QG2Z()y*f?L|U6>GgfPlVm%$Q%G!eA}~-Re~AF9Fr}E{-}~UsN4~-tJuLbX zE2trtZz9SPDQSbhKdmHhAtk65?iJ)3!6U7{>dL2rATB*Rau6vJ%&57ZYkkCd^~hLp zKMD=J>GFDx9OnUMS)=b)QnFzZxshTp<7VZ1VwelJ#1ve&Vu;*SxaAti_HmO9aQQ3< zMdv#)sPu{B{CgTcQ;2hn#WcWs-5UpyW;{2q#Bp)Vy^G+i7itxkn~KF|UzKg8#52^g z{OK7e0T$=YheqdgJ79MD8wgQ!JK+gj3Nmuxd*>UKl zd2kb9{POX`2br&I5-cO{DWDFF@O(309gpxor6n6LJv=iWY_>L+AuSqO{`Qz!iI1su z@VwPZx6{`D59k}?@d-yHOZPJ)%Utwjvgov09W)O66bccYpT|@*N0)&#Cp#glTb(w+ zuX8mH>FsnkTRhRsPn(&>>k4FmnxNG)cZ+A8u>N8hR+xcrRAEA#lg4KgAO?ihA74)D_WF&*Qlxtj#)^hz436QpsES-2NqYB23zP#i zo2t+AA`LNjV!J1Ejc(Tk#z&|9Jy3v15Cgrhm}H=?G@3`xjP6h&3@96X;JY1oU#L8X zUU0g?y%IRvdrDOq6oNN;8ciq6jLQLi*$872CQ%+kg?9uR@Ch_^1l3b*j+~J)+;oQ( zUw^n73|!D=#;g*0CYZsE7iO|91`z5Vunk;7lyF<(R4uN)wg9$o@)t-j`DXZ72uz6h zzr?LqXERXRSPh-u3h?1R7E=MA!#&4C<~Tdls$d~th2!B7l6s)bqt&6MsYPSU6A&9+ zwtuwBlPs`{$sjI-S&Ob?9JT=mDa9`Jrvnre!nlx8NMAri&Oc|oV8G$G*JP~1 zo%yiA)R}~Zgjy+i{^s%Mv2=0NjT5d<>Dj#NVRSXbQvZ2S5t63J5JoXfyY2-eOfj1^2A z19{w0g&qJ%xz$U7u`q*xrkE4XAd0}_5Dy<1weXF~@ysd=7e0qH#Y0C|3^ptK`qy|Z zse*A2NcQx=1CP_|XAmAgl$$lNg_;w2mW?JGm;&g!U@P?>FRU$mAPFZXk`t?p>33+A zJql18+6-+sQC3>MJKT#$RP+D`cpI-9jrSwo5dIs|!cU22=kyr?G3>{J9cjE>(ft{1 z>{>JQ1lD6%WMg5IMhZw@)-DF?WyD~xl4*<}qk-T&gI(b_k!QW#I9X)=TbrSA>&kC0 z&}l>3Uwn4uvp6A3yn%d;0C^VY*$Iz8;nXn?GQObUMa_eyqlZr(JjQ(<2*l|X2DtK( zxumsBgXp7-9!dYSW{jbeDrXUC&?G#hK~%tv46lC-M@?)%m7pwRhQ!0`gxv}+cyO5n zDJWdxlBOb1t5Ks7CNt4Av5yFEZ@T(Aw{V8cgZC4VR3P&D`$XR}S!k*@B?D@n5r{oJ zU9+sklhXzk&P^EbDLipEJO?Rzr)PCuIhjB*T>_N&un83iPv%km6|McKiSIPU7*Zyt Jer4ld{{tE>Zx8?g delta 1748 zcmZ8i&u15v8aJO@cDsecm0io>|R| zW4jS@rAoabvAtHPJ@j0OUin8_sj4{k!oScrvjI{zmb{)H?|t9*zW4Ha_w}vcpI(^U z&1DxDYRDdI{jfS37&}BMP#k28S?|gFLAyxR`E>McpWN+ZL`oM%CiNGN-_5K!Le1lCG*F zAaIF9?LKy=Xt6zi2H=o6tDd@NKl}p;$ox-jJINWMQ}zEC-_) Ua`S9w;XW)WlGEY!_ zP%D4F_^CzlzlXwTx@CGKBNlG-lOQC;lHgS(0gG0iMQF@7{Lr8_p2B=fYCd|wF1?(Z+&=3+2E>Pu#7qcO zO;<<;58Im?gq z3ls^8cwD*d3UrYwTu3S{>6uV)dgG?NJAI@4ZTfoo?=+P4nP&NNW@fUl7hC%}*sC`V z*1T|W&3A9KG9Nhw0C*2PZm{pei8F)_ZMV3ky)O>@W$Hs=xESCvJaJ+Y=C+f}$fA|w zG3IcDV<3#$amcBSL-;CZ;s`#s;%M@_=7f{X&dAzE=B|5_uw0+HdVz~AnjxC}*kifI z@}t=s<+Iu7d|N3tzIjq+vmUIj!U-j>yqj=Ehwb1AA4@L0c$k#3K7E-+f$LiuEY-}| zJ6Nujztpc!c|E`UvtBR%{^a}&Ejl7qEIBYsa3)jv>%!&o%G`AM<=lL^JLi`_%w1ie zLzO5NgMI8^pe(MyN$7qxYvuE6-+tJ1C!YArFQyirPq~pv?MkGpUZhG>=g1ZriWz>4 z&dj!cL#AlbQ$pV z*>O@J*te}p&s=bVR+hq7Wx|O9aIb`fwO~UStw}-;}X$$^Ina}Y8aArs_wQC zmMTp`n$0( f.text).map(f => f.unwrapped + ? f.text + : `\n${f.text}\n`); + return parts.length ? parts.join('\n\n') : null; +} + +/** WHY: discovery belongs to session startup, not synchronous prompt assembly or + * context-panel retrieval. Git boundaries do not stop instruction-file ancestry. */ +export async function prepareProjectInstructions(cwd: string, budgetTokens: number, fixtureBoundary?: string): Promise { + const dirs: string[] = []; + let dir = path.resolve(cwd); + for (;;) { + dirs.unshift(dir); + // Evaluator-only boundary: disposable A/B fixtures must not inherit real + // machine instructions; production always leaves this unset. + if (fixtureBoundary && dir === path.resolve(fixtureBoundary)) break; + const parent = path.dirname(dir); + if (parent === dir) break; + dir = parent; + } + const sources: Array<{ path: string; name: string; full: string }> = []; + const seen = new Set(); + for (const folder of dirs) { + for (const name of ['AGENTS.md', 'CLAUDE.md']) { + const file = path.join(folder, name); + let full: string; + try { full = await fs.readFile(file, 'utf8'); } + catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') continue; + // An unreadable preferred file is not permission to silently switch sources. + break; + } + // WHY: aliases through a symlink can expose the same instruction twice. + // Canonicalize only identity, never the displayed path or the session cwd. + let identity: string; + try { identity = await fs.realpath(file); } + catch { identity = file; } // If it disappeared after reading, preserve the captured text. + if (!seen.has(identity)) { sources.push({ path: file, name, full }); seen.add(identity); } + break; + } + } + // WHY: account for every byte in the final part (including labels, wrappers, + // separators and omission notices). Never print empty per-file wrappers when + // there is no room to tell the model which sources it missed. + const budget = Math.max(0, Math.floor(budgetTokens * 4)); + const wrappers = sources.reduce((n, s) => n + `\n\n`.length, 0) + + Math.max(0, sources.length - 1) * 2; + const notices = sources.map(s => `[Read ${s.path}: shortened.]`); + const omitted = (text: string | null): ProjectInstructionFile[] => sources.map((source, i) => ({ + ...source, text: i === 0 ? (text ?? '') : '', truncated: true, + note: 'Omitted from the prompt to fit this model’s context window.', + ...(i === 0 && text ? { unwrapped: true } : {}), + })); + if (wrappers + notices.reduce((n, s) => n + s.length, 0) > budget) { + const detailed = `[Project instructions omitted. Read ${sources.map(s => s.path).join(', ')}.]`; + const brief = '[Project instructions omitted.]'; + return omitted(detailed.length <= budget ? detailed : brief.length <= budget ? brief : null); + } + let remaining = budget - wrappers; + return sources.map((source, index) => { + const reserved = notices.slice(index + 1).reduce((n, s) => n + s.length, 0); + const share = Math.min(remaining - reserved, Math.floor((remaining - reserved) / (sources.length - index)) + notices[index].length); + const fitted = fitProjectInstructions(source.full, Math.floor(share / 4), source.path); + const text = fitted.text.length <= share ? fitted.text : notices[index]; + const truncated = fitted.truncated || text !== source.full; + remaining -= text.length; + return { ...source, text, truncated, + ...(truncated ? { note: text === notices[index] ? 'Omitted; read the file for the full instructions.' : text.match(/\[[^\]]+\]\s*$/)?.[0] ?? 'Shortened to fit this model’s context window.' } : {}) }; + }); +} diff --git a/desktop/src/main/harness/injection/retained-rule-visibility.ts b/desktop/src/main/harness/injection/retained-rule-visibility.ts new file mode 100644 index 000000000..3683a669e --- /dev/null +++ b/desktop/src/main/harness/injection/retained-rule-visibility.ts @@ -0,0 +1,16 @@ +import type { ModelMessage } from 'ai'; +import { isAppGenerated } from '../compaction'; + +/** WHY: a session-lifetime ID cannot vouch for guidance discarded by clear or + * summary. Inspect only at history replacement boundaries, never per token. + * Literal source labels are accepted only on app-authored, complete rule blocks; + * a human quotation or a summary mentioning a rule does not mean it is loaded. */ +export function retainedRuleMessages(history: readonly ModelMessage[]): Set { + const messages = new Set(); + for (const message of history) { + if (!isAppGenerated(message) || typeof message.content !== 'string') continue; + const match = /^\n[\s\S]*\n<\/project-rule>$/.exec(message.content); + if (match) messages.add(message.content); + } + return messages; +} diff --git a/desktop/src/main/harness/injection/rule-delivery.ts b/desktop/src/main/harness/injection/rule-delivery.ts new file mode 100644 index 000000000..d920be599 --- /dev/null +++ b/desktop/src/main/harness/injection/rule-delivery.ts @@ -0,0 +1,68 @@ +import type { NativeTool } from '../tools/types'; +import { parseToolArgs } from '../tool-args'; +import { fitInjection } from './injection-budget'; +import { fitRuleGroupDelivery } from './fit-rule-group'; +import type { PathTrigger, TriggerIndex } from './path-triggers'; + +export interface PathCall { toolName: string; input: unknown } +const fullContent = (t: PathTrigger, budget: number) => + `\n${fitInjection(t.body, budget, t.source).text}\n`; +// fitInjection may fit *only* a notice; that is not guidance for a later write. +const noticeOnly = (content: string) => /\n\s*\[\.\.\.truncated/.test(content); + +/** WHY: both post-read and pre-write selection use the same validated subject + * and exact retained-body dedupe. Non-path tool subjects are never file paths. */ +export function pendingRules(calls: readonly PathCall[], index: TriggerIndex | undefined, + byName: ReadonlyMap, budget: number, nonPath: ReadonlySet, + injected: Set, retained: Set): PathTrigger[] { + if (!index) return []; + const pending: PathTrigger[] = []; + const seen = new Set(); + for (const call of calls) { + if (nonPath.has(call.toolName)) continue; + const tool = byName.get(call.toolName); + if (!tool) continue; + const parsed = parseToolArgs(tool, call.input); + if (!parsed.success) continue; + const subject = tool.permissionSubject(parsed.data); + if (!subject) continue; + for (const rule of index.match(subject)) { + if (seen.has(rule.id) || injected.has(rule.id)) continue; + seen.add(rule.id); + const full = fullContent(rule, budget); + if (retained.has(full) && !noticeOnly(full)) { injected.add(rule.id); continue; } + pending.push(rule); + } + } + // A resumed pre-write group may retain its *bounded* body rather than the + // per-file full-budget fit. Compare exact app-generated content, not source + // alone; a changed body must still be delivered again. + const bounded = fitRuleGroupDelivery(pending, budget); + return pending.filter((rule, i) => { + const content = bounded.contents[pending.length > 1 && bounded.contents.length === 1 ? 0 : i]; + if (!bounded.omitted.includes(rule.id) && content?.startsWith(' pooled entry. Absence means "never touched yet" — NOT the - // same as a connected-but-errored server, which stays present (see acquire). + // serverId -> CURRENT pooled entry. Retired generations remain in entries + // until their holders release; an errored current entry stays reportable. private pool = new Map(); + /** Retired entries stay alive until THEIR holders release, even if ID reused. */ + private entries = new Set(); + /** Exact entries held by a lease, including retired generations. */ + private leaseEntries = new Map(); // Monotonic, process-lifetime counter behind every lease id. Its ONLY job is // uniqueness — no ordering is read off it — so wraparound/overflow concerns // don't apply at any realistic session count. @@ -156,20 +176,16 @@ export class McpManager { if (entry.conn.state === 'ready') { ready.push({ id: entry.server.id, - label: entry.server.label, + label: servers.find(s => s.id === entry.server.id)?.label ?? entry.server.label, tools: entry.conn.listTools(), call: (tool, args, signal) => entry.conn.callTool(tool, args, signal), }); } else { - // Fix (Finding 5): status()/lastError have always been correct — - // nothing has ever LOGGED them. A developer with a typo'd command - // got no tool, no error dialog, and (until now) no log line either: - // the exact silent failure this whole design exists to prevent, and - // the reason the dogfood checklist's "confirm the server drops and - // is reported" step could never actually pass. One line per - // excluded server, naming it and its real (never reworded) error. + // WHY: status() retains the real diagnostic, but a provider error + // may echo resolved credentials. Log only the ID/state, never the + // effective config or a possibly secret-bearing provider error. log('WARN', 'McpManager', 'MCP server excluded from this session — not ready', { - sessionId, serverId: entry.server.id, state: entry.conn.state, error: entry.conn.lastError, + sessionId, serverId: entry.server.id, state: entry.conn.state, }); } } @@ -209,22 +225,17 @@ export class McpManager { // awaits), so only one connect() ever runs no matter how many sessions ask // at once. // - // Except for a credential-error placeholder whose secrets now resolve (below), - // WHY no retry: if `entry` already exists — even in `error` or - // `needs-setup` state — it is returned as-is; connect() is never retried - // while the entry stays pooled (i.e. while any holder remains). This is - // deliberate, not an oversight: retry/backoff was out of scope for this - // manager. The user-visible effect is that fixing a broken server's config - // has no effect until every current holder releases (or the app - // restarts) — see status(), which surfaces the real lastError so this - // isn't silent. + // WHY no retry for unchanged settings: an error/needs-setup entry stays + // pooled until its holders release. A changed effective configuration gets + // a fresh generation immediately, without disturbing those older holders; + // retrying an unchanged broken connection on each acquire remains out of scope. private ensureConnected(server: ResolvedMcpServer, leaseId: string): PooledEntry { + const key = effectiveConfig(server); let entry = this.pool.get(server.id); - let recoveredHolders: Set | undefined; - if (entry?.server.credentialError && !server.credentialError) { - // WHY: this entry was a resource-free error placeholder, never a live - // connection. Retry after wallet recovery without discarding older leases. - recoveredHolders = entry.holders; + if (entry && entry.configKey !== key) { + // WHY: old leases retain their exact connection. Never move holders into + // the replacement (even an error placeholder): release of OLD must not + // close NEW, and A→B→A must not resurrect the retired A connection. this.pool.delete(server.id); entry = undefined; } @@ -244,6 +255,13 @@ export class McpManager { // case in mcp-client.ts); this reuses that same state value rather than // inventing a parallel one, so acquire()'s ready-check and status() // both treat it exactly like any other not-ready server. + // Snapshot before connectionFactory: a mutable registry object must not + // change the settings that an already-acquired connection actually uses. + const snapshot: ResolvedMcpServer = { ...server, transport: server.transport.type === 'stdio' + ? { ...server.transport, args: server.transport.args?.slice() } : { ...server.transport }, + env: server.env ? { ...server.env } : undefined, + headers: server.headers ? { ...server.headers } : undefined, + missingSecrets: [...server.missingSecrets] }; const credentialFailure = server.credentialError || (server.missingSecrets.length > 0 ? `${server.label} needs setup — missing secret(s): ${server.missingSecrets.join(', ')}.` : null); const conn: McpConnectionLike = credentialFailure @@ -261,9 +279,10 @@ export class McpManager { }), close: async () => {}, } - : this.connectionFactory(server); - entry = { server, conn, holders: recoveredHolders ?? new Set() }; + : this.connectionFactory(snapshot); + entry = { server: snapshot, configKey: key, conn, holders: new Set() }; this.pool.set(server.id, entry); + this.entries.add(entry); if (!credentialFailure) { entry.connecting = conn.connect().finally(() => { entry!.connecting = undefined; @@ -271,6 +290,9 @@ export class McpManager { } } entry.holders.add(leaseId); + const held = this.leaseEntries.get(leaseId) ?? []; + held.push(entry); + this.leaseEntries.set(leaseId, held); return entry; } @@ -292,42 +314,54 @@ export class McpManager { * server" check; no separate `has()` is needed. */ private async releaseLease(leaseId: string): Promise { - for (const [id, entry] of [...this.pool.entries()]) { - if (!entry.holders.delete(leaseId)) continue; - if (entry.holders.size > 0) continue; - // Remove from the pool BEFORE awaiting close(): an acquire() that lands - // during the await must build a fresh entry rather than hand out this - // one, which is already on its way down. - this.pool.delete(id); - // Unguarded close() would throw out of this loop and strand every - // remaining entry this lease still holds. The connection is being - // discarded either way, so a close() failure isn't actionable for the - // caller — log the REAL error rather than swallow it - // (error-message-standards.md), matching McpConnection.close()'s own - // best-effort teardown policy. + const held = this.leaseEntries.get(leaseId) ?? []; + this.leaseEntries.delete(leaseId); + for (const entry of held) { + if (!entry.holders.delete(leaseId) || entry.holders.size > 0) continue; + // WHY: only delete an ID when this EXACT generation is still current. + // Remove before awaiting close so an acquire during teardown connects anew. + if (this.pool.get(entry.server.id) === entry) this.pool.delete(entry.server.id); try { - await entry.conn.close(); + // WHY: keep the generation discoverable to destroyAll until close + // settles; removing it before this await let app teardown return while + // an older released transport was still closing. + await this.closeEntry(entry); } catch (err) { log('ERROR', 'McpManager', 'closing a released MCP connection failed', { - serverId: id, error: String(err), + serverId: entry.server.id, errorType: err instanceof Error ? err.name : 'unknown', }); + } finally { + this.entries.delete(entry); } } } - /** App-quit teardown: close every pooled connection regardless of - * refcount. A leaked MCP subprocess would otherwise outlive the app. */ - async destroyAll(): Promise { - const entries = [...this.pool.values()]; - this.pool.clear(); - for (const entry of entries) { + private closeEntry(entry: PooledEntry): Promise { + // WHY: release and destroyAll may overlap; close exactly once and wait for + // a pending connect before closing its transport. Never reuse while closing. + if (!entry.closing) entry.closing = (async () => { + if (entry.connecting) { try { await entry.connecting; } catch { /* close failed connect too */ } } await entry.conn.close(); - } + })(); + return entry.closing; + } + + /** App-quit teardown: close current AND retired/closing generations. */ + async destroyAll(): Promise { + const entries = [...this.entries]; + this.pool.clear(); // no acquisition may reuse a closing connection + this.leaseEntries.clear(); + // WHY: a second destroyAll must also await pending closes. Retain each + // generation in entries until its close settles, even across concurrent + // destroy/release calls. closeEntry ensures the transport closes only once. + await Promise.all(entries.map(async entry => { + try { await this.closeEntry(entry); } + finally { this.entries.delete(entry); } + })); } - /** Every server this manager has ever touched, including ones that failed - * to connect — their REAL error (McpConnection.lastError, never reworded) - * so a session picker can show why a server is unavailable. */ + /** One current entry per server ID, including failed connections. Retired + * generations remain usable through leases but never duplicate status rows. */ status(): Array<{ id: string; state: string; error: string | null }> { return [...this.pool.values()].map((entry) => ({ id: entry.server.id, diff --git a/desktop/src/main/harness/native-session-host.ts b/desktop/src/main/harness/native-session-host.ts index 44ad5765d..a5434593c 100644 --- a/desktop/src/main/harness/native-session-host.ts +++ b/desktop/src/main/harness/native-session-host.ts @@ -31,7 +31,8 @@ import { resolvePreset, type ResolvedPreset } from './preset-registry'; import { decidePermission } from './permission-engine'; import { getShell } from './tools/bash'; import { rulesForMode, sameRule, isCrossProjectRule, CROSS_PROJECT_SLUG, DESTRUCTIVE_DENY_LIST, type NativePermissionMode, type PermissionRule } from '../../shared/permission-types'; -import { assembleSystemPrompt, assembleSystemPromptParts, findProjectInstructions, gitSnapshotAsync } from './prompt-assembly'; +import { assembleSystemPrompt, assembleSystemPromptParts, gitSnapshotAsync } from './prompt-assembly'; +import { prepareProjectInstructions, type ProjectInstructionFile } from './injection/project-instructions'; import { resolveProfile, effectiveContextForModel, type CapabilityProfile, type ProfileProviderType } from './capability-profile'; import { CORE_TOOLS } from './tools'; import type { ToolServices, SpecialistReservation, SpecialistSpawnOpts, SpecialistManageOutcome, SpecialistResumeOutcome } from './tools/types'; @@ -51,7 +52,7 @@ import { PermissionModeStore } from './permission-mode-store'; import { computeReportBudget } from './specialists/report-budget'; import { truncateOutput, composeNotice } from './tools/truncate'; import { APPROX_CHARS_PER_TOKEN } from './message-size'; -import { fitInjection, fitProjectInstructions } from './injection/injection-budget'; +import { fitInjection } from './injection/injection-budget'; import { frameSkillInvocation } from './skills/skill-invocation'; import { buildTriggerIndex, type TriggerIndex } from './injection/path-triggers'; import { costForUsage, isFreePricing, type ModelPricing } from './pricing'; @@ -300,6 +301,7 @@ interface LiveEntry { // The "What the assistant was given" record last pushed for this session, so a // model swap can re-push it with the new model's window (republishWindow). sessionContext?: SessionContext; + projectInstructionFiles?: readonly ProjectInstructionFile[]; // Per-session append serialization: each transcript event extends this chain // (append(prev).then(next)) so the SessionStore contract (serialized appends) // holds. Starts resolved; a failed append is logged but never breaks the @@ -324,7 +326,7 @@ interface LiveEntry { // `attachments` are absolute composer file paths; image ones become image // parts on the user message. Carried through the QUEUE too, or a message sent // while a turn was in flight would silently lose its pictures. - queue: { id: string; text: string; attachments: string[] }[]; + queue: { id: string; text: string; attachments: string[]; ready?: boolean }[]; // True from dispatch until runTurns finishes the last queued turn. Host-owned // (HarnessSession's in-flight state is private); safe because Node is single-threaded. inFlight: boolean; @@ -378,16 +380,6 @@ function persistedUuidsAfterLastClear(events: TranscriptEvent[]): string[] { // pass" (a typed message, a queued message, a skill) from "the host did". const IDLE_PASS = async (): Promise => {}; -/** The bracketed sentence a fitter appends to say what it cut. Read back rather - * than re-described here: a second wording would eventually disagree with the - * one the model was actually given, and this panel exists to be trusted. - * Null when the text carries no such notice. */ -function truncationNote(fittedText: string): string | null { - const at = fittedText.lastIndexOf('\n\n['); - if (at === -1 || !fittedText.trimEnd().endsWith(']')) return null; - return fittedText.slice(at + 3).trimEnd().slice(0, -1); -} - /** The name a person recognises out of a skill id. Ids are `plugin:skill` or a * bare name; the part after the colon is what the user typed to install it. */ function skillLabel(id: string): string { @@ -958,9 +950,9 @@ export class NativeSessionHost extends EventEmitter { const { contextLength, profile, pricing, free } = await this.resolveContextAndProfile(binding); const title = header.title ?? record.title; - const [gitSnapshot, triggers] = await Promise.all([gitSnapshotAsync(workDir), buildTriggerIndex(workDir)]); + const [gitSnapshot, triggers, projectInstructionFiles] = await Promise.all([gitSnapshotAsync(workDir), buildTriggerIndex(workDir), prepareProjectInstructions(workDir, profile.injectionBudgetTokens)]); const session = this.buildSpecialistSession( - parentId, opts.childId, workDir, title, specialist, binding, contextLength, profile, pricing, free, opts.parentToolCallId, preset, parent, gitSnapshot, triggers, + parentId, opts.childId, workDir, title, specialist, binding, contextLength, profile, pricing, free, opts.parentToolCallId, preset, parent, gitSnapshot, triggers, projectInstructionFiles, ); // Cold state rebuilt from the child's OWN transcript — seedHistory resets // readRegistry + todos too (the same reset-on-resume contract root @@ -2598,7 +2590,7 @@ export class NativeSessionHost extends EventEmitter { * `profile` is accepted here so Task 6 can add a prompt variant without another * signature change; this task doesn't use it yet (the session itself carries it * via opts.profile). */ - private toolWiring(sessionId: string, cwd: string, preset: ResolvedPreset, profile: CapabilityProfile, gitSnapshot: string, triggers: TriggerIndex): Pick { + private toolWiring(sessionId: string, cwd: string, preset: ResolvedPreset, profile: CapabilityProfile, gitSnapshot: string, triggers: TriggerIndex, projectInstructionFiles: readonly ProjectInstructionFile[]): Pick { return { // G-1: this session's background-command registry, host-owned. shells: this.shellsFor(sessionId), @@ -2727,7 +2719,7 @@ export class NativeSessionHost extends EventEmitter { // timeouts) and could land on a different date or branch than the prompt the // model actually got. presetName is label-only and reaches no model. ...(() => { - const promptParts = assembleSystemPromptParts({ presetBody: preset.body, cwd, appVersion: this.appVersion, promptVariant: profile.promptVariant, hasTools: profile.supportsTools, instructionBudgetTokens: profile.injectionBudgetTokens, supportsParallelToolCalls: profile.supportsParallelToolCalls, audience: 'user', presetName: preset.manifest.name, gitSnapshot }); + const promptParts = assembleSystemPromptParts({ presetBody: preset.body, cwd, appVersion: this.appVersion, promptVariant: profile.promptVariant, hasTools: profile.supportsTools, instructionBudgetTokens: profile.injectionBudgetTokens, supportsParallelToolCalls: profile.supportsParallelToolCalls, audience: 'user', presetName: preset.manifest.name, gitSnapshot, projectInstructionFiles }); return { promptParts, systemPrompt: promptParts.map((p) => p.text).join('\n\n') }; })(), }; @@ -3130,8 +3122,8 @@ export class NativeSessionHost extends EventEmitter { /** Subscribe a freshly-built HarnessSession: forward its events to the * renderer immediately, and enqueue each on the session's append chain. */ - private wire(sessionId: string, cwd: string, session: HarnessSession, mcpLease?: McpLease): void { - const entry: LiveEntry = { session, cwd, appendChain: Promise.resolve(), compactionGeneration: this.restoredCompactionGeneration.get(session) ?? 0, queue: [], inFlight: false, mcpLease }; + private wire(sessionId: string, cwd: string, session: HarnessSession, mcpLease?: McpLease, projectInstructionFiles: readonly ProjectInstructionFile[] = []): void { + const entry: LiveEntry = { session, cwd, appendChain: Promise.resolve(), compactionGeneration: this.restoredCompactionGeneration.get(session) ?? 0, queue: [], inFlight: false, mcpLease, projectInstructionFiles }; this.live.set(sessionId, entry); this.retainModel(sessionId, session.binding.modelId); // ref-count this model // Persist "Always allow" decisions for THIS session's project. The session @@ -3167,11 +3159,10 @@ export class NativeSessionHost extends EventEmitter { // AFTER the listeners above and BEFORE the held-message drain, so the line is // on screen before the first turn's output starts arriving. // - // Never fatal: this reads the instruction file and syncs the tool set, and a - // session must still open if either fails. A missing line is a missing - // explanation; a thrown one is a chat that never starts. + // Never fatal: this describes the already-captured instructions and tool + // set; a missing explanation must not stop a chat from opening. try { - entry.sessionContext = this.buildSessionContext(cwd, session); + entry.sessionContext = this.buildSessionContext(entry); this.emit('session-context', { sessionId, context: entry.sessionContext }); } catch (err) { log('ERROR', 'NativeSessionHost', 'could not describe the session context', { sessionId, error: String(err) }); @@ -3199,8 +3190,8 @@ export class NativeSessionHost extends EventEmitter { /** What this session was given, for the line above the conversation and the * "What the assistant was given" panel. * - * Built ONCE per session, at wire() — root sessions only, so a specialist - * child (which is never wired) never grows a line of its own. + * Built ONCE per session from its prompt's captured inventory, at wire() — + * root sessions only; children have no context-panel record. * * NO FILE BODIES. The only text here is the system prompt, which is already * assembled and in memory; every file the panel can show is fetched by @@ -3212,10 +3203,10 @@ export class NativeSessionHost extends EventEmitter { * a small model — the skill catalog itself, which the model is simply never * told about. A skill being too long to fit is a thing that has not happened * yet, so it is reported on the skill's own row and never in this summary. */ - private buildSessionContext(cwd: string, session: HarnessSession): SessionContext { + private buildSessionContext(entry: LiveEntry): SessionContext { + const session = entry.session; const inv = session.contextInventory(); - const found = findProjectInstructions(cwd); - const fitted = found ? fitProjectInstructions(found.text, inv.injectionBudgetTokens, found.name) : null; + const files = entry.projectInstructionFiles ?? []; return { modelLabel: session.binding.modelId, contextWindowTokens: session.contextWindowTokens, @@ -3224,9 +3215,10 @@ export class NativeSessionHost extends EventEmitter { // The project-instructions part is filtered out: it has its own tab, and // showing it under System too would say the same thing twice. systemPromptSections: inv.promptParts.filter((p) => p.id !== 'project'), - projectInstructions: found && fitted - ? { path: found.path, truncated: fitted.truncated, note: truncationNote(fitted.text) } + projectInstructions: files.length + ? { path: files[files.length - 1].path, truncated: files[files.length - 1].truncated, note: files[files.length - 1].note ?? null } : null, + projectInstructionFiles: files.map(f => ({ path: f.path, truncated: f.truncated, note: f.note ?? null })), skills: inv.skills.map((s) => ({ id: s.id, label: skillLabel(s.id), description: s.description })), skillsOffered: inv.skillsOffered, tools: inv.toolNames, @@ -3267,10 +3259,11 @@ export class NativeSessionHost extends EventEmitter { if (!entry) return { error: 'not-live' }; const budget = entry.session.profileSnapshot.injectionBudgetTokens; if (kind === 'project') { - const found = findProjectInstructions(entry.cwd); + const files = entry.projectInstructionFiles ?? []; + // WHY: id is only a selector into the captured inventory, never a path read. + const found = id ? files.find(f => f.path === id) : files[files.length - 1]; if (!found) return { error: 'not-found' }; - const fitted = fitProjectInstructions(found.text, budget, found.name); - return { path: found.path, text: fitted.text, full: found.text, truncated: fitted.truncated }; + return { path: found.path, text: found.text, full: found.full, truncated: found.truncated }; } if (!id) return { error: 'not-found' }; try { @@ -3335,7 +3328,7 @@ export class NativeSessionHost extends EventEmitter { await this.specialistCatalog.ensureFresh(opts.cwd); // The git line, read off the main thread before anything is built // (2026-09-16 C3). Never throws (a non-repo answers a fixed string). - const [gitSnapshot, triggers] = await Promise.all([gitSnapshotAsync(opts.cwd), buildTriggerIndex(opts.cwd)]); + const [gitSnapshot, triggers, projectInstructionFiles] = await Promise.all([gitSnapshotAsync(opts.cwd), buildTriggerIndex(opts.cwd), prepareProjectInstructions(opts.cwd, profile.injectionBudgetTokens)]); // Acquire this session's MCP servers (Task 6) BEFORE constructing the // session, so mcpServers is available for the very first buildAiTools(). const mcpLease = await this.acquireMcp(opts.sessionId); @@ -3353,8 +3346,9 @@ export class NativeSessionHost extends EventEmitter { session = new HarnessSession( { sessionId: opts.sessionId, cwd: opts.cwd, harness, binding: opts.binding, contextLength, profile, pricing, free, commitCompaction: proposal => this.commitCompaction(opts.sessionId, session, proposal), + takeReadyBusyMessage: () => this.takeReadyBusyMessage(opts.sessionId, session), ...(mcpServers ? { mcpServers } : {}), - ...this.toolWiring(opts.sessionId, opts.cwd, preset, profile, gitSnapshot, triggers) }, + ...this.toolWiring(opts.sessionId, opts.cwd, preset, profile, gitSnapshot, triggers, projectInstructionFiles) }, this.modelFactory, ); } catch (err) { @@ -3362,7 +3356,7 @@ export class NativeSessionHost extends EventEmitter { throw err; } this.presetIdFor.set(opts.sessionId, preset.manifest.id); - this.wire(opts.sessionId, opts.cwd, session, mcpLease); + this.wire(opts.sessionId, opts.cwd, session, mcpLease, projectInstructionFiles); if (slotsUnknown) this.live.get(opts.sessionId)!.refreshSlotsAfterTurn = true; } @@ -3435,9 +3429,9 @@ export class NativeSessionHost extends EventEmitter { // buildSpecialistSession is fallible synchronous work, and a throw after // the header write would leave a session file on disk for a child that // never existed. The git line (C3) and trigger walk (B8) are awaited first. - const [gitSnapshot, triggers] = await Promise.all([gitSnapshotAsync(workDir), buildTriggerIndex(workDir)]); + const [gitSnapshot, triggers, projectInstructionFiles] = await Promise.all([gitSnapshotAsync(workDir), buildTriggerIndex(workDir), prepareProjectInstructions(workDir, profile.injectionBudgetTokens)]); const session = this.buildSpecialistSession( - parentId, childId, workDir, title, opts.specialist, binding, contextLength, profile, pricing, free, opts.parentToolCallId, preset, parent, gitSnapshot, triggers, + parentId, childId, workDir, title, opts.specialist, binding, contextLength, profile, pricing, free, opts.parentToolCallId, preset, parent, gitSnapshot, triggers, projectInstructionFiles, ); // `title` was drawn earlier (before this session was built — see that @@ -3487,9 +3481,15 @@ export class NativeSessionHost extends EventEmitter { pricing: ModelPricing | null, free: boolean, parentToolCallId: string, preset: ResolvedPreset, parent: LiveEntry, // The child's git line and trigger index, awaited by the caller off the main thread (C3, B8). - gitSnapshot: string, triggers: TriggerIndex, + gitSnapshot: string, triggers: TriggerIndex, projectInstructionFiles: readonly ProjectInstructionFile[], ): HarnessSession { - const allowed = new Set(specialist.allowedTools); + // WHY: the spawn-time Bash capability already advertises these helpers; + // hand the SAME derived list to tool exposure and the permission cap so a + // child can manage only its own background runs without editing the definition. + const effectiveAllowedTools = specialist.allowedTools.includes('Bash') + ? [...new Set([...specialist.allowedTools, 'BashOutput', 'KillShell'])] + : [...specialist.allowedTools]; + const allowed = new Set(effectiveAllowedTools); let session: HarnessSession; session = new HarnessSession( { @@ -3499,12 +3499,10 @@ export class NativeSessionHost extends EventEmitter { // lifecycle controls, and the delegation spawn backstop—not an arbitrary // per-child action count, so root limits never flow into a child. harness: preset.manifest, - // TOOLS: the definition's allowlist, filtered out of the same CORE_TOOLS - // set every session is built from. The Task tool is structurally absent - // because no definition lists it — that omission IS the depth-1 rule. - // G-1: a helper allowed Bash gets the companions too — its own - // background command would otherwise be unreadable and unstoppable. - tools: CORE_TOOLS.filter((t) => allowed.has(t.name) || (allowed.has('Bash') && (t.name === 'BashOutput' || t.name === 'KillShell'))), + // TOOLS: the spawn-time effective list, filtered from CORE_TOOLS. + // Task stays absent by definition (depth 1); Bash companions are usable + // only when Bash is in that definition, never through an unrelated grant. + tools: CORE_TOOLS.filter((t) => allowed.has(t.name)), // G-1: children get their OWN registry; their runs die with the child // under 'conversation-closed' when destroyChildrenOf tears them down. shells: this.shellsFor(childId), @@ -3513,7 +3511,7 @@ export class NativeSessionHost extends EventEmitter { // parent's conversation crosses over — the brief in the first user turn // is the entire context the child gets. systemPrompt: assembleSystemPrompt({ - presetBody: specialist.systemPrompt, cwd: workDir, appVersion: this.appVersion, gitSnapshot, + presetBody: specialist.systemPrompt, cwd: workDir, appVersion: this.appVersion, gitSnapshot, projectInstructionFiles, promptVariant: profile.promptVariant, hasTools: profile.supportsTools, instructionBudgetTokens: profile.injectionBudgetTokens, // audience 'parent': the shared doctrine's writing-for-the-user block is @@ -3546,7 +3544,7 @@ export class NativeSessionHost extends EventEmitter { decide: buildChildDecide({ parentDecide: this.buildDecide(parentId, parent.cwd, preset.presetRules, { specialistScope: specialist.id }), charter: specialist.charter, - allowedTools: specialist.allowedTools, + allowedTools: effectiveAllowedTools, // envelopeGranted: true means the hire was PERMITTED, not that the user was necessarily // asked. Two ways that happens: (1) a Task-tool ask card (new spawn) or the original // spawn's ask card (resume) was answered — real consent; or (2) the active permission @@ -3825,7 +3823,7 @@ export class NativeSessionHost extends EventEmitter { // destroy() can only release the lease on the LiveEntry it captured. See // McpLease in mcp-manager.ts. // The git line + trigger index, off the main thread, before the session is built (C3, B8). - const [gitSnapshot, triggers] = await Promise.all([gitSnapshotAsync(cwd), buildTriggerIndex(cwd)]); + const [gitSnapshot, triggers, projectInstructionFiles] = await Promise.all([gitSnapshotAsync(cwd), buildTriggerIndex(cwd), prepareProjectInstructions(cwd, profile.injectionBudgetTokens)]); const mcpLease = await this.acquireMcp(sessionId); const mcpServers = mcpLease?.servers; const harness = header.stepGuard === undefined @@ -3844,8 +3842,9 @@ export class NativeSessionHost extends EventEmitter { // `binding` (not header.binding) — same override reason as above. { sessionId, cwd, harness, binding, contextLength, profile, pricing, free, commitCompaction: proposal => this.commitCompaction(sessionId, session, proposal), + takeReadyBusyMessage: () => this.takeReadyBusyMessage(sessionId, session), ...(mcpServers ? { mcpServers } : {}), - ...this.toolWiring(sessionId, cwd, preset, profile, gitSnapshot, triggers) }, + ...this.toolWiring(sessionId, cwd, preset, profile, gitSnapshot, triggers, projectInstructionFiles) }, this.modelFactory, ); // Full history rebuild (spec §2.5): rebuildHistory reconstructs the assistant @@ -3860,7 +3859,7 @@ export class NativeSessionHost extends EventEmitter { throw err; } this.presetIdFor.set(sessionId, preset.manifest.id); - this.wire(sessionId, cwd, session, mcpLease); + this.wire(sessionId, cwd, session, mcpLease, projectInstructionFiles); if (slotsUnknown) this.live.get(sessionId)!.refreshSlotsAfterTurn = true; // Task 9 — AFTER wire(), not before: reconcileDelegations's own // queueDelivery() call needs this.live.get(sessionId) to already resolve @@ -3925,7 +3924,16 @@ export class NativeSessionHost extends EventEmitter { // Task 11: mint a stable id per queued entry so the renderer can target // this exact message later with removeQueued() (Cancel/Edit before send). const queueId = randomUUID(); - entry.queue.push({ id: queueId, text, attachments }); + // WHY: both the host drainer and a future in-turn claimant must leave this + // head untouched until the IPC acknowledgement has had a macrotask to flush. + const queued = { id: queueId, text, attachments, ready: false }; + entry.queue.push(queued); + setImmediate(() => { + queued.ready = true; + // The turn may have settled while this head was unready. Readiness is + // itself a progress signal, not something the next send must discover. + if (this.live.get(sessionId) === entry) this.kickReadyQueue(sessionId, entry); + }); return { status: 'queued', queueId }; } entry.inFlight = true; @@ -3976,97 +3984,83 @@ export class NativeSessionHost extends EventEmitter { return true; } - // Runs the dispatched turn, then drains the queue turn-by-turn. send() settling - // is the ONLY drain trigger — it settles strictly after turn-complete / - // session-error / user-interrupt, and stays unsettled across a permission ask - // (an ask pauses the turn; draining on it would hard-throw re-entrancy). + /** WHY: only the live root driver may remove an acknowledged FIFO head. + * No awaits between checking generation/readiness/quiesce and the shift. */ + private takeReadyBusyMessage(sessionId: string, session: HarnessSession): + { id: string; text: string; attachments: string[]; restore: () => void } | undefined { + const entry = this.live.get(sessionId); + if (!entry || entry.session !== session || entry.parentSessionId || !entry.inFlight || + entry.quiescing || entry.compacting || entry.queue[0]?.ready === false) return; + const item = entry.queue.shift(); + if (!item) return; + return { ...item, restore: () => { + if (this.live.get(sessionId) === entry && !entry.quiescing) entry.queue.unshift(item); + else log('ERROR', 'NativeSessionHost', 'claimed message could not be restored after delivery failure', { sessionId, queueId: item.id }); + } }; + } + + /** Claim an idle slot for a head whose acknowledgement fence just opened. + * Never bypass an unready FIFO head; runTurns does the actual shift. */ + private kickReadyQueue(sessionId: string, entry: LiveEntry): void { + if (entry.inFlight || entry.quiescing || entry.compacting || !entry.queue[0] || entry.queue[0].ready === false) return; + entry.inFlight = true; + entry.running = new Promise((resolve) => { + setImmediate(() => { void this.runTurns(sessionId, entry, IDLE_PASS).then(resolve, resolve); }); + }); + } + + // Run turns serially. A settled send, the notice tail and a queued head's + // readiness transition are drain triggers. An ask pauses the current turn; + // no other send runs until it settles (the driver rejects re-entrancy). /** `first` is a plain string for an ordinary send, or a THUNK when the turn * starts some other way — today only /skill-name, whose opener is * `session.runSkill` (same turn machinery, different transcript event). * Queued follow-ups are always plain sends, so queue semantics are unchanged. */ private async runTurns(sessionId: string, entry: LiveEntry, first: SendUnit | (() => Promise)): Promise { - // Fix (Task 4 fix pass 3): the WHOLE body is now wrapped in a single - // try/finally so "every runTurns exit clears entry.inFlight" is true by - // CONSTRUCTION — one statement, not a comment asserting a property the - // code has to remember to uphold at every return/break/throw site. Before - // this, `entry.inFlight = false` was a bare statement at the tail: a - // throw from ANY of the unguarded `await this.ledger.*` calls inside the - // delivery loop below (claimUndelivered at the top of the loop, - // releaseClaim on either liveness-mismatch branch) propagated straight - // out of this function, so that tail statement was never reached and the - // session was permanently stuck "in flight" — the exact bug the loop - // unification was meant to prevent, arriving by exception instead of by - // `return`. The early `return` in the queue-drain loop below (destroy() - // raced this turn) still runs this finally too, which is harmless: by - // definition `this.live.get(sessionId) !== entry` there, so `entry` is - // already a discarded object and setting its `inFlight` flag touches - // nothing live. + // WHY: even a failed ledger claim or a replaced generation must release + // this pass's slot; the finally covers every exit. try { - // Task 4 (plan 1c) — re-read this project folder's specialist catalog - // before dispatching this pass's turn(s): a file dropped into a - // specialists folder since the last turn is offered starting on THIS - // turn, without a session restart. ONE call per runTurns invocation - // (not per queued follow-up) — every turn drained in this same pass - // shares the roster this one read resolved. ensureFresh()'s own - // fingerprint check makes an unchanged folder cheap (a handful of - // stat() calls, never a re-parse); it never throws (every fallible fs - // call inside it is already individually guarded — see catalog.ts's - // own WHY comments), so this is not wrapped in its own try/catch. - // Fix (Task 4 review): gated on `!entry.parentSessionId` — ROOT sessions - // ONLY. A specialist child DOES reach this function: runSpecialist's - // runTurn() closure calls this.send(childId, ...), and send() dispatches - // unconditionally into this.runTurns() for whatever id it's given, so - // every child turn (the opening turn AND the empty-report nudge) used to - // run this call too — an earlier comment here claimed otherwise, which - // was simply wrong. The real reason to skip it for a child: its roster - // is fixed at spawn (R12) and never read again — createChild builds a - // child's tools by hand and never calls toolWiring(), so no child ever - // consults this.specialistCatalog.roster(cwd) for ITS OWN cwd. Without - // this gate, every child turn wrote a fresh entry into the catalog's - // per-cwd cache (a Map with no eviction) for a cwd nothing will ever - // read a roster for — often a work_dir narrowed to a subfolder nothing - // else touches — so the cache grew forever across the process's life. + // WHY: refresh the root's roster once per pass, not per queued turn. + // Children use their fixed spawn roster; refreshing each child's cwd + // would grow the catalog cache without a consumer. if (!entry.parentSessionId) await this.specialistCatalog.ensureFresh(entry.cwd); let next: SendUnit | (() => Promise) | undefined = first; - while (next !== undefined) { - // A user-started turn lifts the post-Stop hold: everything parked since - // the Stop is spliced into history NOW, silently, so the model reads it - // as context for this message rather than getting a turn of its own. - // Checked per turn, not once per pass, because a message queued before - // the Stop still runs after it (pinned: "the queue still drains"). - if (entry.holdDeliveries && next !== IDLE_PASS) { - entry.holdDeliveries = false; - await this.drainDeliveries(sessionId, entry, 'splice'); + // WHY: notices may await the model after the last queued shift. Alternate + // delivery and FIFO dispatch until BOTH lanes are quiet at the same boundary. + for (;;) { + while (next !== undefined) { if (this.live.get(sessionId) !== entry) return; - } - try { - if (typeof next === 'function') await next(); - else await entry.session.send(next.text, next.attachments); - } catch (err) { - log('ERROR', 'NativeSessionHost', 'send failed', { sessionId, error: String(err) }); - } - // Destroy() may have removed/replaced the entry mid-turn — stop draining then. - if (this.live.get(sessionId) !== entry) return; - // A local model is loaded by now (a real turn just ran): re-read the - // helper cap the engine could not answer at create/resume/swap. INSIDE - // the drain loop, before the shift below, on purpose — a message - // queued while this await is in the air is picked up by that shift, - // whereas an await placed after the loop (first cut, 2026-09-16) - // stranded such a message until the next send, since inFlight was - // still true and nothing re-read the queue. Root sessions only (a - // child's roster is fixed at spawn); never after a delivery-only pass, - // which loads nothing. - if (entry.refreshSlotsAfterTurn && !entry.parentSessionId && typeof next !== 'function') { - await this.refreshLocalSlots(sessionId, entry); + // A user-started turn lifts the post-Stop hold so the model reads + // parked reports as context, not as a new turn of their own. + if (entry.holdDeliveries && next !== IDLE_PASS) { + entry.holdDeliveries = false; + await this.drainDeliveries(sessionId, entry, 'splice'); + if (this.live.get(sessionId) !== entry) return; + } + try { + if (typeof next === 'function') await next(); + else await entry.session.send(next.text, next.attachments); + } catch (err) { + log('ERROR', 'NativeSessionHost', 'send failed', { sessionId, error: String(err) }); + } if (this.live.get(sessionId) !== entry) return; + // A local model can answer its slot count only after a real turn. + // Refresh inside the loop so sends accepted during the await are seen. + if (entry.refreshSlotsAfterTurn && !entry.parentSessionId && typeof next !== 'function') { + await this.refreshLocalSlots(sessionId, entry); + if (this.live.get(sessionId) !== entry) return; + } + // The atomic shift owns the queue ID; an unready FIFO head blocks + // dispatch, and its scheduled readiness transition restarts the pass. + next = entry.quiescing || entry.queue[0]?.ready === false ? undefined : entry.queue.shift(); } - // .text: queue entries are {id, text} (Task 11) — the id only matters to - // removeQueued(); shift() here is what makes a removed entry unreachable. - next = entry.queue.shift(); + // Stop pressed during this pass: leave reports parked, but never park + // submitted user messages. A notice pass can receive sends while awaiting. + if (!entry.holdDeliveries && !entry.quiescing) await this.drainDeliveries(sessionId, entry, 'turn'); + if (this.live.get(sessionId) !== entry || entry.quiescing) return; + next = entry.queue[0]?.ready === false ? undefined : entry.queue.shift(); + if (next === undefined) break; } - // Stop pressed during this pass: leave everything parked (holdDeliveries' - // own WHY). The next user message drains it via the splice above. - if (!entry.holdDeliveries) await this.drainDeliveries(sessionId, entry, 'turn'); } finally { entry.inFlight = false; } diff --git a/desktop/src/main/harness/prompt-assembly.ts b/desktop/src/main/harness/prompt-assembly.ts index 45f73bdc1..8200d1a67 100644 --- a/desktop/src/main/harness/prompt-assembly.ts +++ b/desktop/src/main/harness/prompt-assembly.ts @@ -4,8 +4,9 @@ // // WHY not reuse project-context.ts / context-discovery.ts: the former is a pure // mapper over pre-computed basenames and the latter only scans the exact project -// dir + .claude (async, for the context UI). Neither does the session-start -// walk-up-to-git-root that the assembled prompt needs, so this owns its own IO. +// dir + .claude (async, for the context UI). Production now supplies the +// asynchronously captured ancestor inventory; the sync reader remains for +// legacy single-file callers and Claude Code context compatibility. import * as fs from 'fs'; import * as path from 'path'; import { execFile, execFileSync } from 'child_process'; @@ -14,6 +15,7 @@ import type { PromptVariant } from './capability-profile'; import { variantOverlay } from './prompts/variants'; import { fitProjectInstructions } from './injection/injection-budget'; import { sharedDoctrine } from './prompts/shared-doctrine'; +import { renderProjectInstructionFiles, type ProjectInstructionFile } from './injection/project-instructions'; // promptVariant is the capability-profile steering overlay (see prompts/variants.ts). // Optional so pre-variant callers assemble byte-identically; only local-small adds text. @@ -38,7 +40,7 @@ const DEFAULT_INSTRUCTION_BUDGET_TOKENS = 20_000; // gitSnapshot: the git line, computed AHEAD by the caller with // gitSnapshotAsync (2026-09-16 C3). When absent the sync shell-out below runs, // which only the evaluator and tests should reach — see the WHY on gitSnapshotAsync. -export interface PromptInputs { presetBody: string; cwd: string; appVersion: string; promptVariant?: PromptVariant; hasTools?: boolean; instructionBudgetTokens?: number; supportsParallelToolCalls?: boolean; audience?: 'user' | 'parent'; presetName?: string; gitSnapshot?: string } +export interface PromptInputs { presetBody: string; cwd: string; appVersion: string; promptVariant?: PromptVariant; hasTools?: boolean; instructionBudgetTokens?: number; supportsParallelToolCalls?: boolean; audience?: 'user' | 'parent'; presetName?: string; gitSnapshot?: string; projectInstructionFiles?: readonly ProjectInstructionFile[] } const GIT_TIMEOUT_MS = 3000; @@ -107,7 +109,10 @@ export function findProjectInstructions(cwd: string): { path: string; name: stri return null; } -function projectInstructions(cwd: string, budgetTokens: number): string | null { +function projectInstructions(cwd: string, budgetTokens: number, files?: readonly ProjectInstructionFile[]): string | null { + // WHY: the host supplies one captured startup snapshot; neither the prompt nor + // the panel may repeat discovery after disk contents have changed. + if (files) return renderProjectInstructionFiles(files); const found = findProjectInstructions(cwd); if (!found) return null; // The budget bounds the FILE BODY only — the wrapping tag is added after, @@ -159,7 +164,7 @@ export function assembleSystemPromptParts(i: PromptInputs): PromptPart[] { text: 'You are the YouCoded assistant, an agentic AI running inside the YouCoded app. You may be running on any model the user chose, cloud or local — Claude, GPT, Grok, Gemini, Qwen, Gemma and others.', }, { id: 'preset', label: i.presetName ? `Its preset — ${i.presetName}` : 'Its preset', text: i.presetBody }, - partOrNull('project', 'Your project instructions', projectInstructions(i.cwd, i.instructionBudgetTokens ?? DEFAULT_INSTRUCTION_BUDGET_TOKENS)), + partOrNull('project', 'Your project instructions', projectInstructions(i.cwd, i.instructionBudgetTokens ?? DEFAULT_INSTRUCTION_BUDGET_TOKENS, i.projectInstructionFiles)), { id: 'doctrine', label: 'How it works', diff --git a/desktop/src/main/harness/shell-registry.ts b/desktop/src/main/harness/shell-registry.ts index 49f2b7ce8..fa792712d 100644 --- a/desktop/src/main/harness/shell-registry.ts +++ b/desktop/src/main/harness/shell-registry.ts @@ -270,11 +270,19 @@ export class ShellRegistry extends EventEmitter { } catch (e: any) { return { ok: false, reason: 'spawn-failed', detail: e?.message ?? String(e) }; } - const run = this.register({ - toolUseId: spec.toolUseId, command: spec.command, cwd: spec.cwd, child, - startedAt: Date.now(), seedLog: null, recent: '', logPath: null, logStream: null, captureEnv: false, - }, { detached: false, explicit: true }); - return { ok: true, run, runningExplicit: running.length + 1 }; + try { + const run = this.register({ + toolUseId: spec.toolUseId, command: spec.command, cwd: spec.cwd, child, + startedAt: Date.now(), seedLog: null, recent: '', logPath: null, logStream: null, captureEnv: false, + }, { detached: false, explicit: true }); + return { ok: true, run, runningExplicit: running.length + 1 }; + } catch (e: any) { + // WHY: a spawned but unregistered child is still ours to stop; give the + // caller a real setup failure, never an id for a run no registry owns. + child.on('error', () => {}); // late spawn errors cannot crash main + if (child.exitCode === null && child.signalCode === null) killTree(child, { graceMs: 0 }); + return { ok: false, reason: 'spawn-failed', detail: e?.message ?? String(e) }; + } } /** Hand-off (spec §5.5): the same process bash.ts already spawned, adopted @@ -296,6 +304,9 @@ export class ShellRegistry extends EventEmitter { fs.mkdirSync(dir, { recursive: true }); logPath = path.join(dir, `bash-${Date.now()}-${shellId}.txt`); logStream = fs.createWriteStream(logPath); + // WHY: opening is asynchronous; an EACCES can arrive before the sweep or + // the registry's run listeners. Keep the tail as the fallback, not a crash. + logStream.on('error', () => {}); // The 7-day sweep used to fire only from bash.ts's foreground spill; a // user whose long commands all run in the background would never have // triggered it, and these logs would pile up forever (2026-08-28 review). diff --git a/desktop/src/main/harness/tool-args.ts b/desktop/src/main/harness/tool-args.ts new file mode 100644 index 000000000..0ecbde166 --- /dev/null +++ b/desktop/src/main/harness/tool-args.ts @@ -0,0 +1,17 @@ +import type { NativeTool } from './tools/types'; + +/** WHY: the pre-write guidance gate must see exactly the same validated, + * normalized path as execution; invalid args remain ordinary tool errors. */ +export function parseToolArgs(tool: NativeTool, input: unknown): ReturnType { + let parsed = tool.inputSchema.safeParse(input); + if (!parsed.success && typeof input === 'string') { + try { + const recovered: unknown = JSON.parse(input); + if (recovered && typeof recovered === 'object') { + const reparsed = tool.inputSchema.safeParse(recovered); + if (reparsed.success) parsed = reparsed; + } + } catch { /* Let the normal argument-error result explain the invalid input. */ } + } + return parsed; +} diff --git a/desktop/src/main/harness/tool-group-finalization.ts b/desktop/src/main/harness/tool-group-finalization.ts new file mode 100644 index 000000000..ebe5dc414 --- /dev/null +++ b/desktop/src/main/harness/tool-group-finalization.ts @@ -0,0 +1,27 @@ +// WHY: a permission callback may fail after the assistant has announced an entire +// group. Tag only pre-execution callback failures; a started action is not retried +// or silently reclassified as an approval/denial. +export class PermissionCallbackFailure { + constructor(readonly cause: unknown) {} +} + +export async function permissionCallback(call: () => Promise): Promise { + try { return await call(); } + catch (err) { throw new PermissionCallbackFailure(err); } +} + +/** Emit exactly one result for each not-yet-recorded call; the caller retains + * completed results and commits the complete tool message and event origins. */ +export function finalizeRemainingCalls( + calls: readonly TCall[], start: number, textFor: (index: number) => string, + emit: (call: TCall, text: string) => string, part: (call: TCall, text: string) => TPart, +): { parts: TPart[]; origins: string[] } { + const parts: TPart[] = []; + const origins: string[] = []; + for (let i = start; i < calls.length; i++) { + const text = textFor(i); + origins.push(emit(calls[i], text)); + parts.push(part(calls[i], text)); + } + return { parts, origins }; +} diff --git a/desktop/src/main/harness/tools/ask-user-question.ts b/desktop/src/main/harness/tools/ask-user-question.ts index 06ef4ad60..7e1bc38a0 100644 --- a/desktop/src/main/harness/tools/ask-user-question.ts +++ b/desktop/src/main/harness/tools/ask-user-question.ts @@ -48,15 +48,25 @@ export function formatAnswers(args: AskUserQuestionInput, updatedInput: Record; - const lines = args.questions.map((qq) => { - const answer = renderAnswer(answers[qq.question]); + // WHY: the legacy text-keyed map cannot distinguish identical wording. Only + // accept a COMPLETE, well-typed ordered response; malformed remote clients + // fall back to the old map without ever creating a dangling tool result. + const rawOrdered = updatedInput?.orderedAnswers; + const ordered = Array.isArray(rawOrdered) && rawOrdered.length === args.questions.length + && rawOrdered.every((item) => item && typeof item === 'object' && !Array.isArray(item) + && (typeof item.answer === 'string' || (Array.isArray(item.answer) && item.answer.every((v: unknown) => typeof v === 'string'))) + && (item.note === undefined || typeof item.note === 'string')) + ? rawOrdered as Array<{ answer: string | string[]; note?: string }> : null; + const lines = args.questions.map((qq, index) => { + const answer = renderAnswer(ordered ? ordered[index].answer : answers[qq.question]); // A typed "Other" answer arrives as plain text in the answer slot. Tell the // model when what came back was NOT one of its options, so it reads the // text as the user's own answer instead of hunting for a matching label. const labels = new Set(qq.options.map((o) => o.label)); const parts = answer === NO_SELECTION ? [] : answer.split(', '); const ownAnswer = parts.length > 0 && parts.some((p) => !labels.has(p)); - const note = typeof notes[qq.question] === 'string' ? (notes[qq.question] as string).trim() : ''; + const note = ordered ? (ordered[index].note ?? '').trim() + : typeof notes[qq.question] === 'string' ? (notes[qq.question] as string).trim() : ''; let out = `Q: ${qq.question}\nA${ownAnswer ? ' (the user typed their own answer)' : ''}: ${answer}`; if (note) out += `\nNote from the user: ${note}`; return out; diff --git a/desktop/src/main/harness/tools/bash.ts b/desktop/src/main/harness/tools/bash.ts index cdf799862..46f80ee99 100644 --- a/desktop/src/main/harness/tools/bash.ts +++ b/desktop/src/main/harness/tools/bash.ts @@ -658,8 +658,10 @@ export const BashTool = defineTool({ tailBuf = (tailBuf + s).slice(-TAIL_RETAIN_CHARS); if (probe) probeTail = (probeTail + s).slice(-4096); }; - child.stdout.on('data', (d) => cap(String(d))); - child.stderr.on('data', (d) => cap(String(d))); + const onStdout = (d: Buffer) => cap(String(d)); + const onStderr = (d: Buffer) => cap(String(d)); + child.stdout.on('data', onStdout); + child.stderr.on('data', onStderr); let done = false; /** G-1: set by the timeout timer once the registry adopted this process. */ let handedOffTo: string | null = null; @@ -946,6 +948,7 @@ export const BashTool = defineTool({ // restarted — and this call resolves with the output so far. Two // cases keep the old kill: no registry in this context (tests, // one-off tools), and a leading `sleep` (backgrounding a sleep is churn). + if (done) return; // A close/abort that won the deadline retains its result. if (!ctx.shells || LEADING_SLEEP.test(args.command)) { killTree(child, { graceMs: 0 }); finish( @@ -956,24 +959,37 @@ export const BashTool = defineTool({ ); return; } - // From here the registry reads the pipes; this call must stop - // listening or every byte would be counted twice. - child.stdout.removeAllListeners('data'); - child.stderr.removeAllListeners('data'); - child.removeAllListeners('close'); - child.removeAllListeners('error'); + // WHY: registration can throw before ownership transfers. Keep foreground + // listeners until it succeeds, then remove only OUR listeners (not the + // registry's) so no output or close/error is lost during a failed setup. + let run; + try { + run = ctx.shells.adopt({ + toolUseId: ctx.toolCallId ?? 'unknown', command: args.command, cwd: startCwd, child, startedAt, + seedLog: spillStream ? null : headBuf, + recent: tailBuf, logPath: spillPath, logStream: spillStream, captureEnv, + }); + } catch (e: any) { + // A failed handoff must neither strand the Bash promise nor leave an + // untracked command running. C2's deferred descendant guarantees do + // not follow from this best-effort stop of our freshly spawned child. + if (child.exitCode === null && child.signalCode === null) killTree(child, { graceMs: 0 }); + finish(`Failed to hand off background command: ${e?.message ?? e}. Cleanup was attempted; process exit was not confirmed.\n`, true); + return; + } + child.stdout.off('data', onStdout); + child.stderr.off('data', onStderr); + child.off('close', onClose); + child.off('error', onError); + if (done) { + // An abort/close can settle during a re-entrant adoption callback. + // The registry now owns this run; stop it there, never report success. + void ctx.shells.kill(run.shellId, 'user').catch(() => {}); + return; + } // D4: a handed-off command can no longer be answered — close stdin so // a prompt fails fast with its own error instead of hanging forever. try { child.stdin?.end(); } catch { /* already closed */ } - const run = ctx.shells.adopt({ - toolUseId: ctx.toolCallId ?? 'unknown', command: args.command, cwd: startCwd, child, startedAt, - // headBuf is complete when no spill has started (nothing dropped yet); - // otherwise the spill stream already holds everything. - seedLog: spillStream ? null : headBuf, - recent: tailBuf, - logPath: spillPath, logStream: spillStream, - captureEnv, - }); handedOffTo = run.shellId; spillPath = run.logPath; finish( @@ -1003,12 +1019,14 @@ export const BashTool = defineTool({ // Async spawn failure (the path Windows takes for a bad cwd): name the // shell + cwd actually used, not just Node's bare `spawn ` — // same diagnosability contract as the sync catch above. - child.on('error', (err) => finish(`Failed to start shell: ${err.message} (shell=${shell.cmd}; cwd=${startCwd})\n`, true)); + const onError = (err: Error) => finish(`Failed to start shell: ${err.message} (shell=${shell.cmd}; cwd=${startCwd})\n`, true); + child.on('error', onError); // WHY no exit-code prefix here anymore: the metadata line above now states // `exit N` for every result, so a leading "(exit code N)" duplicated the // same fact in two places. The timeout/abort handlers keep their prefixes — // those are messages, not exit codes. - child.on('close', (code) => finish('', code !== 0, code)); + const onClose = (code: number | null) => finish('', code !== 0, code); + child.on('close', onClose); }); }, }); diff --git a/desktop/src/main/harness/tools/net-guard.ts b/desktop/src/main/harness/tools/net-guard.ts index f27ba56d3..6947a908b 100644 --- a/desktop/src/main/harness/tools/net-guard.ts +++ b/desktop/src/main/harness/tools/net-guard.ts @@ -53,7 +53,8 @@ export function isPrivateIp(ip: string): boolean { /** Scheme + address validation for ONE URL. Throws NetGuardError with an honest, * specific message (docs/error-message-standards.md). Returns the parsed URL. */ -export async function assertPublicHttpUrl(raw: string, lookup: LookupFn = defaultLookup): Promise { +export async function assertPublicHttpUrl(raw: string, lookup: LookupFn = defaultLookup, signal?: AbortSignal): Promise { + signal?.throwIfAborted(); let url: URL; try { url = new URL(raw); } catch { throw new NetGuardError(`"${raw}" is not a valid URL.`); } if (url.protocol !== 'http:' && url.protocol !== 'https:') { @@ -69,10 +70,32 @@ export async function assertPublicHttpUrl(raw: string, lookup: LookupFn = defaul if (host === 'localhost' || host.endsWith('.localhost') || host.endsWith('.local')) { throw new NetGuardError(`${host} is a local address — fetching it is blocked.`); } + // WHY: DNS promises do not accept a portable cancellation signal. Settle our + // wait on abort while consuming any late lookup result/rejection. The listener + // is scoped to this hop; the same signal/deadline is shared by every hop. + const resolveHost = () => Promise.resolve().then(() => { + signal?.throwIfAborted(); + try { + return Promise.resolve(lookup(host)).catch(() => { + throw new NetGuardError(`Could not resolve ${host} — check the URL or the network connection.`); + }); + } catch { + throw new NetGuardError(`Could not resolve ${host} — check the URL or the network connection.`); + } + }); let addrs: Array<{ address: string }>; - try { addrs = await lookup(host); } catch { - throw new NetGuardError(`Could not resolve ${host} — check the URL or the network connection.`); - } + if (signal) { + let onAbort!: () => void; + const aborted = new Promise((_resolve, reject) => { + onAbort = () => reject(signal.reason); + signal.addEventListener('abort', onAbort, { once: true }); + }); + try { + signal.throwIfAborted(); + addrs = await Promise.race([resolveHost(), aborted]); + } finally { signal.removeEventListener('abort', onAbort); } + } else addrs = await resolveHost(); + signal?.throwIfAborted(); if (addrs.length === 0) throw new NetGuardError(`Could not resolve ${host}.`); const bad = addrs.find((a) => isPrivateIp(a.address)); if (bad) throw new NetGuardError(`${host} resolves to the private/internal address ${bad.address} — fetching it is blocked.`); @@ -122,8 +145,8 @@ export interface GuardedFetchOpts { } /** Fetch with MANUAL redirect following: every hop re-runs assertPublicHttpUrl. - * Timeout/abort surfaces as a DOMException (AbortError) from fetch — NOT a - * NetGuardError — which the tool layer translates for the user (spec §3.1). */ + * Caller abort / deadline (AbortError / TimeoutError) also ends DNS waiting; + * neither is a NetGuardError or proof that OS resolution was canceled. */ export async function guardedFetch(rawUrl: string, opts: GuardedFetchOpts): Promise<{ res: Response; finalUrl: string }> { const fetchImpl = opts.fetchImpl ?? fetch; const lookup = opts.lookup ?? defaultLookup; @@ -136,7 +159,7 @@ export async function guardedFetch(rawUrl: string, opts: GuardedFetchOpts): Prom let method = (opts.method ?? 'GET').toUpperCase(); let body = opts.body; for (let hop = 0; ; hop++) { - const url = await assertPublicHttpUrl(current, lookup); + const url = await assertPublicHttpUrl(current, lookup, deadline); const host = url.hostname.toLowerCase(); if (originHost === null) originHost = host; const decision = opts.allowHost?.(url.hostname, hop); @@ -147,6 +170,7 @@ export async function guardedFetch(rawUrl: string, opts: GuardedFetchOpts): Prom const onOrigin = host === originHost; if (!onOrigin) for (const p of opts.credentialQueryParams ?? []) url.searchParams.delete(p); const target = url.toString(); + deadline.throwIfAborted(); // no dispatch after DNS or allowHost used the remaining budget const res = await fetchImpl(target, { redirect: 'manual', method, diff --git a/desktop/src/main/harness/tools/read.ts b/desktop/src/main/harness/tools/read.ts index 604ef7a30..1829adf17 100644 --- a/desktop/src/main/harness/tools/read.ts +++ b/desktop/src/main/harness/tools/read.ts @@ -203,28 +203,8 @@ export const ReadTool = defineTool({ const offset = args.offset ?? 1; const limit = Math.min(args.limit ?? 2000, 2000); const canonical = canonicalize(args.file_path, ctx.cwd); - // G-11 (2026-08-26 tools investigation): the same slice of an UNCHANGED file - // was already served this session — the model still has it (the session - // forgets these marks whenever history is discarded or shrunk, so this is - // never claimed across a compaction or resume). A short notice beats a - // second copy: Claude Code and Hermes both do this. Checked BEFORE the file - // is read so the repeat costs a stat and nothing else. Still counts as a - // Read for the edit gate — the registry stamp happens here as well as on the - // full path below (a binary file is only stamped AFTER it passes the binary - // refusal, so a refused Read never satisfies the gate; a dedupe hit can - // only be for a file that already passed it). const servedKey = `${canonical}|${offset}|${limit}`; const prior = ctx.servedReads?.get(servedKey); - if (prior && prior.mtimeMs === st.mtimeMs) { - ctx.readRegistry.set(canonical, prior.fingerprint); - const ago = ctx.toolCallIndex !== undefined ? ctx.toolCallIndex - prior.callIndex : undefined; - const when = ago !== undefined ? `(${ago} call${ago === 1 ? '' : 's'} ago)` : '(earlier this session)'; - return { - text: `Read ${args.file_path}: lines ${prior.from}–${prior.to} — ` - + `Unchanged since your earlier Read this session ${when} — the content you already have is current. ` - + 'Use a different offset/limit to see another part of the file.', - }; - } // fs.promises (2026-09-16 C4): up to MAX_READ_BYTES used to be read // synchronously on the main thread, several times per turn. const buf = await fs.promises.readFile(abs); @@ -235,15 +215,25 @@ export const ReadTool = defineTool({ // 4) — drop it so line counts and the paging trailer are honest. if (raw.endsWith('\n')) all.pop(); const totalLines = all.length; - // Record for the read-before-edit gate (a content fingerprint, so a later - // external change invalidates it and a mere touch does not) — the file - // exists and was readable, so it counts as read even if the requested page - // is past EOF. - const fingerprint = fingerprintOf(buf); - ctx.readRegistry.set(canonical, fingerprint); if (offset > totalLines) { return { text: `Read failed: ${args.file_path}: offset ${offset} is past the end of the file (${totalLines} lines).`, isError: true }; } + // WHY: mtime is not identity. A replacement can preserve both mtime and + // size, while a touch changes only mtime. Verify the SAME bounded async + // buffer that passed the binary/offset checks and supplies the text below; + // no stale prior fingerprint may refresh the edit gate on a refused read. + const fingerprint = fingerprintOf(buf); + if (prior?.fingerprint === fingerprint) { + ctx.readRegistry.set(canonical, fingerprint); + const ago = ctx.toolCallIndex !== undefined ? ctx.toolCallIndex - prior.callIndex : undefined; + const when = ago !== undefined ? `(${ago} call${ago === 1 ? '' : 's'} ago)` : '(earlier this session)'; + return { + text: `Read ${args.file_path}: lines ${prior.from}–${prior.to} — ` + + `Unchanged since your earlier Read this session ${when} — the content you already have is current. ` + + 'Use a different offset/limit to see another part of the file.', + }; + } + ctx.readRegistry.set(canonical, fingerprint); const slice = all.slice(offset - 1, offset - 1 + limit); const MAX_LINE = 2000; // G-5: a second, per-call cap in CHARS on top of the line cap — 2,000 lines diff --git a/desktop/src/main/harness/tools/types.ts b/desktop/src/main/harness/tools/types.ts index c1a4df084..6abec7708 100644 --- a/desktop/src/main/harness/tools/types.ts +++ b/desktop/src/main/harness/tools/types.ts @@ -212,13 +212,10 @@ export interface ToolServices { }; } -/** What an earlier Read served (G-11): the file's mtime at that moment, which - * tool call did it, and the line range it showed. */ +/** What an earlier text Read served: verified byte identity, call and slice. */ export interface ServedRead { mtimeMs: number; - /** The content fingerprint recorded for the read gate at that Read, so a - * dedupe hit (mtime unchanged) can re-stamp the registry without reading - * the file again (2026-09-16). */ + /** Compare against freshly read, validated bytes before claiming unchanged. */ fingerprint: string; callIndex: number; from: number; diff --git a/desktop/src/renderer/components/InputBar.test.tsx b/desktop/src/renderer/components/InputBar.test.tsx index 74b211230..031ed9574 100644 --- a/desktop/src/renderer/components/InputBar.test.tsx +++ b/desktop/src/renderer/components/InputBar.test.tsx @@ -3,9 +3,10 @@ import '@testing-library/jest-dom/vitest'; import React from 'react'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, cleanup, fireEvent, waitFor, act } from '@testing-library/react'; -import { ChatProvider, useChatDispatch, useChatStore } from '../state/chat-context'; +import { ChatProvider, useChatDispatch, useChatStore, useChatState } from '../state/chat-context'; import { SkillProvider } from '../state/skill-context'; import InputBar, { InputBarHandle } from './InputBar'; +import QueuedMessagesStrip from './QueuedMessagesStrip'; import { buildContextMenu } from './context-menu/build-menu'; import type { VoiceEvent, VoiceReadiness } from '../../shared/voice-types'; @@ -472,6 +473,56 @@ describe('InputBar — stop button (Task 10 placement)', () => { expect(screen.queryByRole('button', { name: 'Stop generating' })).not.toBeInTheDocument(); }); + it('keeps Stop beside native busy queued input, with only Edit and Cancel on its queued row', () => { + const onEdit = vi.fn(); + const onCancel = vi.fn(); + function QueuedFromStore() { + const state = useChatState('sess-1'); + return ; + } + render( + + + + + + + , + ); + act(() => { + capturedDispatch!({ type: 'SESSION_INIT', sessionId: 'sess-1' }); + capturedDispatch!({ type: 'USER_PROMPT', sessionId: 'sess-1', content: 'first action', timestamp: 1 }); + capturedDispatch!({ type: 'QUEUED_MESSAGE_ADDED', sessionId: 'sess-1', queueId: 'q-1', content: 'follow up', timestamp: 2 }); + }); + const stop = screen.getByRole('button', { name: 'Stop generating' }); + expect(stop).toBeEnabled(); + expect(screen.getByRole('button', { name: 'Send message' })).toBeInTheDocument(); + const strip = screen.getByLabelText('Queued messages'); + const queuedActions = () => [...strip.querySelectorAll('button')].map(button => button.getAttribute('aria-label')); + // An exact DOM inventory detects any extra urgent-send control, regardless + // of its label. The separate forbidden-name check catches one elsewhere. + const expected = ['Edit queued message', 'Cancel queued message']; + expect(queuedActions()).toEqual(expected); + const injected = document.createElement('button'); + injected.setAttribute('aria-label', 'Send now'); + strip.append(injected); + expect(() => expect(queuedActions()).toEqual(expected)).toThrow(); + injected.remove(); + expect(screen.queryByRole('button', { name: /send now|after this finishes|stop and send|steer|urgent|priority/i })).not.toBeInTheDocument(); + fireEvent.click(stop); + expect((window as any).claude.native.interrupt).toHaveBeenCalledWith('sess-1'); + expect((window as any).claude.session.sendInput).not.toHaveBeenCalled(); + fireEvent.click(screen.getByRole('button', { name: 'Edit queued message' })); + fireEvent.click(screen.getByRole('button', { name: 'Cancel queued message' })); + expect(onEdit).toHaveBeenCalledWith('q-1', 'follow up'); + expect(onCancel).toHaveBeenCalledWith('q-1'); + act(() => { + capturedDispatch!({ type: 'PERMISSION_REQUEST', sessionId: 'sess-1', toolName: 'Bash', input: { command: 'build' }, requestId: 'req-1' }); + }); + expect(screen.getByRole('button', { name: 'Stop generating' })).toBeEnabled(); + expect(queuedActions()).toEqual(expected); + }); + it('provider="native": click calls native.interrupt, never session.sendInput', () => { renderInputBar('native'); act(() => { capturedDispatch!({ type: 'SESSION_INIT', sessionId: 'sess-1' }); }); diff --git a/desktop/src/renderer/components/SessionContextPopup.tsx b/desktop/src/renderer/components/SessionContextPopup.tsx index 8ce3c5556..d34c8e1be 100644 --- a/desktop/src/renderer/components/SessionContextPopup.tsx +++ b/desktop/src/renderer/components/SessionContextPopup.tsx @@ -215,16 +215,17 @@ function FileText({ sessionId, kind, id, what }: { sessionId?: string; kind: 'pr * Claude Code chat it means only that WE did not cut it — Claude Code manages * its own window and we cannot see what it did. Saying "read in full" there * would be a claim about someone else's work. */ -function RulesCard({ file, kind, label, sessionId, openFile, assembledByClaudeCode }: { - file: { path: string; truncated: boolean }; +function RulesCard({ file, kind, label, sessionId, openFile, assembledByClaudeCode, id }: { + file: { path: string; truncated: boolean; note?: string | null }; kind: 'project' | 'user'; + id?: string; label: string; sessionId?: string; openFile: (p: string) => void | Promise; assembledByClaudeCode: boolean; }) { const description = file.truncated - ? 'Shortened to headings only' + ? (file.note ?? 'Shortened to fit') : assembledByClaudeCode ? 'YouCoded didn’t shorten it' : 'Read in full'; @@ -241,7 +242,7 @@ function RulesCard({ file, kind, label, sessionId, openFile, assembledByClaudeCo /> )} > - + ); } @@ -352,7 +353,10 @@ function SessionContextPanel({ open, onClose, context, sessionId }: Props & { co const [tab, setTab] = useState('overview'); const openFile = useOpenFilepath(sessionId); - const rules = context.projectInstructions ?? null; + // WHY: older native/Claude Code records still have one file; captured native + // sessions list the actual broad-to-narrow chain without a second selection. + const projectFiles = context.projectInstructionFiles ?? (context.projectInstructions ? [context.projectInstructions] : []); + const rules = projectFiles[projectFiles.length - 1] ?? null; const skills = context.skills ?? []; const tools = context.tools ?? []; const dropped = context.droppedMcpServers ?? []; @@ -435,12 +439,12 @@ function SessionContextPanel({ open, onClose, context, sessionId }: Props & { co

What was left out

- {rules?.truncated && ( + {projectFiles.some(f => f.truncated) && ( } title="This project’s rules" - description={`Shortened to headings only · ${basename(rules.path)}`} + description={`${projectFiles.filter(f => f.truncated).length} instruction file(s) shortened`} onClick={() => setTab('project')} /> )} @@ -480,7 +484,7 @@ function SessionContextPanel({ open, onClose, context, sessionId }: Props & { co 0 ? toolsWord : null].filter(Boolean).join(' · ')} + value={[projectFiles.length ? `${projectFiles.length} rules file${projectFiles.length === 1 ? '' : 's'}` : null, skillsWord, tools.length > 0 ? toolsWord : null].filter(Boolean).join(' · ')} />
@@ -534,16 +538,18 @@ function SessionContextPanel({ open, onClose, context, sessionId }: Props & { co

There is no rules file for this project, and none of your own.

) : (
- {rules && ( + {projectFiles.map((file) => ( - )} + ))} {/* Your own rules, the ones that apply everywhere. Claude Code reads this file; the native harness does not, so this card appears on a Claude Code chat and not on a native one — the diff --git a/desktop/src/renderer/components/ToolCard.tsx b/desktop/src/renderer/components/ToolCard.tsx index ef810fbe0..1fb5142c8 100644 --- a/desktop/src/renderer/components/ToolCard.tsx +++ b/desktop/src/renderer/components/ToolCard.tsx @@ -17,6 +17,7 @@ import { useExpandAllToggle, getInitialExpanded } from '../hooks/useExpandAllTog import { isTypingTarget } from '../utils/is-typing-target'; import { useCardKeysLive } from '../state/card-keys-context'; import { asString } from '../utils/tool-input'; +import { normalizeQuestions, isValidQuestions } from './ask-question-normalize'; // Full-auto safety stop (spec 2026-08-12, M5 2b): per-family copy + the // status-bar chip colors, so the footer band can never drift from the chip. import { FullAutoStops, BROAD_ALLOW_COLORS } from './permissions/FullAutoStops'; @@ -923,45 +924,6 @@ export function PermissionButtons({ requestId, suggestions, denyListed, command, // Unlike regular tools (allow/deny), we must collect the user's selections // and return them via updatedInput.answers so Claude gets the actual answer. -interface AskQuestion { - question: string; - header: string; - options: Array<{ label: string; description?: string }>; - multiSelect: boolean; -} - -// Fix: isValidQuestions only checked questions[0].question, so an object-valued -// header/label/description DEEPER in the array still reached React children in -// the expanded card ("Objects are not valid as a React child" crashes the whole -// Chat pane via its ErrorBoundary). Normalize the whole array up front with the -// same asString idiom as friendlyToolDisplay: coerce every rendered field, and -// drop members that can't render or be answered (no question text, no labeled -// options) so one malformed member degrades gracefully instead of crashing or -// invalidating the rest of the card. -function normalizeQuestions(input: Record): AskQuestion[] { - const raw = input.questions; - if (!Array.isArray(raw)) return []; - const out: AskQuestion[] = []; - for (const q of raw as Array | null | undefined>) { - const question = asString(q?.question); - if (!question) continue; // no question text — nothing to render or key answers by - const rawOptions = Array.isArray(q?.options) ? (q.options as Array | null | undefined>) : []; - const options: AskQuestion['options'] = []; - for (const o of rawOptions) { - const label = asString(o?.label); - if (!label) continue; // an unlabeled option can't be selected or echoed back - options.push({ label, description: asString(o?.description) || undefined }); - } - if (options.length === 0) continue; // nothing selectable - out.push({ question, header: asString(q?.header), multiSelect: q?.multiSelect === true, options }); - } - return out; -} - -function isValidQuestions(input: Record): boolean { - return normalizeQuestions(input).length > 0; -} - // Ledger G-2 (2026-08-26 harness comparison): every other harness lets the user // type their own answer; ours only offered the listed choices, and Dismiss ended // the turn. The card now adds an "Other" row per question plus one text box @@ -991,11 +953,12 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: { // see normalizeQuestions above. Memoized so the array identity is stable for // the hook deps below. const questions = useMemo(() => normalizeQuestions(tool.input), [tool.input]); - // answers: map from question text → selected label(s) + const isNative = requestId.startsWith('native-'); + // WHY: native asks have independent request-local positions; Claude Code's + // legacy answer map and card state were keyed by question wording. Q22 keeps + // that CC behavior, including duplicates, without inventing CC transport IDs. + const keyFor = (question: string, index: number) => isNative ? String(index) : question; const [answers, setAnswers] = useState>>({}); - // text: question text → what the user typed in that question's box. One - // state serves both roles (Other's answer, or a note on a listed choice); - // which role it plays is decided at submit from the selection. const [text, setText] = useState>({}); const textRefs = useRef>({}); const [responding, setResponding] = useState(false); @@ -1005,18 +968,18 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: { // Track which question is "active" for keyboard nav, and which option is focused const [focusedOption, setFocusedOption] = useState(0); - const allAnswered = questions.every(q => { - const sel = answers[q.question]; + const allAnswered = questions.every((q, qi) => { + const sel = answers[keyFor(q.question, qi)]; if (!sel || sel.size === 0) return false; // "Other" with nothing typed is not an answer yet — Submit stays off until // the user explains, so the model never receives an empty "own answer". - if (sel.has(OTHER) && !(text[q.question] ?? '').trim()) return false; + if (sel.has(OTHER) && !(text[keyFor(q.question, qi)] ?? '').trim()) return false; return true; }); - const handleSelect = useCallback((question: string, label: string, multiSelect: boolean) => { + const handleSelect = useCallback((key: string, label: string, multiSelect: boolean) => { setAnswers(prev => { - const current = prev[question] || new Set(); + const current = prev[key] || new Set(); const next = new Set(current); if (multiSelect) { // Toggle for multi-select @@ -1027,12 +990,12 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: { next.clear(); next.add(label); } - return { ...prev, [question]: next }; + return { ...prev, [key]: next }; }); // Picking Other is an invitation to type — put the cursor in the box so the // user doesn't have to click twice. (Deselecting Other leaves focus alone.) if (label === OTHER) { - requestAnimationFrame(() => textRefs.current[question]?.focus()); + requestAnimationFrame(() => textRefs.current[key]?.focus()); } }, []); @@ -1051,12 +1014,15 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: { // `annotations[question].notes` — send that shape too so a CC session // receives the note through the same contract its CLI uses. const annotationsObj: Record = {}; - for (const q of questions) { - const sel = answers[q.question] ?? new Set(); - const typed = (text[q.question] ?? '').trim(); + const orderedAnswers: Array<{ answer: string; note?: string }> = []; + for (const [qi, q] of questions.entries()) { + const key = keyFor(q.question, qi); + const sel = answers[key] ?? new Set(); + const typed = (text[key] ?? '').trim(); const labels = Array.from(sel).filter((l) => l !== OTHER); if (sel.has(OTHER) && typed) labels.push(typed); answersObj[q.question] = labels.join(', '); + orderedAnswers.push({ answer: labels.join(', '), ...(!sel.has(OTHER) && typed ? { note: typed } : {}) }); if (!sel.has(OTHER) && typed) { notesObj[q.question] = typed; annotationsObj[q.question] = { notes: typed }; @@ -1071,6 +1037,9 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: { questions, // Echo back the (normalized) questions array answers: answersObj, ...(hasNotes ? { notes: notesObj, annotations: annotationsObj } : {}), + // Native broker alone understands this field. Never add it to CC's + // documented question-text-keyed response envelope. + ...(isNative ? { orderedAnswers } : {}), }, }, }); @@ -1088,7 +1057,7 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: { // The card stays answerable and says the answer could not be confirmed. setUnconfirmed(true); } - }, [allAnswered, responding, questions, answers, text, requestId, onResponded, onFailed]); + }, [allAnswered, responding, questions, answers, text, requestId, isNative, onResponded, onFailed]); const handleDeny = useCallback(async () => { setResponding(true); @@ -1113,9 +1082,9 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: { // Total flat list of all options across questions (for keyboard nav). The // synthetic Other row is a real stop in the list, after each question's own // options, so arrow keys reach it like any other choice. - const allOptions = questions.flatMap(q => [ - ...q.options.map(o => ({ q, o })), - { q, o: { label: OTHER, description: OTHER_DESCRIPTION } }, + const allOptions = questions.flatMap((q, qi) => [ + ...q.options.map(o => ({ q, qi, o })), + { q, qi, o: { label: OTHER, description: OTHER_DESCRIPTION } }, ]); const optionCount = allOptions.length; @@ -1138,8 +1107,8 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: { e.preventDefault(); // Select the focused option if (optionCount > 0) { - const { q, o } = allOptions[focusedOption]; - handleSelect(q.question, o.label, q.multiSelect); + const { q, qi, o } = allOptions[focusedOption]; + handleSelect(keyFor(q.question, qi), o.label, q.multiSelect); } } else if (e.key === 'Enter' && (e.ctrlKey || e.metaKey)) { // Ctrl/Cmd+Enter to submit @@ -1165,7 +1134,7 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: {
{[...q.options, { label: OTHER, description: OTHER_DESCRIPTION }].map((opt, oi) => { const idx = flatIdx++; - const selected = answers[q.question]?.has(opt.label) ?? false; + const selected = answers[keyFor(q.question, qi)]?.has(opt.label) ?? false; const focused = idx === focusedOption; const isOther = opt.label === OTHER; return ( @@ -1173,7 +1142,7 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: { key={oi} aria-label={isOther ? `${OTHER_LABEL} — ${OTHER_DESCRIPTION}` : undefined} disabled={responding} - onClick={() => handleSelect(q.question, opt.label, q.multiSelect)} + onClick={() => handleSelect(keyFor(q.question, qi), opt.label, q.multiSelect)} // P-18: one row shape for both states — a visible dim border // when unselected, the accent border when selected — so rows // don't jump between "flat" and "outlined" as the user picks. @@ -1205,16 +1174,16 @@ function AskUserQuestionCard({ tool, requestId, onResponded, onFailed }: { inside the box, matching the card-wide shortcut — the window handler skips typing targets, so the box handles it itself. */}