Skip to content

Commit 3daf0c6

Browse files
committed
Surface plugin load warnings as plugin ! and in /plugins
Standing plugin diagnostics were easy to miss as startup notices. Keep them as a prompt attention mark shared with mcp !\, attribute warnings per plugin, and show them in the /plugins surface. Also document the current Enter steer / Alt+Enter queue / Ctrl+C stop mid-run gestures.
1 parent cce97a0 commit 3daf0c6

11 files changed

Lines changed: 302 additions & 34 deletions

docs/IMPLEMENTATION.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,8 @@ Unmatched shell auto-allows. Writes under the session state root (`~/.corbits/pr
180180

181181
`ChatInputProps` carries `isProcessing?: boolean` and `onInterrupt?: (message: string) => void`. When `isProcessing` is true:
182182

183-
- **Enter** calls `onInterrupt`. `App.handleInterrupt` calls `requestStop()` synchronously — which calls `sendAbortRef.current.abort()` — before `resolveAtMentions` yields, ensuring the abort signal reaches the in-flight HTTP request before any async work begins.
184-
- **Alt+Enter** calls `onSubmit` immediately, pushing the message onto `pendingQueueRef` for drain at the next `connector.reply`.
183+
- **Enter** soft-steers — enqueues kind `"steer"` and delivers at the next tool.boundary (does not interrupt).
184+
- **Alt+Enter** queues a follow-up (kind `"queue"`) delivered only when the run goes idle. **Ctrl+C** stops the run.
185185

186186
`src/tui/stream-event-map.ts` maps reactor events onto the bridge's inbound events, and `src/tui/turn-state.ts` tracks the turn's status. `src/tui/turns-to-blocks.ts` hydrates a resumed session's stored turns into the same content blocks.
187187

@@ -364,7 +364,7 @@ the directors guard on; the full set of reactor and stream event types is
364364
treat that as canonical rather than this section or any other doc's partial
365365
list.
366366

367-
Mid-run queue/steer/interrupt state is a pure state machine in `src/tui/session-queue.ts` (interaction contract §3): `enqueue` (kind `"queue"`) and `enqueueSteer` (kind `"steer"`) share one pending pool, drained steer-first, then queue, both FIFO within their class. The prompt hint (`src/tui/stream.ts`, `PROMPT_HINT`) reads `Enter queue · Alt+Enter steer · Ctrl+C stop`.
367+
Mid-run queue/steer/interrupt state is a pure state machine in `src/tui/session-queue.ts` (interaction contract §3): `enqueue` (kind `"queue"`) and `enqueueSteer` (kind `"steer"`) share one pending pool, drained steer-first, then queue, both FIFO within their class. Mid-run gestures: Enter soft-steers (drain at tool.boundary), Alt+Enter queues a follow-up (drain on idle), Ctrl+C stops.
368368

369369
### Lifecycle Hooks
370370

src/plugins/diagnostics.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import {
88
emitPluginWarningSummary,
99
formatPluginWarningsSummary,
1010
pluginWarningSink,
11+
pluginWarningSubjectId,
12+
warningsForPluginEntry,
1113
} from "./diagnostics.js";
1214
import { loadPluginEntry } from "./loader.js";
1315

@@ -79,6 +81,40 @@ describe("formatPluginWarningsSummary", () => {
7981
});
8082
});
8183

84+
describe("pluginWarningSubjectId / warningsForPluginEntry", () => {
85+
test("extracts agent and tool-plugin subject ids", () => {
86+
expect(
87+
pluginWarningSubjectId(
88+
'agent a: skill "style" referenced but not found in skill search path',
89+
),
90+
).toBe("a");
91+
expect(
92+
pluginWarningSubjectId('tool-plugin: failed to start "exa": boom'),
93+
).toBe("exa");
94+
expect(pluginWarningSubjectId("other problem")).toBeUndefined();
95+
});
96+
97+
test("attributes skill-miss warnings via plugin id or agent profile id", () => {
98+
const warnings = [
99+
'agent a: skill "style" referenced but not found in skill search path',
100+
'agent b: skill "philosophy" referenced but not found in skill search path',
101+
'tool-plugin: failed to start "exa": boom',
102+
];
103+
expect(
104+
warningsForPluginEntry(warnings, {
105+
id: "pack",
106+
agentProfiles: [{ id: "a" }],
107+
}),
108+
).toEqual([
109+
'agent a: skill "style" referenced but not found in skill search path',
110+
]);
111+
expect(warningsForPluginEntry(warnings, { id: "exa" })).toEqual([
112+
'tool-plugin: failed to start "exa": boom',
113+
]);
114+
expect(warningsForPluginEntry(warnings, { id: "other" })).toEqual([]);
115+
});
116+
});
117+
82118
describe("emitPluginWarningSummary", () => {
83119
test("writes one summary line via custom sink", () => {
84120
const diag = createPluginLoadDiagnostics();

src/plugins/diagnostics.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,34 @@ export function emitPluginWarningSummary(
105105
export function emitPluginWarningLog(diag: PluginLoadDiagnostics): void {
106106
emitPluginWarningSummary(diag, (line) => pluginDiagnosticsLogger.warn(line));
107107
}
108+
109+
/**
110+
* Extract a plugin or agent id a warning names, when present. Skill-miss lines
111+
* lead with `agent <id>:`; tool-plugin start failures quote the candidate id.
112+
*/
113+
export function pluginWarningSubjectId(warning: string): string | undefined {
114+
const agent = /^agent ([^:]+):/.exec(warning)?.[1];
115+
if (agent !== undefined) return agent;
116+
const tool = /tool-plugin: failed to start "([^"]+)"/.exec(warning)?.[1];
117+
if (tool !== undefined) return tool;
118+
return undefined;
119+
}
120+
121+
/**
122+
* Warnings attributable to one plugin: subject id matches the plugin id or any
123+
* of its agent profile ids.
124+
*/
125+
export function warningsForPluginEntry(
126+
warnings: readonly string[],
127+
plugin: {
128+
readonly id: string;
129+
readonly agentProfiles?: readonly { readonly id: string }[];
130+
},
131+
): string[] {
132+
const ids = new Set<string>([plugin.id]);
133+
for (const profile of plugin.agentProfiles ?? []) ids.add(profile.id);
134+
return warnings.filter((w) => {
135+
const subject = pluginWarningSubjectId(w);
136+
return subject !== undefined && ids.has(subject);
137+
});
138+
}

src/tui/command-surfaces.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,21 @@ describe("surface labels", () => {
8484
).toBe("exa — enabled")
8585
})
8686

87+
test("plugin label surfaces standing load warnings", () => {
88+
expect(
89+
pluginRowLabel({
90+
id: "agents",
91+
name: "agents",
92+
enabled: true,
93+
credentials: [],
94+
credentialValues: {},
95+
warnings: [
96+
'agent a: skill "style" referenced but not found in skill search path',
97+
],
98+
}),
99+
).toBe("agents — enabled — has warnings")
100+
})
101+
87102
})
88103

89104
/** Build a settings deps bag over a mutable snapshot, recording every write. */
@@ -396,6 +411,37 @@ function pluginActionDeps(overrides?: Partial<PluginEntry>): {
396411
}
397412

398413
describe("plugins surface admin actions", () => {
414+
test("load warnings appear as a summary row under /plugins", async () => {
415+
await withShell(async (shell) => {
416+
const warnings = [
417+
'agent a: skill "style" referenced but not found in skill search path',
418+
'agent a: skill "philosophy" referenced but not found in skill search path',
419+
]
420+
const { deps } = pluginActionDeps({
421+
id: "agents",
422+
name: "agents",
423+
kind: "agent",
424+
enabled: true,
425+
credentials: [],
426+
credentialValues: {},
427+
warnings,
428+
agentProfiles: [{ id: "a" }],
429+
})
430+
// pluginActionDeps builds PluginsSurfaceDeps without loadWarnings; splice it in.
431+
const plugins = deps.plugins!
432+
const withWarnings: CommandSurfaceDeps = {
433+
...deps,
434+
plugins: {
435+
...plugins,
436+
loadWarnings: () => warnings,
437+
},
438+
}
439+
openCommandSurface(shell, "plugins", withWarnings)
440+
expect(shell.overlayItems.some((l) => l.includes("2 skills missing"))).toBe(true)
441+
expect(shell.overlayItems.some((l) => l.includes("has warnings"))).toBe(true)
442+
})
443+
})
444+
399445
test("c opens credentials, typing a 40+ char key and s saves it in full", async () => {
400446
await withShell(async (shell) => {
401447
const { deps, calls } = pluginActionDeps()

src/tui/command-surfaces.ts

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import type { KeyEvent } from "@opentui/core"
1212

13+
import { formatPluginWarningsSummary } from "../plugins/diagnostics.js"
1314
import { maskEcho, maskSecret } from "./provider-setup.js"
1415
import { residualIdFromSelection, type ResidualCatalogEntry } from "./residuals.js"
1516
import {
@@ -54,6 +55,11 @@ export type PluginEntry = {
5455
readonly agentProfiles?: readonly { readonly id: string; readonly description?: string }[]
5556
/** Absolute path an untrusted path-origin plugin was discovered at. */
5657
readonly originPath?: string
58+
/**
59+
* Standing load warnings attributable to this plugin (skill misses named by
60+
* agent id, failed tool starts, …). Surfaced in the row hint and description.
61+
*/
62+
readonly warnings?: readonly string[]
5763
}
5864

5965
/** Result of a verify/addPath admin action, reported via `deps.notify`. */
@@ -98,6 +104,12 @@ export type PluginsSurfaceDeps = {
98104
readonly webProviders: () => readonly WebProviderChoice[]
99105
readonly currentWebProvider: () => string | undefined
100106
readonly setWebProvider: (id: string | undefined) => Promise<void> | void
107+
/**
108+
* Standing session-level load warnings (or the full set when attribution is
109+
* weak). Shown as a summary row under `/plugins`; drives `plugin !` via the
110+
* runner, not this surface.
111+
*/
112+
readonly loadWarnings?: () => readonly string[]
101113
}
102114

103115
/** Discovered lifecycle hook, live enablement, and enough to describe what it runs. */
@@ -176,6 +188,8 @@ export type CommandSurfaceKind =
176188

177189
const CLOSE_ID = "__close__"
178190
const BACK_ID = "__back__"
191+
/** Synthetic `/plugins` row for standing load warnings (not a plugin id). */
192+
const PLUGIN_LOAD_WARNINGS_ID = "__plugin_load_warnings__"
179193

180194
export function grantRowLabel(entry: GrantEntry): string {
181195
const suffix = entry.providerModel !== undefined ? ` (${entry.providerModel})` : ""
@@ -186,12 +200,18 @@ function pluginMissingCredential(entry: PluginEntry): boolean {
186200
return entry.credentials.some((f) => (entry.credentialValues[f.key] ?? "").length === 0)
187201
}
188202

203+
function pluginHasWarnings(entry: PluginEntry): boolean {
204+
return (entry.warnings?.length ?? 0) > 0
205+
}
206+
189207
export function pluginRowLabel(entry: PluginEntry): string {
190208
const state = entry.needsTrust === true ? "untrusted" : entry.enabled ? "enabled" : "disabled"
191209
const blocker =
192210
entry.needsTrust !== true && !entry.enabled && pluginMissingCredential(entry)
193211
? "needs api key"
194-
: entry.kind
212+
: pluginHasWarnings(entry)
213+
? "has warnings"
214+
: entry.kind
195215
return blocker ? `${entry.name}${state}${blocker}` : `${entry.name}${state}`
196216
}
197217

@@ -208,6 +228,11 @@ function pluginDescription(entry: PluginEntry): ItemDescription {
208228
if (!entry.enabled && pluginMissingCredential(entry)) {
209229
return { what, impact: "Needs an API key before it can be enabled — press Alt+C." }
210230
}
231+
if (pluginHasWarnings(entry) && entry.warnings !== undefined) {
232+
const summary =
233+
formatPluginWarningsSummary(entry.warnings) ?? entry.warnings.join("; ")
234+
return { what, impact: summary, tone: "consequence" }
235+
}
211236
return { what }
212237
}
213238

@@ -749,6 +774,14 @@ export function openPluginsSurface(shell: AppShell, deps: CommandSurfaceDeps): v
749774
id: e.id,
750775
label: pluginRowLabel(e),
751776
}))
777+
const loadWarnings = plugins.loadWarnings?.() ?? []
778+
const loadSummary = formatPluginWarningsSummary(loadWarnings)
779+
if (loadSummary !== undefined) {
780+
rows.unshift({
781+
id: PLUGIN_LOAD_WARNINGS_ID,
782+
label: loadSummary.replace(/^plugins:\s*/, ""),
783+
})
784+
}
752785
if (rows.length === 0) {
753786
rows.push({ id: CLOSE_ID, label: "No plugins discovered" })
754787
}
@@ -760,12 +793,19 @@ export function openPluginsSurface(shell: AppShell, deps: CommandSurfaceDeps): v
760793
frameId: "overlay-plugins",
761794
...payload(rows),
762795
describe: (id) => {
796+
if (id === PLUGIN_LOAD_WARNINGS_ID) {
797+
return {
798+
what: loadSummary ?? "Plugin load warnings.",
799+
impact: "Standing diagnostics from plugin discovery and load. Fix the named skills or plugins, then relaunch.",
800+
tone: "consequence",
801+
}
802+
}
763803
const target = byId.get(id)
764804
return target === undefined ? null : pluginDescription(target)
765805
},
766806
onAccept: (selection) => {
767807
const id = selectedId(selection, rows)
768-
if (id === undefined || id === CLOSE_ID) return
808+
if (id === undefined || id === CLOSE_ID || id === PLUGIN_LOAD_WARNINGS_ID) return
769809
const target = byId.get(id)
770810
if (target === undefined) return
771811
if (target.needsTrust === true) {
@@ -788,6 +828,7 @@ export function openPluginsSurface(shell: AppShell, deps: CommandSurfaceDeps): v
788828
// branch returns before that handler is reached (see shell.ts's
789829
// top-level onKey), so exactly one of the two can ever fire.
790830
if (key.ctrl || !(key.meta || key.option)) return false
831+
if (id === PLUGIN_LOAD_WARNINGS_ID) return false
791832
const target = byId.get(id)
792833
if (target === undefined) return false
793834
const name = typeof key.name === "string" ? key.name.toLowerCase() : ""

src/tui/landing.test.ts

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import {
1414
noticeText,
1515
paintChrome,
1616
setChromeZones,
17+
setPluginNeedsAttention,
18+
setPromptModelLabel,
1719
setPromptWorkspace,
1820
isLanding,
1921
paintLanding,
@@ -626,12 +628,10 @@ describe("landing screen", () => {
626628
}, SIZE)
627629
})
628630

629-
test("startup plugin diagnostics keep the mountain too", async () => {
630-
// CL-5718: CL-5618 routed MCP and hook notices away from the transcript
631-
// but left plugin diagnostics going through the runner's own system-row
632-
// helper, so any missing skill wiped the whole hero on load. The flush is
633-
// a named seam now precisely so no producer of a startup diagnostic gets
634-
// to decide this again.
631+
test("startup plugin diagnostics keep the mountain and ride plugin !", async () => {
632+
// Plugin load warnings no longer go through surfaceSystemNotice — they
633+
// drive the standing `plugin !` attention mark instead. The mountain must
634+
// still stay up while that mark is painted.
635635
await withTestRenderer(async (h) => {
636636
const shell = createAppShell(h.renderer, {
637637
terminal: { columns: 80, rows: 24 },
@@ -644,14 +644,18 @@ describe("landing screen", () => {
644644
const before = markRows(h)
645645
expect(before.length).toBeGreaterThan(0)
646646

647-
const summary = "plugins: 3 skills missing: brand-identity, style, philosophy"
648-
surfaceSystemNotice(shell, summary)
647+
setPromptModelLabel(shell, { profile: "xai", model: "grok" })
648+
setPluginNeedsAttention(shell, true)
649649
await settle(h)
650650

651651
expect(isLanding(shell)).toBe(true)
652652
expect(markRows(h).length).toBe(before.length)
653653
expect(streamRowCount(shell)).toBe(0)
654-
expect(noticeText(shell)).toContain("3 skills missing")
654+
expect(noticeText(shell)).toBe("")
655+
expect(shell.pluginNeedsAttention).toBe(true)
656+
const frame = h.captureCharFrame()
657+
expect(frame).toContain("plugin !")
658+
expect(frame).not.toContain("skills missing")
655659
} finally {
656660
shell.dispose()
657661
}
@@ -661,7 +665,8 @@ describe("landing screen", () => {
661665
test("a flushed startup notice never carries a plumbing gutter label", async () => {
662666
// The transcript must never label a row "command": a system row's text
663667
// already says what it is, and the meta column is the operator's, not the
664-
// wiring's.
668+
// wiring's. (MCP notices still use the notice strip; plugin skill-miss
669+
// summaries do not.)
665670
await withTestRenderer(async (h) => {
666671
const shell = createAppShell(h.renderer, {
667672
terminal: { columns: 80, rows: 24 },
@@ -670,13 +675,16 @@ describe("landing screen", () => {
670675
})
671676
try {
672677
await settle(h)
673-
surfaceSystemNotice(shell, "plugins: 1 skill missing: style")
678+
surfaceSystemNotice(
679+
shell,
680+
"mcp github did not connect (ECONNREFUSED) — its tools are unavailable; /mcp for detail",
681+
)
674682
appendStreamRow(shell, { role: "user", text: "first prompt" })
675683
await settle(h)
676684

677685
expect(isLanding(shell)).toBe(false)
678686
const frame = h.captureCharFrame()
679-
expect(frame).toContain("1 skill missing")
687+
expect(frame).toContain("mcp github did not connect")
680688
expect(frame).not.toContain("command")
681689
expect(frame).not.toContain("overlay")
682690
} finally {

src/tui/plugin-diagnostics-sink.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,22 @@ describe("interactive plugin diagnostics never hit raw stderr", () => {
248248
});
249249
});
250250

251+
describe("plugin warnings route to plugin ! / /plugins, not startup notices", () => {
252+
test("runner does not push formatPluginWarningsSummary into startupPluginNotices", async () => {
253+
// Product lock: discovery / tool-plugin / profile skill-miss summaries must
254+
// never become fire-and-forget surfaceSystemNotice chatter. They drive
255+
// standingPluginWarnings → setPluginNeedsAttention + /plugins instead.
256+
const src = await Bun.file(new URL("./runner.ts", import.meta.url)).text();
257+
expect(src).toContain("standingPluginWarnings");
258+
expect(src).toContain("setPluginNeedsAttention");
259+
expect(src).not.toMatch(
260+
/startupPluginNotices\.push\(\s*(discoveryNotice|toolPluginNotice|profileNotice)/,
261+
);
262+
// Unverified provider-key notice is still allowed on the startup path.
263+
expect(src).toMatch(/startupPluginNotices\.push\([\s\S]*couldn't confirm your/);
264+
});
265+
});
266+
251267
// The interactive paths above always hand `resolveToolPlugins` a diagnostics
252268
// collector. Headless/standalone callers (exec's tool-plugin resolution,
253269
// direct unit tests) may supply neither `diagnostics` nor `onWarning` —

0 commit comments

Comments
 (0)