diff --git a/.opencode/command/specgit-issue.md b/.opencode/command/specgit-issue.md index aff9adebd1..7ad133cde6 100644 --- a/.opencode/command/specgit-issue.md +++ b/.opencode/command/specgit-issue.md @@ -13,17 +13,18 @@ AGENTS.md SpecGit block; this command only launches it. 1. Collect the argument: `$ARGUMENTS` is either an issue title (create) or a pure number (reuse). Multiple arguments = N issues in one delivery. -2. Run from the repo root: +2. Run from the repo root — keep `$ARGUMENTS` UNQUOTED so each quoted title + arrives as its own argument: ```bash - specgit issue "$ARGUMENTS" --json + specgit issue $ARGUMENTS --json ``` 3. On success report the brief: issue URL(s), PR URL (draft), branch name — then fill each issue body it created (Why / Scope / Approach / Acceptance) from the discussion with `gh issue edit `, then implement. Fill in the draft PR's scaffold (Why / What changed / - Evidence) as you deliver; its placeholders are advisory, never gates, - and the closing references stay intact. + Evidence / Checklist) as you deliver; its placeholders are advisory, + never gates, and the closing references stay intact. 4. Switch to the delivery branch and begin the TDD loop. 5. On error, read `errors[].fix` and follow it — never bypass the record. diff --git a/.opencode/hooks/specgit-merge-guard.sh b/.opencode/hooks/specgit-merge-guard.sh index f261123f35..01981b8098 100755 --- a/.opencode/hooks/specgit-merge-guard.sh +++ b/.opencode/hooks/specgit-merge-guard.sh @@ -26,7 +26,7 @@ esac command=$(printf '%s' "$payload" | node -e "let s='';process.stdin.on('data',d=>s+=d).on('end',()=>{try{const j=JSON.parse(s);process.stdout.write((j.tool_input&&j.tool_input.command)||'')}catch{process.stdout.write('')}})") case "$command" in - gh\ pr\ merge*) + gh\ pr\ merge*|glab\ mr\ merge*) exec node -e ' const { spawn } = require("child_process"); const fs = require("fs"); diff --git a/.specgit.yaml b/.specgit.yaml index 7930002482..abd02f7f60 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,9 +1,8 @@ version: 1 -delivery: 477-remove-legacy-artifacts +delivery: shell-silence-guard context: - kind: worktree - label: docs-477 - branch: docs/477-remove-legacy-artifacts + kind: branch + branch: feat/433-shell-silence-guard issues: - - 477 -pr: 480 + - 433 +pr: 491 diff --git a/packages/opencode/src/effect/runtime-flags.ts b/packages/opencode/src/effect/runtime-flags.ts index 58dc50d027..303e7da706 100644 --- a/packages/opencode/src/effect/runtime-flags.ts +++ b/packages/opencode/src/effect/runtime-flags.ts @@ -50,6 +50,7 @@ export class Service extends ConfigService.Service()("@opencode/Runtime experimentalIconDiscovery: enabledByExperimental("OPENCODE_EXPERIMENTAL_ICON_DISCOVERY"), outputTokenMax: positiveInteger("OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX"), bashDefaultTimeoutMs: positiveInteger("OPENCODE_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS"), + bashSilenceWarnMs: positiveInteger("OPENCODE_EXPERIMENTAL_BASH_SILENCE_WARN_MS"), experimentalNativeLlm: bool("OPENCODE_EXPERIMENTAL_NATIVE_LLM"), experimentalWebSockets: bool("OPENCODE_EXPERIMENTAL_WEBSOCKETS"), client: Config.string("OPENCODE_CLIENT").pipe(Config.withDefault("cli")), diff --git a/packages/opencode/src/tool/shell.ts b/packages/opencode/src/tool/shell.ts index 96957beed1..5f3be73c7e 100644 --- a/packages/opencode/src/tool/shell.ts +++ b/packages/opencode/src/tool/shell.ts @@ -27,6 +27,11 @@ export { Parameters } from "./shell/prompt" export const SHELL_ABORT_NOTE = "The command was aborted before completion (client interrupt or session cancel). For long-running work, bound it with `timeout ` and stream progress instead of piping into a silent buffer." +const DEFAULT_SILENCE_WARN_MS = 5 * 60 * 1000 + +const shellSilenceNote = (ms: number) => + `shell tool emitted an inactivity warning after ${ms} ms without output; the command was left running. If this command is expected to stay silent, pass expectedSilent: true to opt out.` + const MAX_METADATA_LENGTH = 30_000 const CWD = new Set(["cd", "chdir", "popd", "pushd", "push-location", "set-location"]) const FILES = new Set([ @@ -352,6 +357,7 @@ export const ShellTool = Tool.define( const plugin = yield* Plugin.Service const flags = yield* RuntimeFlags.Service const defaultTimeoutMs = flags.bashDefaultTimeoutMs ?? 2 * 60 * 1000 + const silenceWarnMs = flags.bashSilenceWarnMs ?? DEFAULT_SILENCE_WARN_MS const cygpath = Effect.fn("ShellTool.cygpath")(function* (shell: string, text: string) { const lines = yield* spawner @@ -439,6 +445,7 @@ export const ShellTool = Tool.define( cwd: string env: NodeJS.ProcessEnv timeout: number + expectedSilent: boolean }, ctx: Tool.Context, ) { @@ -453,6 +460,9 @@ export const ShellTool = Tool.define( let cut = false let expired = false let aborted = false + let lastActivity = Date.now() + let silenceWarned = false + let silenceWarnings = 0 const closeSink = Effect.fnUntraced(function* () { const stream = sink @@ -492,6 +502,8 @@ export const ShellTool = Tool.define( const readerFiber = yield* Effect.forkScoped( Stream.runForEach(Stream.decodeText(handle.all), (chunk) => { + lastActivity = Date.now() + silenceWarned = false const size = Buffer.byteLength(chunk, "utf-8") list.push({ text: chunk, size }) used += size @@ -537,6 +549,27 @@ export const ShellTool = Tool.define( }), ) + // Warn-only silence guard: lives on its own fiber and must never + // join the race below — a silence warning may not change exit.kind. + if (!input.expectedSilent) { + yield* Effect.forkScoped( + Effect.forever( + Effect.gen(function* () { + const idle = Date.now() - lastActivity + yield* Effect.sleep(`${idle < silenceWarnMs ? silenceWarnMs - idle : silenceWarnMs} millis`) + if (silenceWarned || Date.now() - lastActivity < silenceWarnMs) return + silenceWarned = true + silenceWarnings++ + yield* ctx.metadata({ + metadata: { + output: last + `\n\n${shellSilenceNote(silenceWarnMs)}`, + }, + }) + }), + ), + ) + } + const abort = Effect.callback((resume) => { if (ctx.abort.aborted) return resume(Effect.void) const handler = () => resume(Effect.void) @@ -579,6 +612,7 @@ export const ShellTool = Tool.define( ) } if (aborted) meta.push(SHELL_ABORT_NOTE) + for (let i = 0; i < silenceWarnings; i++) meta.push(shellSilenceNote(silenceWarnMs)) const raw = list.map((item) => item.text).join("") const end = tail(raw, limits.maxLines, limits.maxBytes) if (end.cut) cut = true @@ -614,7 +648,7 @@ export const ShellTool = Tool.define( const shell = Shell.acceptable(cfg.shell) const name = Shell.name(shell) const limits = yield* trunc.limits() - const prompt = ShellPrompt.render(name, process.platform, limits, defaultTimeoutMs) + const prompt = ShellPrompt.render(name, process.platform, limits, defaultTimeoutMs, silenceWarnMs) yield* Effect.logInfo("shell tool using shell", { shell }) return { @@ -649,6 +683,7 @@ export const ShellTool = Tool.define( cwd, env: yield* shellEnv(ctx, cwd), timeout, + expectedSilent: params.expectedSilent === true, }, ctx, ) diff --git a/packages/opencode/src/tool/shell/prompt.ts b/packages/opencode/src/tool/shell/prompt.ts index b576b77297..8efe0f5479 100644 --- a/packages/opencode/src/tool/shell/prompt.ts +++ b/packages/opencode/src/tool/shell/prompt.ts @@ -19,6 +19,10 @@ export function parameterSchema() { workdir: Schema.optional(Schema.String).annotate({ description: `The working directory to run the command in. Defaults to the current directory. Use this instead of 'cd' commands.`, }), + expectedSilent: Schema.optional(Schema.Boolean).annotate({ + description: + "Set true when the command is expected to produce no output for long stretches (watchers, listeners, waits). Suppresses the warn-only shell inactivity warning.", + }), }) } @@ -75,7 +79,7 @@ function chainGuidance(name: string) { return "If the commands depend on each other and must run sequentially, use a single Bash call with '&&' to chain them together (e.g., `git add . && git commit -m \"message\" && git push`). For instance, if one operation must complete before another starts (like mkdir before cp, Write before Bash for git operations, or git add before git commit), run these operations sequentially instead." } -function bashCommandSection(chain: string, limits: Limits, defaultTimeoutMs: number) { +function bashCommandSection(chain: string, limits: Limits, defaultTimeoutMs: number, silenceWarnMs: number) { return `Before executing the command, please follow these steps: 1. Directory Verification: @@ -95,6 +99,7 @@ function bashCommandSection(chain: string, limits: Limits, defaultTimeoutMs: num Usage notes: - The command argument is required. - You can specify an optional timeout in milliseconds. If not specified, commands will time out after ${defaultTimeoutMs}ms. + - If a command produces no output for ${silenceWarnMs}ms, a warn-only inactivity warning is emitted. For commands that legitimately stay silent (watchers, listeners, waits), pass \`expectedSilent: true\` to opt out. - If the output exceeds ${limits.maxLines} lines or ${limits.maxBytes} bytes, it will be truncated and the full output will be written to a file. You can use Read with offset/limit to read specific sections or Grep to search the full content. Do NOT use \`head\`, \`tail\`, or other truncation commands to limit output; the full output will already be captured to a file for more precise searching. - Avoid using Bash with the \`find\`, \`grep\`, \`cat\`, \`head\`, \`tail\`, \`sed\`, \`awk\`, or \`echo\` commands, unless explicitly instructed or when these commands are truly necessary for the task. Instead, always prefer using the dedicated tools for these commands: @@ -124,6 +129,7 @@ function powershellCommandSection( pathSep: string, limits: Limits, defaultTimeoutMs: number, + silenceWarnMs: number, ) { return `${powershellNotes(name)} @@ -146,6 +152,7 @@ Before executing the command, please follow these steps: Usage notes: - The command argument is required. - You can specify an optional timeout in milliseconds. If not specified, commands will time out after ${defaultTimeoutMs}ms. + - If a command produces no output for ${silenceWarnMs}ms, a warn-only inactivity warning is emitted. For commands that legitimately stay silent (watchers, listeners, waits), pass \`expectedSilent: true\` to opt out. - If the output exceeds ${limits.maxLines} lines or ${limits.maxBytes} bytes, it will be truncated and the full output will be written to a file. You can use Read with offset/limit to read specific sections or Grep to search the full content. Do NOT use \`Select-Object -First\`, \`Select-Object -Last\`, or other truncation commands to limit output; the full output will already be captured to a file for more precise searching. - Avoid using Shell with PowerShell file/content cmdlets unless explicitly instructed or when these cmdlets are truly necessary for the task. Instead, always prefer using the dedicated tools for these commands: @@ -169,7 +176,7 @@ Usage notes: ` } -function cmdCommandSection(chain: string, limits: Limits, defaultTimeoutMs: number) { +function cmdCommandSection(chain: string, limits: Limits, defaultTimeoutMs: number, silenceWarnMs: number) { return `# cmd.exe shell notes - Use double quotes for paths with spaces. - Use %VAR% for environment variables. @@ -195,6 +202,7 @@ Before executing the command, please follow these steps: Usage notes: - The command argument is required. - You can specify an optional timeout in milliseconds. If not specified, commands will time out after ${defaultTimeoutMs}ms. + - If a command produces no output for ${silenceWarnMs}ms, a warn-only inactivity warning is emitted. For commands that legitimately stay silent (watchers, listeners, waits), pass \`expectedSilent: true\` to opt out. - If the output exceeds ${limits.maxLines} lines or ${limits.maxBytes} bytes, it will be truncated and the full output will be written to a file. You can use Read with offset/limit to read specific sections or Grep to search the full content. Do NOT use \`more\` or other pagination commands to limit output; the full output will already be captured to a file for more precise searching. - Avoid using Shell with cmd.exe file/content commands unless explicitly instructed or when these commands are truly necessary for the task. Instead, always prefer using the dedicated tools for these commands: @@ -218,7 +226,7 @@ Usage notes: ` } -function profile(name: string, platform: NodeJS.Platform, limits: Limits, defaultTimeoutMs: number) { +function profile(name: string, platform: NodeJS.Platform, limits: Limits, defaultTimeoutMs: number, silenceWarnMs: number) { const isPowerShell = PS.has(name) const chain = chainGuidance(name) if (CMD.has(name)) { @@ -226,7 +234,7 @@ function profile(name: string, platform: NodeJS.Platform, limits: Limits, defaul intro: `Executes a given ${shellDisplayName(name)} command with optional timeout, ensuring proper handling and security measures.`, workdirSection: "All commands run in the current working directory by default. Use the `workdir` parameter if you need to run a command in a different directory. AVOID changing directories inside the command - use `workdir` instead.", - commandSection: cmdCommandSection(chain, limits, defaultTimeoutMs), + commandSection: cmdCommandSection(chain, limits, defaultTimeoutMs, silenceWarnMs), gitCommands: "git commands", gitCommandRestriction: "git commands", createPrInstruction: "Create PR using a temporary body file so cmd.exe quoting stays simple.", @@ -244,6 +252,7 @@ function profile(name: string, platform: NodeJS.Platform, limits: Limits, defaul platform === "win32" ? "\\" : "/", limits, defaultTimeoutMs, + silenceWarnMs, ), gitCommands: "git commands", gitCommandRestriction: "git commands", @@ -259,7 +268,7 @@ function profile(name: string, platform: NodeJS.Platform, limits: Limits, defaul "Executes a given bash command in a persistent shell session with optional timeout, ensuring proper handling and security measures.", workdirSection: "All commands run in the current working directory by default. Use the `workdir` parameter if you need to run a command in a different directory. AVOID using `cd && ` patterns - use `workdir` instead.", - commandSection: bashCommandSection(chain, limits, defaultTimeoutMs), + commandSection: bashCommandSection(chain, limits, defaultTimeoutMs, silenceWarnMs), gitCommands: "bash commands", gitCommandRestriction: "git bash commands", createPrInstruction: @@ -270,8 +279,8 @@ function profile(name: string, platform: NodeJS.Platform, limits: Limits, defaul } } -export function render(name: string, platform: NodeJS.Platform, limits: Limits, defaultTimeoutMs: number) { - const selected = profile(name, platform, limits, defaultTimeoutMs) +export function render(name: string, platform: NodeJS.Platform, limits: Limits, defaultTimeoutMs: number, silenceWarnMs: number) { + const selected = profile(name, platform, limits, defaultTimeoutMs, silenceWarnMs) return { description: renderPrompt(DESCRIPTION, { intro: selected.intro, diff --git a/packages/opencode/test/effect/runtime-flags.test.ts b/packages/opencode/test/effect/runtime-flags.test.ts index 2e1226b38b..6b024ff05d 100644 --- a/packages/opencode/test/effect/runtime-flags.test.ts +++ b/packages/opencode/test/effect/runtime-flags.test.ts @@ -282,6 +282,35 @@ describe("RuntimeFlags", () => { ) } + for (const input of [ + { name: "absent", config: {}, expected: undefined }, + { + name: "valid positive integer", + config: { OPENCODE_EXPERIMENTAL_BASH_SILENCE_WARN_MS: "1234" }, + expected: 1234, + }, + { + name: "invalid string", + config: { OPENCODE_EXPERIMENTAL_BASH_SILENCE_WARN_MS: "nope" }, + expected: undefined, + }, + { name: "zero", config: { OPENCODE_EXPERIMENTAL_BASH_SILENCE_WARN_MS: "0" }, expected: undefined }, + { name: "negative", config: { OPENCODE_EXPERIMENTAL_BASH_SILENCE_WARN_MS: "-1" }, expected: undefined }, + { + name: "non-integer", + config: { OPENCODE_EXPERIMENTAL_BASH_SILENCE_WARN_MS: "1.5" }, + expected: undefined, + }, + ]) { + it.effect(`parses bashSilenceWarnMs from config: ${input.name}`, () => + Effect.gen(function* () { + const flags = yield* readFlags.pipe(Effect.provide(fromConfig(input.config))) + + expect(flags.bashSilenceWarnMs).toBe(input.expected) + }), + ) + } + for (const input of [ { name: "absent", config: {}, expected: undefined }, { diff --git a/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap b/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap index c7ddfbc5fa..a01a903e39 100644 --- a/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap +++ b/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap @@ -24,6 +24,10 @@ exports[`tool parameters JSON Schema (wire shape) bash 1`] = ` "description": "The command to execute", "type": "string", }, + "expectedSilent": { + "description": "Set true when the command is expected to produce no output for long stretches (watchers, listeners, waits). Suppresses the warn-only shell inactivity warning.", + "type": "boolean", + }, "timeout": { "description": "Optional timeout in milliseconds", "exclusiveMinimum": 0, diff --git a/packages/opencode/test/tool/shell.test.ts b/packages/opencode/test/tool/shell.test.ts index f93a896f85..fbc0e531c8 100644 --- a/packages/opencode/test/tool/shell.test.ts +++ b/packages/opencode/test/tool/shell.test.ts @@ -1126,6 +1126,113 @@ describe("tool.shell abort", () => { ) }) +describe("tool.shell silence guard", () => { + const collector = (warned: string[]) => ({ + ...ctx, + metadata: (input: { title?: string; metadata?: { output?: string } }) => + Effect.sync(() => { + const output = input.metadata?.output + if (output?.includes("inactivity warning after")) warned.push(output) + }), + }) + + it.live( + "warns once after bashSilenceWarnMs without output and leaves the command running", + () => + runIn( + projectRoot, + Effect.gen(function* () { + const warned: string[] = [] + const result = yield* run({ command: `sleep 1` }, collector(warned)) + expect(result.metadata.exit).toBe(0) + expect(warned.length).toBe(1) + expect(result.output.match(/inactivity warning after/g)?.length).toBe(1) + expect(result.output).toContain("expectedSilent: true") + }), + ).pipe(Effect.provide(RuntimeFlags.layer({ bashSilenceWarnMs: 300 }))), + 15_000, + ) + + it.live( + "expectedSilent suppresses the inactivity warning", + () => + runIn( + projectRoot, + Effect.gen(function* () { + const warned: string[] = [] + const result = yield* run({ command: `sleep 1`, expectedSilent: true }, collector(warned)) + expect(result.metadata.exit).toBe(0) + expect(warned.length).toBe(0) + expect(result.output).not.toContain("inactivity warning after") + }), + ).pipe(Effect.provide(RuntimeFlags.layer({ bashSilenceWarnMs: 300 }))), + 15_000, + ) + + it.live( + "resets the silence window when output resumes", + () => + runIn( + projectRoot, + Effect.gen(function* () { + const warned: string[] = [] + const result = yield* run( + { command: `sleep 1 && echo tick && sleep 1 && echo done` }, + collector(warned), + ) + expect(result.metadata.exit).toBe(0) + expect(result.output).toContain("tick") + expect(result.output).toContain("done") + expect(result.output.match(/inactivity warning after/g)?.length).toBe(2) + }), + ).pipe(Effect.provide(RuntimeFlags.layer({ bashSilenceWarnMs: 300 }))), + 15_000, + ) + + it.live( + "keeps abort behavior when the silence guard is active", + () => + runIn( + projectRoot, + Effect.gen(function* () { + const controller = new AbortController() + const res = yield* run( + { command: `echo before && sleep 30` }, + { + ...ctx, + abort: controller.signal, + metadata: (input) => + Effect.sync(() => { + const output = input.metadata?.output + if (output && output.includes("before") && !controller.signal.aborted) { + controller.abort() + } + }), + }, + ) + expect(res.output).toContain("before") + expect(res.output).toContain("aborted before completion") + }), + ).pipe(Effect.provide(RuntimeFlags.layer({ bashSilenceWarnMs: 300 }))), + 15_000, + ) + + it.live( + "keeps timeout behavior when a silence warning was emitted", + () => + runIn( + projectRoot, + Effect.gen(function* () { + const result = yield* run({ command: `sleep 60`, timeout: 2000 }) + expect(result.output).toContain("shell tool terminated command after exceeding timeout") + expect(result.output).toContain("retry with a larger timeout value in milliseconds") + expect(result.output.match(/inactivity warning after/g)?.length).toBe(1) + }), + ).pipe(Effect.provide(RuntimeFlags.layer({ bashSilenceWarnMs: 100 }))), + 15_000, + ) +}) + describe("tool.shell truncation", () => { it.live("truncates output exceeding line limit", () => runIn( diff --git a/packages/tui/src/feature-plugins/system/dag-inspector-utils.ts b/packages/tui/src/feature-plugins/system/dag-inspector-utils.ts index 0fc612f54d..dedc81341a 100644 --- a/packages/tui/src/feature-plugins/system/dag-inspector-utils.ts +++ b/packages/tui/src/feature-plugins/system/dag-inspector-utils.ts @@ -4,7 +4,7 @@ /** Pure topology helpers for the DAG inspector. Extracted for unit testing, * mirroring the diff-viewer-file-tree-utils pattern in this directory. */ -import type { DagNode } from "@opencode-ai/sdk/v2" +import type { DagNode, DagWorkflowSummary } from "@opencode-ai/sdk/v2" export type { DagNode } @@ -200,3 +200,41 @@ export function dagControlProgressMessage(operation: DagControlOperation) { if (operation === "step") return "Stepping workflow..." return "Cancelling workflow..." } + +/** + * Unique owning sessions of a project-level workflow list, preserving the + * list's order. The summary endpoint is session-scoped, so discovery groups + * the flat `GET /dag` rows by `session_id` before fetching summaries. + */ +export function dagWorkflowSessions(workflows: ReadonlyArray<{ session_id: string }>): string[] { + const seen = new Set() + const sessions: string[] = [] + for (const workflow of workflows) { + if (seen.has(workflow.session_id)) continue + seen.add(workflow.session_id) + sessions.push(workflow.session_id) + } + return sessions +} + +/** + * Merge per-session summary lists into one row set ordered by the project + * list, dropping summaries for workflows the list no longer reports. Rows + * keep the list's identity as the source of truth — a session-scoped summary + * can legitimately lag a concurrent cancel. + */ +export function mergeDagWorkflowSummaries( + list: ReadonlyArray<{ id: string }>, + summaries: ReadonlyArray>, +): DagWorkflowSummary[] { + const byID = new Map() + for (const rows of summaries) { + for (const row of rows) byID.set(row.id, row) + } + const merged: DagWorkflowSummary[] = [] + for (const workflow of list) { + const row = byID.get(workflow.id) + if (row) merged.push(row) + } + return merged +} diff --git a/packages/tui/src/feature-plugins/system/dag-inspector.tsx b/packages/tui/src/feature-plugins/system/dag-inspector.tsx index d67bb39d68..bbf3eae290 100644 --- a/packages/tui/src/feature-plugins/system/dag-inspector.tsx +++ b/packages/tui/src/feature-plugins/system/dag-inspector.tsx @@ -18,11 +18,13 @@ import { dagNodeGlyph, dagNodeHistoryLabel, dagStatusColor, + dagWorkflowSessions, formatDagDeadline, formatDagDuration, formatDagError, formatDagOutputPreview, formatDagProgress, + mergeDagWorkflowSummaries, dagEscalationLabel, type DagControlOperation, type DagNode, @@ -111,6 +113,33 @@ function cancelActiveWorkflow(api: TuiPluginApi) { }) } +// Project-level discovery: the inspector's workflow list must not depend on +// the route's sessionID chain, so a missing or stale link can never produce +// a zero-request empty state. The flat project list groups by owning session +// for the session-scoped summary endpoint; one dead session must not kill +// the whole discovery. +async function discoverProjectWorkflows( + api: TuiPluginApi, + timeoutMs: number, +): Promise<{ summaries: DagWorkflowSummary[]; sessionByWorkflow: Map }> { + const response = await withTimeout(api.client.dag.list(), timeoutMs) + const workflows = response.data ?? [] + const grouped = await Promise.all( + dagWorkflowSessions(workflows).map(async (sessionID) => { + try { + const summaries = await withTimeout(api.client.dag.summary({ sessionID }), timeoutMs) + return summaries.data ?? [] + } catch { + return [] + } + }), + ) + return { + summaries: mergeDagWorkflowSummaries(workflows, grouped), + sessionByWorkflow: new Map(workflows.map((workflow) => [workflow.id, workflow.session_id])), + } +} + function DagInspector(props: { api: TuiPluginApi }) { const theme = () => props.api.theme.current // The plugin-facing theme omits the resolver flags selectedForeground needs, @@ -131,56 +160,94 @@ function DagInspector(props: { api: TuiPluginApi }) { const [selectedNode, setSelectedNode] = createSignal(undefined) const [nodes, setNodes] = createSignal([]) const [fetchedWorkflows, setFetchedWorkflows] = createSignal | undefined>() + const [projectWorkflows, setProjectWorkflows] = createSignal>([]) const [workflowLoad, setWorkflowLoad] = createSignal<"loading" | "loaded" | "error">("loading") const [actionMessage, setActionMessage] = createSignal() + // Owning session per discovered workflow — plain lookup for the summary + // event gate, which is scoped to the selected workflow's session, not the + // route's. Rebuilt on every discovery run. + const sessionByWorkflow = new Map() let workflowScroll: ScrollBoxRenderable | undefined let nodeScroll: ScrollBoxRenderable | undefined const workflows = createMemo(() => { + // Session-scoped live data wins when the route names a session; the + // project-level discovery is the floor that keeps the list populated + // when that chain is broken, stale, or absent. const sid = params()?.sessionID - if (!sid) return [] - const synced = props.api.state.session.dag(sid) - return synced.length > 0 ? synced : (fetchedWorkflows() ?? []) + if (sid) { + const synced = props.api.state.session.dag(sid) + if (synced.length > 0) return synced + const fetched = fetchedWorkflows() + if (fetched && fetched.length > 0) return fetched + } + return projectWorkflows() }) // Refresh authoritative state when the inspector opens. Summary events are // ephemeral, so the shared sync slice can legitimately be empty after a - // missed event even though the workflow exists on the server. + // missed event even though the workflow exists on the server. Project + // discovery runs on every mount — it is the one source that does not + // depend on the route's sessionID. createEffect(() => { const sessionID = params()?.sessionID setFetchedWorkflows([]) + setProjectWorkflows([]) + sessionByWorkflow.clear() setSelectedWorkflow(undefined) setSelectedNode(undefined) setNodes([]) setActionMessage(undefined) - if (!sessionID) { - setWorkflowLoad("loaded") - return - } let disposed = false - let attemptsLeft = 1 + let pendingSources = sessionID ? 2 : 1 + let anySourceLoaded = false onCleanup(() => { disposed = true }) - const attempt = () => { - void withTimeout(props.api.client.dag.summary({ sessionID }), fetchTimeoutMs(props.api)) + const settle = (loaded: boolean) => { + if (disposed) return + pendingSources -= 1 + anySourceLoaded = anySourceLoaded || loaded + if (pendingSources === 0) setWorkflowLoad(anySourceLoaded ? "loaded" : "error") + } + const attemptDiscovery = (attemptsLeft: number) => { + void withTimeout(discoverProjectWorkflows(props.api, fetchTimeoutMs(props.api)), fetchTimeoutMs(props.api)) + .then((discovered) => { + if (disposed) return + setProjectWorkflows(discovered.summaries) + for (const [workflowID, owner] of discovered.sessionByWorkflow) { + sessionByWorkflow.set(workflowID, owner) + } + settle(true) + }) + .catch(() => { + if (disposed) return + if (attemptsLeft > 0) { + setTimeout(() => attemptDiscovery(attemptsLeft - 1), RETRY_DELAY_MS) + return + } + settle(false) + }) + } + const attemptSession = (session: string, attemptsLeft: number) => { + void withTimeout(props.api.client.dag.summary({ sessionID: session }), fetchTimeoutMs(props.api)) .then((response) => { - if (disposed || params()?.sessionID !== sessionID) return + if (disposed || params()?.sessionID !== session) return setFetchedWorkflows(response.data ?? []) - setWorkflowLoad("loaded") + settle(true) }) .catch(() => { - if (disposed || params()?.sessionID !== sessionID) return + if (disposed || params()?.sessionID !== session) return if (attemptsLeft > 0) { - attemptsLeft -= 1 - setTimeout(attempt, RETRY_DELAY_MS) + setTimeout(() => attemptSession(session, attemptsLeft - 1), RETRY_DELAY_MS) return } - setWorkflowLoad("error") + settle(false) }) } setWorkflowLoad("loading") - attempt() + attemptDiscovery(1) + if (sessionID) attemptSession(sessionID, 1) }) // Keep a valid workflow selected: adopt the first workflow when nothing is @@ -213,10 +280,11 @@ function DagInspector(props: { api: TuiPluginApi }) { let lastSignature = "" const signatureFor = (wfId: string): string => { + // The sync slice is read directly — event handlers fire outside any + // reactive scope, so a memoized read could serve a stale signature. const sid = params()?.sessionID - if (!sid) return "" - const wfs = props.api.state.session.dag(sid) - const wf = wfs.find((w) => w.id === wfId) + const wf = (sid ? props.api.state.session.dag(sid) : []).find((w) => w.id === wfId) ?? + workflows().find((w) => w.id === wfId) if (!wf) return "" return `${wf.nodeCount}:${wf.completedNodes}:${wf.runningNodes}:${wf.failedNodes}:${wf.graphRev}` } @@ -232,12 +300,13 @@ function DagInspector(props: { api: TuiPluginApi }) { // open has something to compare against. lastSignature = signatureFor(wf) void fetchNodes(wf) - // Re-fetch nodes only when a summary event for THIS session indicates the - // selected workflow's node-level state changed. Summary events for other - // sessions and unchanged summaries do not trigger a fetch. - const sid = params()?.sessionID + // Re-fetch nodes only when a summary event for the selected workflow's + // owning session indicates the workflow's node-level state changed. + // Project discovery supplies the owning session when the route carries + // no sessionID; events for other sessions never trigger a fetch. + const owner = sessionByWorkflow.get(wf) ?? params()?.sessionID const off = props.api.event.on("dag.workflow.summary.updated", (event) => { - if (!sid || event.properties.sessionID !== sid) return + if (!owner || event.properties.sessionID !== owner) return const sig = signatureFor(wf) if (sig === lastSignature) return lastSignature = sig @@ -549,7 +618,7 @@ function DagInspector(props: { api: TuiPluginApi }) { - {"No session context — reopen /dag from within a conversation."} + {"No DAG workflows in this project — run /dag-auto to start an orchestration."} diff --git a/packages/tui/test/feature-plugins/dag-inspector-utils.test.ts b/packages/tui/test/feature-plugins/dag-inspector-utils.test.ts index 782befae9e..dcdefacc1d 100644 --- a/packages/tui/test/feature-plugins/dag-inspector-utils.test.ts +++ b/packages/tui/test/feature-plugins/dag-inspector-utils.test.ts @@ -8,11 +8,13 @@ import { dagNodeGlyph, dagNodeHistoryLabel, dagStatusColor, + dagWorkflowSessions, formatDagDeadline, formatDagDuration, formatDagError, formatDagOutputPreview, formatDagProgress, + mergeDagWorkflowSummaries, type DagNode, } from "../../src/feature-plugins/system/dag-inspector-utils" @@ -177,3 +179,45 @@ describe("node detail formatting", () => { expect(formatDagProgress({ nodeCount: 2, completedNodes: 0, skippedNodes: 0 })).toBe("0/2") }) }) + +describe("dagWorkflowSessions", () => { + test("keeps first-seen order and drops duplicate owners", () => { + expect( + dagWorkflowSessions([{ session_id: "b" }, { session_id: "a" }, { session_id: "b" }]), + ).toEqual(["b", "a"]) + }) + + test("empty list yields no sessions", () => { + expect(dagWorkflowSessions([])).toEqual([]) + }) +}) + +describe("mergeDagWorkflowSummaries", () => { + const summary = (id: string) => ({ + id, + title: id, + status: "running", + nodeCount: 1, + completedNodes: 0, + runningNodes: 1, + failedNodes: 0, + skippedNodes: 0, + queuedNodes: 0, + escalatedNodes: 0, + graphRev: 1, + }) + + test("orders merged rows by the project list and keeps the list as source of truth", () => { + const merged = mergeDagWorkflowSummaries([{ id: "b" }, { id: "a" }], [[summary("b")], [summary("a")]]) + expect(merged.map((row) => row.id)).toEqual(["b", "a"]) + }) + + test("drops summaries for workflows the list no longer reports", () => { + const merged = mergeDagWorkflowSummaries([{ id: "a" }], [[summary("a"), summary("ghost")]]) + expect(merged.map((row) => row.id)).toEqual(["a"]) + }) + + test("empty discovery inputs merge to nothing", () => { + expect(mergeDagWorkflowSummaries([], [])).toEqual([]) + }) +}) diff --git a/packages/tui/test/feature-plugins/dag-inspector.test.tsx b/packages/tui/test/feature-plugins/dag-inspector.test.tsx index abc8537c31..149470bf2e 100644 --- a/packages/tui/test/feature-plugins/dag-inspector.test.tsx +++ b/packages/tui/test/feature-plugins/dag-inspector.test.tsx @@ -3,7 +3,7 @@ import { describe, expect, test } from "bun:test" import { createDefaultOpenTuiKeymap } from "@opentui/keymap/opentui" import { testRender, useRenderer } from "@opentui/solid" import type { TuiPluginApi, TuiRouteCurrent, TuiRouteDefinition } from "@opencode-ai/plugin/tui" -import type { DagNode, DagWorkflowSummary } from "@opencode-ai/sdk/v2" +import type { DagNode, DagWorkflow, DagWorkflowSummary } from "@opencode-ai/sdk/v2" import { KVProvider } from "../../src/context/kv" import { ThemeProvider } from "../../src/context/theme" import { TuiConfigProvider } from "../../src/config" @@ -34,6 +34,7 @@ const wfSummary = (overrides: Partial = {}): DagWorkflowSumm type RenderOpts = { workflows?: DagWorkflowSummary[] serverWorkflows?: DagWorkflowSummary[] + projectWorkflows?: DagWorkflow[] nodes?: DagNode[] initialRoute?: TuiRouteCurrent summary?: (sessionID: string) => Promise<{ data: DagWorkflowSummary[] }> @@ -41,6 +42,17 @@ type RenderOpts = { width?: number } +const projectWorkflow = (overrides: Partial & { id: string; session_id: string }): DagWorkflow => ({ + project_id: "proj_1", + title: `Workflow ${overrides.id}`, + status: "running", + config: "{}", + seq: 1, + time_created: 0, + time_updated: 0, + ...overrides, +}) + function dagNode(overrides: Partial & { id: string }): DagNode { return { workflow_id: "wf-1", @@ -72,6 +84,7 @@ async function renderDagInspector(opts: RenderOpts = {}) { // Trackable spies const nodesCalls: string[] = [] const summaryCalls: string[] = [] + let listCalls = 0 const controlCalls: { dagID: string; operation: string }[] = [] const commandCalls: unknown[] = [] const navigations: { name: string; params?: Record }[] = [] @@ -92,6 +105,10 @@ async function renderDagInspector(opts: RenderOpts = {}) { keymap, client: { dag: { + list: async () => { + listCalls += 1 + return { data: opts.projectWorkflows ?? [] } + }, summary: async (input: { sessionID: string }) => { summaryCalls.push(input.sessionID) return opts.summary?.(input.sessionID) ?? { data: opts.serverWorkflows ?? workflowsState } @@ -184,6 +201,7 @@ async function renderDagInspector(opts: RenderOpts = {}) { toasts: () => toasts, nodesCalls: () => nodesCalls, summaryCalls: () => summaryCalls, + listCalls: () => listCalls, controlCalls: () => controlCalls, setWorkflows: (wfs: DagWorkflowSummary[]) => { workflowsState = wfs @@ -727,11 +745,59 @@ describe("DagInspector", () => { } }) - test("explains missing session context instead of pretending there are no workflows", async () => { + test("lists project workflows when the route has no session context", async () => { + const viewer = await renderDagInspector({ + initialRoute: { name: "dag" }, + projectWorkflows: [ + projectWorkflow({ id: "wf-p1", session_id: "ses_a", title: "Orphan discovery", status: "running" }), + projectWorkflow({ id: "wf-p2", session_id: "ses_b", title: "Other session", status: "completed" }), + ], + summary: (sessionID) => + Promise.resolve({ + data: + sessionID === "ses_a" + ? [wfSummary({ id: "wf-p1", title: "Orphan discovery", status: "running" })] + : [], + }), + }) + try { + // Discovery renders workflows from any session when the route carries + // no sessionID — the zero-request empty state is gone. + await viewer.app.waitForFrame((frame) => frame.includes("Orphan discovery")) + // Workflows group by owning session for the session-scoped summary; a + // dead session resolves empty without failing the whole discovery. + expect(viewer.summaryCalls()).toEqual(["ses_a", "ses_b"]) + } finally { + viewer.app.renderer.destroy() + } + }) + + test("falls back to project discovery when the routed session has no workflows", async () => { + const viewer = await renderDagInspector({ + projectWorkflows: [projectWorkflow({ id: "wf-p1", session_id: "ses_other" })], + summary: (sessionID) => + Promise.resolve({ + data: + sessionID === SESSION_ID + ? [] + : [wfSummary({ id: "wf-p1", title: "Discovered elsewhere", status: "running" })], + }), + }) + try { + await viewer.app.waitForFrame((frame) => frame.includes("Discovered elsewhere")) + } finally { + viewer.app.renderer.destroy() + } + }) + + test("without session context the empty state stays honest about what was consulted", async () => { const viewer = await renderDagInspector({ initialRoute: { name: "dag" } }) try { - await viewer.app.waitForFrame((frame) => frame.includes("No session context")) - // The no-context branch is local-only; it must not touch the server. + await viewer.app.waitForFrame((frame) => frame.includes("No DAG workflows")) + // The project list is the discovery source that does not depend on the + // route's sessionID chain — it is consulted even without a session, + // while session summaries never run (an empty list has no sessions). + expect(viewer.listCalls()).toBe(1) expect(viewer.summaryCalls()).toEqual([]) } finally { viewer.app.renderer.destroy()