diff --git a/.github/releases/v1.0.37.md b/.github/releases/v1.0.37.md new file mode 100644 index 0000000000..3a3d134488 --- /dev/null +++ b/.github/releases/v1.0.37.md @@ -0,0 +1,44 @@ +## opencode {VERSION} + +{Prerelease/Stable} release from `{branch}` branch. DAG replan topology refresh: equal-count replans now surface a `graphRev` revision that makes the TUI re-fetch superseded node graphs exactly once instead of rendering stale topology. + +--- + +### 🎯 Features + +- **DAG replan topology refresh, #469**: an equal-count replan (node count and status distribution unchanged) previously never triggered a TUI node re-fetch, so the inspector and an expanded sidebar kept rendering superseded topology until the next unrelated state change. The existing `workflow.graph_rev` counter is now exposed end to end as `graphRev` (core summary, schema contract, HttpAPI response, regenerated SDK types) and folded into the inspector and sidebar refresh signatures, so a revision-only change triggers exactly one authoritative node re-fetch while no-op summaries stay refetch-free and no polling is added. + +--- + +### βš™οΈ CI / Engineering + +- `spec_git/policy.yaml` required checks realigned, 29e876ac2c: SpecGit acceptance for dev deliveries now requires the `Typecheck` and `Unit Tests (linux)` checks instead of the `unit-tests`/`e2e-tests` ids, matching the dev gates where E2E does not block. +- JS SDK regenerated: the generated `DagWorkflowSummary` types carry the new `graphRev` field, and the CI `Check generated SDK` freshness gate passes. + +--- + +### πŸ§ͺ Test Summary + +- New `packages/tui/test/feature-plugins/dag-panel.test.tsx` regression suite: equal-count replan bumps `graphRev` alone and refetches exactly once; a no-op summary replacement with the same `graphRev` and counts does not refetch. +- Companion coverage: inspector refetch-once (`dag-inspector.test.tsx`), equal-count replan summary emission (`dag-summary-publisher-behavior.test.ts`), core summary store revision (`dag-store-summaries.test.ts`), and the HttpAPI contract exercise asserting `summary.graphRev`. + +``` +typecheck: root 29/29 tasks green +test:dag-core: pass +test:httpapi:ci: 230 pass / 0 fail +focused suites: inspector 29 pass, sidebar/sync 7 pass +CI @ 29e876ac2c: Typecheck / Unit Tests (linux) / E2E linux+windows / CodeQL all pass +lint: 4836 warnings (ratchet 4850) +``` + +--- + +### πŸ” Verification + +- DAG development workflow `dag_fe5fa90eeff6BrTFcV9rWDOSyK`: final review ACCEPT after R1 fixes, and `specgit finish --json` exited 0 with the PR #469 checklist complete. +- All 8 GitHub checks on PR #469 green at 29e876ac2c, including E2E on linux (2m38s) and windows (7m19s). +- Scope honesty: the only feature surface in this range is the DAG summary and TUI refresh signature; no upstream sync content, no dependency changes, and no standalone architecture or refactor entries. + +--- + +**Full changelog:** [`{previous_tag}`...`{current_tag}`](https://github.com/LeXwDeX/OpenCode-GraphAgent/compare/{previous_tag}...{current_tag}) diff --git a/.specgit.yaml b/.specgit.yaml index 3b760b494e..abd02f7f60 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,8 +1,8 @@ version: 1 -delivery: dag-project-discovery +delivery: shell-silence-guard context: kind: branch - branch: feat/435-dag-project-discovery + branch: feat/433-shell-silence-guard issues: - - 435 -pr: 490 + - 433 +pr: 491 diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index 86505927fa..0000000000 --- a/CLAUDE.md +++ /dev/null @@ -1,288 +0,0 @@ -# CLAUDE.md - -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -## What this is - -**OpenCode-GraphAgent** (product name "GraphAgent"): a fork of the MIT-licensed -[opencode](https://github.com/anomalyco/opencode) terminal AI agent that adds a -**DAG workflow engine** for multi-agent orchestration. A task is decomposed into a -dependency graph of child-agent sessions, driven to completion with durable, -crash-recoverable, inspectable state. Upstream opencode capabilities (multi-provider -LLM, built-in LSP, TUI/desktop/web clients, client/server architecture) are preserved. - -**`AGENTS.md` is the canonical contributor guide.** It holds the full style guide, -git workflow (铁律), andδΊŒζ¬‘εΌ€ε‘ (extending) invariants. Read it for *how* to write -code here; this file covers the *what* and the big-picture architecture, and does not -repeat AGENTS.md. Default branch is `main`. - -## Commands - -Requirements: **Bun 1.3+** (`packageManager: bun@1.3.14`). All commands run from repo -root unless noted. - -```bash -bun install # install (postinstall fixes node-pty) - -# Run the app (bun dev == local equivalent of the built `opencode` CLI) -bun dev # TUI, in packages/opencode by default -bun dev # TUI against another dir (`bun dev .` for repo root) -bun dev serve # headless HTTP API server (default port 4096) -bun dev serve --port 8080 # custom port -bun dev web # server + web UI -bun run --cwd packages/app dev # web app dev server (needs `bun dev serve` running) -bun run --cwd packages/desktop dev # Electron desktop app - -# Quality gates -bun typecheck # turbo typecheck across all packages (the commit gate) -bun typecheck # also runnable from a package dir, e.g. packages/opencode -bun lint # oxlint, ratcheted: --max-warnings=4852 (see below) - -# Tests β€” NEVER run from repo root (guard: do-not-run-tests-from-root; bunfig enforces it) -cd packages/opencode && bun test # full suite (only-failures shown) -cd packages/opencode && bun test path/to/file.test.ts # one file -cd packages/opencode && bun test --test-name-pattern "pattern" # filtered tests -cd packages/opencode && bun run test:dag-core # DAG scheduling/state-machine coverage gate -cd packages/opencode && bun run test:httpapi # HTTP API contract exerciser (3 modes) - -# Build & codegen -./packages/opencode/script/build.ts --single # standalone binary β†’ packages/opencode/dist//bin/opencode -./packages/sdk/js/script/build.ts # regenerate the JS SDK from the OpenAPI spec (after HTTP route changes) -bun run generate # root: regen SDK + openapi.json + format (wrapper of the above) -``` - -**`bun typecheck` (`tsgo --noEmit`) is the real gate.** `bun run build` uses esbuild -and transpiles only β€” a green build can still ship a missing import or non-existent API. -Never invoke `tsc` directly. - -**Lint ratchet:** `bun lint` runs `oxlint --max-warnings=4852`. The threshold only ever -tightens β€” new warnings fail CI and the pre-commit hook. When you fix existing warnings, -lower the number in the root `package.json` `lint` script to match (rationale recorded in -the `_lint_ratchet_note` field there). `oxlint` is `typeAware: true`. - -Pre-commit (husky) runs `lint` + `typecheck`. `post-checkout`/`pre-push` hooks also exist. - -## Architecture - -### Monorepo layout (Bun workspaces + Turborepo) - -`packages/core` (`@opencode-ai/core`) is the framework layer: pure domain logic, the -plugin/SDK, schema, storage, event system, and the **pure half of the DAG engine**. -`packages/opencode` (`opencode`) is the application: the CLI/server entrypoint, session -runtime, HTTP server, and the **execution half of the DAG engine**. `core` has no -dependency on `opencode`; the arrow points the other way. - -Key packages: - -| Package | Role | -|---|---| -| `packages/core` | Domain primitives, storage, schema, events, **pure DAG state machine/projector/store** | -| `packages/opencode` | CLI + headless server, session runtime, **DAG execution/loop/spawn/admission/recovery** | -| `packages/tui` | Terminal UI (SolidJS + opentui), incl. the DAG inspector (`src/feature-plugins/system/dag-inspector.tsx`) | -| `packages/app` Β· `packages/web` Β· `packages/desktop` | Web components / web app / Electron wrapper | -| `packages/sdk/js` | `@opencode-ai/sdk` β€” **generated** from the server's OpenAPI spec (`src/v2/gen`) | -| `packages/plugin` Β· `packages/schema` Β· `packages/protocol` Β· `packages/client` | Plugin SDK, event/schema definitions, wire protocol, server client | - -### The DAG engine is split across two packages (the non-obvious part) - -The workflow engine is the fork's reason for existing. It is deliberately divided: - -- **`packages/core/src/dag`** β€” *pure, side-effect-free*: declared state-machine transition - tables (`core/transitions.ts`), dependency graph + cycle/dangling validation - (`core/graph.ts`), wave-based scheduler (`core/scheduling.ts`), replan fragment merge - (`core/replan.ts`), and the **event projector** (`projector.ts`) that writes the SQLite - read model *inside* the event-publish transaction. History is event replay, not a log - table. `store.ts` / `sql.ts` are the persistence boundary. -- **`packages/opencode/src/dag`** β€” *effectful execution*: the workflow service (`dag.ts`, - `workflows.ts`), the execution loop (`runtime/loop.ts`), spawning real child sessions per - node (`runtime/spawn.ts`, same path as the `task` tool), deep-mode admission Q&A - (`admission.ts`), the `design`/`diff` review lifecycle with implementation-fingerprint - contracts (`review-lifecycle.ts`), lazy evidence-based crash recovery - (`runtime/recovery.ts`), and prompt-template resolution (`templates/`). - -A node never names its own model β€” the graph declares which nodes are *critical* and -`.opencode/dag.jsonc` decides what model runs each tier (`advanced` / `standard`). -Agents drive workflows through a single `workflow` tool; humans observe/control via the -TUI DAG inspector or the `GET/POST /dag*` HTTP routes. - -### Effect-TS is the composition backbone - -The codebase is built on `effect` 4.0.0-beta. Services are `Context.Tag`s wired through -`Layer`s. Two parallel composition systems coexist and **do not share wiring**: - -1. `X.defaultLayer` / `AppLayer` β€” the primary Effect layer graph. -2. `LayerNode` (`.node` exports, `LayerNode.buildLayer`) β€” a separate node-based system. - -Both demand **self-contained layers**: a `defaultLayer` must `Layer.provide` every -dependency its body `yield*`s. `Layer.provideMerge(self, layer)` builds `layer` in -isolation, and `Layer.mergeAll` does not cross-provide siblings β€” so a layer that quietly -assumes an ambient service will compile clean and crash at runtime in a different entry -point. Optional/heavyweight cross-deps (Provider, MCP, HttpClient) are resolved lazily via -`Effect.serviceOption(Tag)` at the call site. See AGENTS.md "Extending the Codebase" for -the full invariant list β€” the build will not catch violations of these. - -### Configuration & data files (all under `.opencode/`) - -| Path | Purpose | -|---|---| -| `.opencode/dag.jsonc` | Model tiers + `thinking_depth` for DAG child sessions (global counterpart in opencode config dir) | -| `.opencode/workflows/*.yaml` | Project-local saved workflow specs; curated workflows live in the config repository | -| `.opencode/dag-prompts/*.md` | Project-local node prompt templates referenced by `prompt_template.id` | -| `.opencode/command/*.md` | Custom slash commands (`commit`, `issues`, `changelog`, `translate`, `learn`, …) | -| `.opencode/opencode.jsonc` | Main app config | - -Curated *global* workflows live in a separate repo, [`LeXwDeX/opencode-dag-config`](https://github.com/LeXwDeX/opencode-dag-config); config-only changes belong there, not here. `dag.jsonc` and the workflow library are read lazily β€” edits apply to the next workflow start without a restart. - -### Spec-driven & domain docs - -- **`openspec/`** β€” spec-driven change proposals. `openspec/changes//` holds - `proposal.md` / `design.md` / `tasks.md` / `specs/`; `openspec/specs/` holds the - established capability specs. Active proposals (e.g. `harden-goal-state-machine`, - `internalize-dag-block-capabilities`) define in-flight work. -- **`CONTEXT-MAP.md` β†’ `CONTEXT.md`** β€” multi-context domain docs. `CONTEXT-MAP.md` is the - index; read the linked `CONTEXT.md`(s) relevant to the area before working in it. -- **`docs/agents/`** β€” issue-tracker workflow, triage labels, domain-doc conventions. - -## Critical, non-obvious rules - -These compile clean but bite at runtime or in CI β€” the build will not catch them: - -- **Regenerate the SDK after touching any HTTP API route.** `packages/sdk/js` is generated - from the server's OpenAPI spec; a stale SDK breaks the TUI at runtime (calling a client - method that doesn't exist) in a way typecheck can't catch. After route changes run - `./packages/sdk/js/script/build.ts`. CI's `Check generated SDK` step - (`bun run check:generated` = regen + `git diff --exit-code -- src/v2/gen`) enforces this. -- **Changing an HTTP route's request/response shape** requires updating its scenario in - `test/server/httpapi-exercise/index.ts`; `bun run test:httpapi --fail-on-missing` fails otherwise. -- **Don't hand-duplicate SDK types in TUI/plugin code** β€” re-export the generated type so a - server schema change surfaces as a typecheck error instead of silent drift. -- **Every event type the TUI consumes** must be `define()`d in `packages/schema` and listed - in `Event manifest.Definitions`, or the generated event union won't contain it. Ephemeral - push events (e.g. `dag.workflow.summary.updated`) stay OUT of the durable manifest β€” emit - via `GlobalBus`, never persist, design consumers to tolerate missed events (re-fetch on bootstrap). -- **Adding a service other services see:** find every consumer's `.node` list (not just its - `defaultLayer`) and add the new service's node there. A missing wire compiles clean and - fails silently (feature no-ops) rather than erroring. -- **Mixed license:** upstream code is MIT; the DAG engine - (`packages/core/src/dag/**`, `packages/opencode/src/dag/**`) is AGPL-3.0-or-later. Exact - boundaries are in `NOTICE`. Don't move AGPL code into MIT-licensed paths or vice versa. - -## Git workflow (summary β€” full rules in AGENTS.md) - -`feat/fix` branches β†’ PR (Typecheck gate) β†’ `dev` (fast integration, push runs full tests) β†’ -PR (full gate: Typecheck + Unit + E2E on linux+windows) β†’ `main` β†’ manual release. Direct -pushes to `main`/`dev` are blocked by GitHub Rulesets. Branch names: `{type}/{short-name}` -(`feat`, `fix`, `chore`, `docs`, `refactor`, `test`, `release`, `hotfix`), enforced by -Ruleset. Commits/PR titles: conventional `type(scope): summary`. All PRs must reference an -existing issue (`Fixes #N`). Curated DAG configs are owned by the `opencode-dag-config` repo. - - -## SpecGit delivery harness - -Managed by `specgit init`. Everything between the markers is regenerated -whenever init writes the harness (a fresh init, or `--force` when a policy -already exists); keep manual guidance outside them. - -### The delivery story - -- Start with `specgit issue ...`: it creates or reuses - the issues, branches, opens the draft pull request pre-filled with a - deterministic scaffold (the `Closes #n` line for every bound issue, - then Why / What changed / Evidence / Checklist sections), and writes - `.specgit.yaml`. Re-running resumes; it is idempotent. -- Issue bodies are filled at bootstrap, from the conversation: right after - `specgit issue` succeeds, edit each issue it created (`gh issue edit `) - with the discussed Why / Scope / Approach / Acceptance, then implement. - The PR scaffold's placeholders are advisory β€” fill those sections in as - you deliver; the closing references are the only body gate. The PR body - is written once at creation; no SpecGit command edits an existing PR - body, and the repository's own pull-request template is never read. -- A draft pull request always fails the verdict (`pr_draft`): before - `specgit finish`, mark it ready for review β€” `gh pr ready ` - on GitHub, `glab mr update --ready` on GitLab. -- Finish with `specgit finish`: the verdict, derived from real git, PR, - and CI evidence. Exit code 0 is the only "done". - -### Issue tags - -- Every bootstrap applies the title's `kind::` member - automatically; pass `--tags ` to choose the full set explicitly. -- Selection is pool-first: existing on-spec labels win verbatim; anything - missing is seeded from the built-in `kind::` catalog or the policy's - `tags:` declarations. Unknown vocabulary exits 2 naming the universe. -- Choose with restraint: at most one label per axis, none when unsure β€” - off-spec pool labels are reported (`tag_pool_dirty` warnings are for - humans) and never renamed by SpecGit. - -### Repair and diagnostics - -- `specgit pr` repairs the pull-request binding: with no arguments it - auto-discovers the pull request for this head branch, errors with a fix - when none is found, and refuses with a list when several match. -- `specgit status` shows local evidence only: record, state, drift, - origin. `specgit doctor` probes git, repository, origin, gh, and - policy. - -### The command surface - -- Ten commands: `specgit init`, `specgit setup`, `specgit issue`, - `specgit pr`, `specgit finish`, `specgit bind`, `specgit unbind`, - `specgit status`, `specgit accept`, `specgit doctor`. -- `specgit setup` installs the agent entry points (commands for opencode, - portable skills for other tools); `specgit bind`, `specgit unbind`, - and `specgit accept` are automation aliases for scripts and CI. - -### Before creating an issue, check for duplicates - -- Before running `specgit issue` with a new title, search the tracker for - similar open work: `gh issue list` with keywords from the title - (state, labels, and search terms via `gh search issues`). -- Open and read every plausible candidate (`gh issue view `) β€” compare - the WHY, not just the wording. -- If a candidate covers the same WHY, continue that issue instead of - creating a new one; if it is close but different, say how they differ. -- When unsure, ask the requester to decide between continuing the existing - issue and creating a duplicate. The team ships one line of work per WHY, - never two. - -### Issue granularity - -One issue = one independently verifiable WHY. If a deliverable cannot be -verified on its own evidence, split it before binding. - -### Iron rules - -- `specgit finish` exit code other than 0: never request merge. Fix the - delivery, not the gate. -- Never weaken `spec_git/policy.yaml` to make a verdict pass. -- `--json` is the only parse surface: stdout is exactly one JSON - document; never scrape human-readable output. - -### Agent contract essentials - -- **SpecGit is the default way of working here.** Any non-trivial - task β€” a feature, a fix, a refactor, a docs change β€” is a delivery: - work items live in this tracker as issues, never in private task - lists or conversational checklists. The trigger is the decision to - start: the moment the conversation settles and you begin turning - the plan into changes, the FIRST action is - `specgit issue : ...` β€” before any file edit. - Working without a binding is a contract violation, not a style - choice. Immediately after bootstrap, fill each issue body - (Why / Scope / Approach / Acceptance) from the discussion with - `gh issue edit`, then implement. Mid-conversation inventories - ("let me list everything to do") become issues, not chat - artifacts. Trivial replies and read-only questions need none of - this. -- The one rule: a delivery is done if and only if `specgit finish` - exits `0`. Never declare completion from task lists, file states, or - test runs you performed yourself. -- Branch on exit codes, not phrasing: `1` = evidence complete, fix what - the gates named; `3` = evidence missing, fix the environment first - (`specgit doctor`). Never present exit `3` as success. -- Keep the `Closes #n` references in the PR body intact; after changing - the PR body, head branch, or CI, re-run `specgit finish`. Never - bypass or reconfig a required check to make acceptance pass. -- Forge evidence flows through the user's authenticated CLI session only - (`gh` / `glab`): never read, log, or pass around tokens. -<!-- specgit:block:end --> diff --git a/packages/core/src/dag/store.ts b/packages/core/src/dag/store.ts index 861bafa78d..1b459101e0 100644 --- a/packages/core/src/dag/store.ts +++ b/packages/core/src/dag/store.ts @@ -75,6 +75,8 @@ export interface WorkflowSummary { id: string title: string status: string + /** Topology invalidation token (#468): bumped by replan, so TUI refresh signatures can detect equal-count replans. */ + graphRev: number nodeCount: number completedNodes: number runningNodes: number @@ -323,6 +325,7 @@ export const layer = Layer.effect( id: wf.id, title: wf.title, status: wf.status, + graphRev: wf.graph_rev, ...(counts.get(wf.id) ?? { nodeCount: 0, completedNodes: 0, runningNodes: 0, failedNodes: 0, skippedNodes: 0, queuedNodes: 0 }), escalatedNodes: escalatedByWorkflow.get(wf.id) ?? 0, })) diff --git a/packages/core/src/git.ts b/packages/core/src/git.ts index b757641ae4..59cde43350 100644 --- a/packages/core/src/git.ts +++ b/packages/core/src/git.ts @@ -852,6 +852,13 @@ export const layer = Layer.effect( }) }) + // forceRequired must be derived from locale-stable porcelain state only: + // git's refusal prose is translated (e.g. zh_CN catalogs) and never matches reliably. + const worktreeDirty = Effect.fnUntraced(function* (directory: AbsolutePath) { + const status = yield* execute(directory, proc)(["status", "--porcelain"]).pipe(Effect.result) + return status._tag === "Success" && status.success.exitCode === 0 && status.success.text.trim() !== "" + }) + const worktreeRun = Effect.fnUntraced(function* ( operation: "create" | "remove" | "list", repository: Repository, @@ -872,7 +879,11 @@ export const layer = Layer.effect( operation, directory: worktreeDirectory, message, - forceRequired: operation === "remove" && /contains modified or untracked files|is dirty/i.test(message), + forceRequired: + operation === "remove" && + result.exitCode === 128 && + worktreeDirectory !== undefined && + (yield* worktreeDirty(worktreeDirectory)), }) }) diff --git a/packages/core/test/dag-rev-view-legacy.test.ts b/packages/core/test/dag-rev-view-legacy.test.ts index 7a4520d4ae..e692be3b29 100644 --- a/packages/core/test/dag-rev-view-legacy.test.ts +++ b/packages/core/test/dag-rev-view-legacy.test.ts @@ -104,6 +104,7 @@ describe("Train A rev-view β€” legacy rows render unchanged (A-p4 PIN)", () => { id: "wf-legacy", title: "Legacy", status: "running", + graphRev: 1, nodeCount: 4, completedNodes: 2, runningNodes: 0, diff --git a/packages/core/test/dag-store-summaries.test.ts b/packages/core/test/dag-store-summaries.test.ts index b00522d987..7f5a6111fd 100644 --- a/packages/core/test/dag-store-summaries.test.ts +++ b/packages/core/test/dag-store-summaries.test.ts @@ -94,6 +94,7 @@ describe("DagStore.getWorkflowSummaries (SQL aggregation)", () => { id: "wf-mixed", title: "Mixed", status: "running", + graphRev: 1, nodeCount: 7, completedNodes: 2, runningNodes: 1, @@ -106,6 +107,7 @@ describe("DagStore.getWorkflowSummaries (SQL aggregation)", () => { id: "wf-empty", title: "Empty", status: "running", + graphRev: 1, nodeCount: 0, completedNodes: 0, runningNodes: 0, diff --git a/packages/core/test/filesystem/watcher.test.ts b/packages/core/test/filesystem/watcher.test.ts index f0826e37e7..7dfd298455 100644 --- a/packages/core/test/filesystem/watcher.test.ts +++ b/packages/core/test/filesystem/watcher.test.ts @@ -225,12 +225,33 @@ describeWatcher("Watcher", () => { const branch = `watch-${Math.random().toString(36).slice(2)}` yield* ready(directory) yield* Effect.promise(() => $`git branch ${branch}`.cwd(directory).quiet()) + // Contract for an existing HEAD: once the backend stream has seen HEAD, an + // update publishes "change" and removal publishes "unlink". The first write + // to a pre-existing but stream-unseen HEAD is backend vocabulary (fs-events + // may flag ItemCreated, surfacing as "add" on macOS), so it is consumed only + // to record HEAD in the stream β€” its classification is deliberately not + // asserted, while the asserted "change" and "unlink" must match exactly. + yield* eventuallyUpdate( + (event) => event.file === head, + () => fs.writeFileString(head, `ref: refs/heads/${branch}\n`), + ) + const renamed = `watch-${Math.random().toString(36).slice(2)}` + yield* Effect.promise(() => $`git branch ${renamed}`.cwd(directory).quiet()) expect( - yield* nextUpdate((event) => event.file === head, fs.writeFileString(head, `ref: refs/heads/${branch}\n`)), + yield* nextUpdate( + (event) => event.file === head && event.event === "change", + fs.writeFileString(head, `ref: refs/heads/${renamed}\n`), + ), ).toEqual({ file: head, event: "change", }) + expect( + yield* nextUpdate((event) => event.file === head && event.event === "unlink", fs.remove(head)), + ).toEqual({ + file: head, + event: "unlink", + }) }), { git: true }, ), @@ -247,14 +268,27 @@ describeWatcher("Watcher", () => { yield* Effect.addFinalizer(() => Effect.promise(() => fs.rm(actual, { recursive: true, force: true }))) yield* ready(directory) const head = path.join(directory, ".git", "HEAD") + const resolved = path.join(actual, "HEAD") const branch = `watch-${Math.random().toString(36).slice(2)}` yield* Effect.promise(() => $`git branch ${branch}`.cwd(directory).quiet()) + // Same contract as the direct .git/HEAD case, observed through the symlink: + // events carry the realpath of the store, so both the update and the + // removal are asserted against actual/HEAD exactly. + yield* eventuallyUpdate( + (event) => event.file === resolved, + () => afs.writeFileString(head, `ref: refs/heads/${branch}\n`), + ) + const renamed = `watch-${Math.random().toString(36).slice(2)}` + yield* Effect.promise(() => $`git branch ${renamed}`.cwd(directory).quiet()) expect( yield* nextUpdate( - (event) => event.file === path.join(actual, "HEAD"), - afs.writeFileString(head, `ref: refs/heads/${branch}\n`), + (event) => event.file === resolved && event.event === "change", + afs.writeFileString(head, `ref: refs/heads/${renamed}\n`), ), - ).toEqual({ file: path.join(actual, "HEAD"), event: "change" }) + ).toEqual({ file: resolved, event: "change" }) + expect( + yield* nextUpdate((event) => event.file === resolved && event.event === "unlink", afs.remove(head)), + ).toEqual({ file: resolved, event: "unlink" }) }), { git: true, diff --git a/packages/core/test/project-copy.test.ts b/packages/core/test/project-copy.test.ts index 8c01e92c50..377fa8691e 100644 --- a/packages/core/test/project-copy.test.ts +++ b/packages/core/test/project-copy.test.ts @@ -192,6 +192,44 @@ describe("ProjectCopy", () => { }), ) + it.live("requires force to remove a git worktree with tracked modifications", () => + Effect.gen(function* () { + const input = yield* setup() + const copy = yield* ProjectCopy.Service + const temp = yield* Effect.promise(() => fs.realpath(path.dirname(input.root.path))) + const parent = abs(path.join(temp, path.basename(input.root.path) + "-copy-tracked")) + yield* Effect.addFinalizer(() => + Effect.promise(() => fs.rm(parent, { recursive: true, force: true })).pipe(Effect.ignore), + ) + yield* Effect.promise(() => Bun.write(path.join(input.sourceDirectory, "tracked.txt"), "base")) + yield* Effect.promise(() => $`git add tracked.txt`.cwd(input.sourceDirectory).quiet()) + yield* Effect.promise(() => $`git commit -m tracked`.cwd(input.sourceDirectory).quiet()) + const created = yield* copy.create({ + projectID: input.projectID, + strategy: gitWorktree, + sourceDirectory: input.sourceDirectory, + directory: parent, + name: "copy", + }) + yield* Effect.promise(() => Bun.write(path.join(created.directory, "tracked.txt"), "modified")) + + const error = yield* copy + .remove({ projectID: input.projectID, directory: created.directory, force: false }) + .pipe(Effect.flip) + + expect(error).toBeInstanceOf(Git.WorktreeError) + if (error instanceof Git.WorktreeError) { + expect(error.operation).toBe("remove") + expect(error.forceRequired).toBe(true) + } + expect(yield* stored(input.projectID)).toContainEqual({ directory: created.directory, strategy: "git_worktree" }) + expect(yield* Effect.promise(() => Bun.file(path.join(created.directory, "tracked.txt")).exists())).toBe(true) + + yield* copy.remove({ projectID: input.projectID, directory: created.directory, force: true }) + expect(yield* Effect.promise(() => Bun.file(created.directory).exists())).toBe(false) + }), + ) + it.live("preserves copies whose stored strategy is unavailable", () => Effect.gen(function* () { const input = yield* setup() 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<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/server/routes/instance/httpapi/groups/dag.ts b/packages/opencode/src/server/routes/instance/httpapi/groups/dag.ts index 34a18a8785..dcff8a5170 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/dag.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/dag.ts @@ -61,6 +61,8 @@ export const WorkflowSummaryResponse = Schema.Struct({ id: Schema.String, title: Schema.String, status: Schema.String, + // Topology invalidation token (#468): bumped by replan, so TUI refresh signatures can detect equal-count replans. + graphRev: Schema.Number, nodeCount: Schema.Number, completedNodes: Schema.Number, runningNodes: Schema.Number, diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/dag.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/dag.ts index 04ef202fd6..47f3e19ee7 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/dag.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/dag.ts @@ -124,6 +124,7 @@ export const dagHandlers = HttpApiBuilder.group(InstanceHttpApi, "dag", (handler id: s.id, title: s.title, status: s.status, + graphRev: s.graphRev, nodeCount: s.nodeCount, completedNodes: s.completedNodes, runningNodes: s.runningNodes, 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 <seconds>` 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<void>((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: </bad-example>` } -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: </bad-example>` } -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 <directory> && <command>` 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/dag/dag-node-started-guard.test.ts b/packages/opencode/test/dag/dag-node-started-guard.test.ts index 3deeff9563..ca587c7815 100644 --- a/packages/opencode/test/dag/dag-node-started-guard.test.ts +++ b/packages/opencode/test/dag/dag-node-started-guard.test.ts @@ -107,6 +107,7 @@ describe("DagProjector: NodeStarted status guard", () => { id: dagID, title: "guard", status: "pending", + graphRev: 1, nodeCount: 1, completedNodes: 0, runningNodes: 1, @@ -119,6 +120,7 @@ describe("DagProjector: NodeStarted status guard", () => { id: otherDagID, title: "other guard", status: "pending", + graphRev: 1, nodeCount: 1, completedNodes: 0, runningNodes: 0, diff --git a/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts b/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts index 1353cb87ff..4629150715 100644 --- a/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts +++ b/packages/opencode/test/dag/dag-summary-publisher-behavior.test.ts @@ -69,11 +69,12 @@ function workflow(id: string, sessionId: string, projectId: string): WorkflowRow } } -function summary(id: string, completedNodes: number): WorkflowSummary { +function summary(id: string, completedNodes: number, graphRev = 1): WorkflowSummary { return { id, title: id, status: "running", + graphRev, nodeCount: completedNodes, completedNodes, runningNodes: 0, @@ -489,6 +490,37 @@ describe("DagSummaryPublisher behavior", () => { ).pipe(Effect.provide(runtime(state, bus))) }) + it.instance("an equal-count replan (graphRev-only change) still emits a fresh summary", () => { + const state = control() + const bus = {} satisfies EventControl + state.sessions.set("dag-replan", "ses-replan") + state.summaries.set("ses-replan", [summary("dag-replan", 3, 1)]) + + return withCollector((collector) => + Effect.gen(function* () { + yield* (yield* DagSummaryPublisher.Service).init() + yield* publishNodeEvents(bus, "dag-replan", 1) + yield* pollWithTimeout( + Effect.sync(() => collector.emissions.length === 1 ? true : undefined), + "pre-replan summary was not emitted", + ) + + // Equal-count replan: node counts and statuses are identical, only the + // topology revision moved. The publisher must NOT content-dedupe β€” the + // TUI change signatures depend on seeing the new graphRev propagate. + state.summaries.set("ses-replan", [summary("dag-replan", 3, 2)]) + yield* publishNodeEvents(bus, "dag-replan", 1) + yield* pollWithTimeout( + Effect.sync(() => (collector.emissions.at(-1)?.summaries[0]?.graphRev === 2 ? true : undefined)), + "graphRev-only replan change was not emitted", + ) + + expect(state.reads.get("ses-replan")).toBe(2) + expect(collector.emissions[1].summaries).toEqual([summary("dag-replan", 3, 2)]) + }), + ).pipe(Effect.provide(runtime(state, bus))) + }) + it.instance("a timeout escalation triggers a fresh summary recompute (F10)", () => { const state = control() const bus = {} satisfies EventControl diff --git a/packages/opencode/test/dag/dag-summary-publisher.test.ts b/packages/opencode/test/dag/dag-summary-publisher.test.ts index b84e74638e..f09b593143 100644 --- a/packages/opencode/test/dag/dag-summary-publisher.test.ts +++ b/packages/opencode/test/dag/dag-summary-publisher.test.ts @@ -17,6 +17,7 @@ describe("DagSummaryPublisher contract (stateless derived view)", () => { id: "wf-1", title: "Test", status: "running", + graphRev: 1, nodeCount: 0, completedNodes: 0, runningNodes: 0, @@ -26,7 +27,7 @@ describe("DagSummaryPublisher contract (stateless derived view)", () => { escalatedNodes: 0, } // If this compiles, the shape is correct. The keys must match the TUI type. - const keys: (keyof WorkflowSummary)[] = ["id", "title", "status", "nodeCount", "completedNodes", "runningNodes", "failedNodes", "skippedNodes", "queuedNodes", "escalatedNodes"] + const keys: (keyof WorkflowSummary)[] = ["id", "title", "status", "graphRev", "nodeCount", "completedNodes", "runningNodes", "failedNodes", "skippedNodes", "queuedNodes", "escalatedNodes"] expect(Object.keys(s).sort()).toEqual([...keys].sort()) }) 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/server/httpapi-exercise/backend.ts b/packages/opencode/test/server/httpapi-exercise/backend.ts index e89b9dd86b..835d361fb5 100644 --- a/packages/opencode/test/server/httpapi-exercise/backend.ts +++ b/packages/opencode/test/server/httpapi-exercise/backend.ts @@ -53,9 +53,9 @@ export function callAuthProbe(scenario: ActiveScenario, credentials: "missing" | }) } -type CachedApp = BackendApp & { readonly dispose: () => Promise<void> } +export type CachedApp = BackendApp & { readonly dispose: () => Promise<void> } -const appCache: Partial<Record<string, CachedApp>> = {} +export const appCache: Partial<Record<string, CachedApp>> = {} export async function disposeApps(heartbeat?: (label: string) => void) { const apps = Object.values(appCache) diff --git a/packages/opencode/test/server/httpapi-exercise/index.ts b/packages/opencode/test/server/httpapi-exercise/index.ts index 5b9ee14d9f..c875c3a3b1 100644 --- a/packages/opencode/test/server/httpapi-exercise/index.ts +++ b/packages/opencode/test/server/httpapi-exercise/index.ts @@ -24,7 +24,6 @@ import path from "path" import { array, boolean, check, isRecord, message, object, stable } from "./assertions" import { controlledPtyInput, http, route } from "./dsl" import { - cleanupExercisePaths, exerciseConfigDirectory, exerciseDataDirectory, exerciseDatabasePath, @@ -33,8 +32,8 @@ import { import { color, printHeader, printResults } from "./report" import { coverageResult, parseOptions, routeKey, routeKeys, selectedScenarios } from "./routing" import { runScenario } from "./runner" -import { disposeApps } from "./backend" import { runtime } from "./runtime" +import { runMainWithHardExit, teardown } from "./teardown" import { type Options, type Scenario } from "./types" import { startProgressWatchdog } from "./watchdog" @@ -1853,6 +1852,10 @@ const scenarios: Scenario[] = [ check(typeof summary.status === "string", "summary should have status") check(typeof summary.title === "string", "summary should have title") check(typeof summary.escalatedNodes === "number", "summary should have escalatedNodes") + // #468: graphRev is the topology invalidation token β€” an equal-count + // replan bumps it alone so TUI signatures can detect the change. + check(typeof summary.graphRev === "number", "summary should have graphRev") + check(summary.graphRev === 1, "fresh fixture workflow should carry graphRev 1") }), ), @@ -2212,13 +2215,7 @@ const llmScenarios = new Set([ ]) const main = Effect.gen(function* () { - yield* Effect.addFinalizer(() => - Effect.promise(() => disposeApps(options.heartbeat)).pipe( - Effect.andThen(Effect.sync(() => options.heartbeat?.("teardown: cleanupExercisePaths"))), - Effect.andThen(cleanupExercisePaths), - Effect.andThen(Effect.sync(() => options.heartbeat?.("teardown: complete"))), - ), - ) + yield* Effect.addFinalizer(() => Effect.promise(() => teardown(options))) const parsed = parseOptions(Bun.argv.slice(2)) const options: Options = parsed.progress ? { ...parsed, heartbeat: startProgressWatchdog() } : parsed const modules = yield* Effect.promise(() => runtime()) @@ -2262,10 +2259,7 @@ const main = Effect.gen(function* () { return undefined }) -Effect.runPromise(main.pipe(Effect.provide(TestLLMServer.layer), Effect.scoped)).then( - () => process.exit(0), - (error: unknown) => { - console.error(`${color.red}${message(error)}${color.reset}`) - process.exit(1) - }, +runMainWithHardExit( + Effect.runPromise(main.pipe(Effect.provide(TestLLMServer.layer), Effect.scoped)), + (code) => process.exit(code), ) diff --git a/packages/opencode/test/server/httpapi-exercise/runner.ts b/packages/opencode/test/server/httpapi-exercise/runner.ts index bc3b7effee..6d95a6c95b 100644 --- a/packages/opencode/test/server/httpapi-exercise/runner.ts +++ b/packages/opencode/test/server/httpapi-exercise/runner.ts @@ -290,7 +290,7 @@ const resetState = Effect.promise(async () => { */ const CLEANUP_STEP_TIMEOUT_MS = 10_000 -async function bounded(label: string, work: () => Promise<unknown>, ms = CLEANUP_STEP_TIMEOUT_MS) { +export async function bounded(label: string, work: () => Promise<unknown>, ms = CLEANUP_STEP_TIMEOUT_MS) { let timer: ReturnType<typeof setTimeout> | undefined const timeout = new Promise<"timeout">((resolve) => { timer = setTimeout(() => resolve("timeout"), ms) diff --git a/packages/opencode/test/server/httpapi-exercise/teardown.test.ts b/packages/opencode/test/server/httpapi-exercise/teardown.test.ts new file mode 100644 index 0000000000..c0d4557138 --- /dev/null +++ b/packages/opencode/test/server/httpapi-exercise/teardown.test.ts @@ -0,0 +1,103 @@ +import { afterAll, describe, expect, test } from "bun:test" +import { Flag } from "@opencode-ai/core/flag/flag" +import path from "path" +import type { CachedApp } from "./backend" + +// The exercise harness re-points the process env at its isolated DB/XDG roots +// at import time. Import it lazily under dedicated preserved paths, then +// restore everything so this file never leaks those overrides into the rest of +// the bun test process (single shared process). +const exerciseDb = path.join(process.env.TMPDIR ?? "/tmp", `opencode-teardown-regression-${process.pid}.db`) +const exerciseGlobal = path.join(process.env.TMPDIR ?? "/tmp", `opencode-teardown-regression-${process.pid}`) +const envKeys = [ + "OPENCODE_DB", + "OPENCODE_HTTPAPI_EXERCISE_DB", + "OPENCODE_HTTPAPI_EXERCISE_GLOBAL", + "OPENCODE_DISABLE_SHARE", + "XDG_DATA_HOME", + "XDG_CONFIG_HOME", + "XDG_STATE_HOME", + "XDG_CACHE_HOME", +] as const +const savedEnv: Record<string, string | undefined> = {} +const savedFlagDb = Flag.OPENCODE_DB + +for (const key of envKeys) { + savedEnv[key] = process.env[key] +} + +process.env.OPENCODE_HTTPAPI_EXERCISE_DB = exerciseDb +process.env.OPENCODE_HTTPAPI_EXERCISE_GLOBAL = exerciseGlobal + +const { createExitBackstop, exitBackstop, runMainWithHardExit, teardown } = await import("./teardown") +const { appCache } = await import("./backend") + +Flag.OPENCODE_DB = savedFlagDb +for (const key of envKeys) { + const value = savedEnv[key] + if (value === undefined) delete process.env[key] + else process.env[key] = value +} + +afterAll(async () => { + exitBackstop.disarm() + const fs = await import("fs/promises") + await fs.rm(exerciseDb, { force: true }).catch(() => undefined) + await fs.rm(exerciseGlobal, { recursive: true, force: true }).catch(() => undefined) +}) + +describe("httpapi-exercise main teardown (#472 regression)", () => { + test( + "teardown completes when an app dispose never settles", + async () => { + const poison = { + dispose: () => new Promise<void>(() => {}), + request: () => { + throw new Error("poison app must never serve a request") + }, + } satisfies CachedApp + appCache["poison:poison"] = poison + try { + const outcome = await Promise.race([ + teardown({}).then(() => "done" as const), + Bun.sleep(15_000).then(() => "hung" as const), + ]) + expect(outcome).toBe("done") + } finally { + delete appCache["poison:poison"] + } + }, + { timeout: 20_000 }, + ) + + test("armed backstop forces exit when settlement stalls", async () => { + const exits: number[] = [] + const backstop = createExitBackstop((code) => exits.push(code), 100) + backstop.arm() + await Bun.sleep(300) + expect(exits).toEqual([1]) + }) + + test("disarm cancels the forced exit", async () => { + const exits: number[] = [] + const backstop = createExitBackstop((code) => exits.push(code), 100) + backstop.arm() + backstop.disarm() + await Bun.sleep(200) + expect(exits).toEqual([]) + }) + + test("runMainWithHardExit exits 0 when the main fiber settles", async () => { + const exits: number[] = [] + runMainWithHardExit(Promise.resolve("settled"), (code) => exits.push(code)) + await Bun.sleep(50) + expect(exits).toEqual([0]) + }) + + test("runMainWithHardExit exits 1 when the main fiber rejects", async () => { + const exits: number[] = [] + runMainWithHardExit(Promise.reject(new Error("boom")), (code) => exits.push(code)) + await Bun.sleep(50) + expect(exits).toEqual([1]) + }) +}) diff --git a/packages/opencode/test/server/httpapi-exercise/teardown.ts b/packages/opencode/test/server/httpapi-exercise/teardown.ts new file mode 100644 index 0000000000..44587b9a38 --- /dev/null +++ b/packages/opencode/test/server/httpapi-exercise/teardown.ts @@ -0,0 +1,56 @@ +import { Effect } from "effect" +import { message } from "./assertions" +import { disposeApps } from "./backend" +import { cleanupExercisePaths } from "./environment" +import { color } from "./report" +import { bounded } from "./runner" +import { type Options } from "./types" + +export async function teardown(options: Pick<Options, "heartbeat">) { + // Main-scope twin of resetState's bounded cleanup: a dispose stalled on a + // ref'd outbound socket must degrade into a loud warning, not hang the whole + // composite `&&` chain (issue #472). Arming the backstop here β€” not at + // startup β€” because the scenario run itself may legitimately take minutes. + exitBackstop.arm() + await bounded("disposeApps", () => disposeApps(options.heartbeat)) + options.heartbeat?.("teardown: cleanupExercisePaths") + await bounded("cleanupExercisePaths", () => Effect.runPromise(cleanupExercisePaths)) + options.heartbeat?.("teardown: complete") +} + +export const HARD_EXIT_TIMEOUT_MS = 30_000 + +// process.exit must not depend on the main fiber settling: even with bounded +// teardown, a ref'd socket can keep the event loop alive past settlement. The +// wall-clock backstop guarantees the process always reaches an exit. +export function createExitBackstop(exit: (code: number) => void, ms = HARD_EXIT_TIMEOUT_MS) { + let timer: ReturnType<typeof setTimeout> | undefined + return { + arm() { + timer ??= setTimeout(() => { + console.error(`[cleanup] main scope settlement exceeded ${ms}ms β€” forcing exit`) + exit(1) + }, ms) + }, + disarm() { + clearTimeout(timer) + timer = undefined + }, + } +} + +export const exitBackstop = createExitBackstop((code) => process.exit(code)) + +export function runMainWithHardExit(main: Promise<unknown>, exit: (code: number) => void) { + main.then( + () => { + exitBackstop.disarm() + exit(0) + }, + (error: unknown) => { + exitBackstop.disarm() + console.error(`${color.red}${message(error)}${color.reset}`) + exit(1) + }, + ) +} 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/schema/src/dag-summary.ts b/packages/schema/src/dag-summary.ts index 638cd70ed9..f857df1878 100644 --- a/packages/schema/src/dag-summary.ts +++ b/packages/schema/src/dag-summary.ts @@ -9,6 +9,8 @@ export const WorkflowSummary = Schema.Struct({ id: Schema.String, title: Schema.String, status: Schema.String, + // Topology invalidation token (#468): bumped by replan, so TUI refresh signatures can detect equal-count replans. + graphRev: Schema.Number, nodeCount: Schema.Number, completedNodes: Schema.Number, runningNodes: Schema.Number, diff --git a/packages/sdk/js/.gitignore b/packages/sdk/js/.gitignore index 179980657b..9a5de3eebf 100644 --- a/packages/sdk/js/.gitignore +++ b/packages/sdk/js/.gitignore @@ -1 +1,2 @@ openapi.json +.build-race-logs/ diff --git a/packages/sdk/js/package.json b/packages/sdk/js/package.json index befaeb498c..df0125d538 100644 --- a/packages/sdk/js/package.json +++ b/packages/sdk/js/package.json @@ -7,7 +7,8 @@ "scripts": { "typecheck": "tsgo --noEmit", "build": "bun ./script/build.ts", - "check:generated": "bun run build && git diff --exit-code -- src/v2/gen" + "check:generated": "bun run build && git diff --exit-code -- src/v2/gen", + "test:build-race": "bun ./script/build-race-stress.ts" }, "exports": { ".": "./src/index.ts", diff --git a/packages/sdk/js/script/build-race-stress.ts b/packages/sdk/js/script/build-race-stress.ts new file mode 100644 index 0000000000..2a1c89a3b9 --- /dev/null +++ b/packages/sdk/js/script/build-race-stress.ts @@ -0,0 +1,55 @@ +#!/usr/bin/env bun +// Regression harness for the #475 openapi.json ownership race: two full +// `bun ./script/build.ts` processes are started simultaneously in this +// checkout, repeated for a bounded number of rounds. The invariant under +// test is that overlapping SDK builds never observe each other's cleanup β€” +// before the per-run-artifact repair, exactly one loser per round died at +// build.ts's final `rm openapi.json` with ENOENT. +// +// Run from packages/sdk/js: bun ./script/build-race-stress.ts +// SDK_BUILD_RACE_ROUNDS bounds the rounds (default 5). +import { fileURLToPath } from "url" +import { mkdirSync } from "fs" + +const rounds = Number(process.env.SDK_BUILD_RACE_ROUNDS ?? 5) +if (!Number.isInteger(rounds) || rounds < 1) { + console.error(`SDK_BUILD_RACE_ROUNDS must be a positive integer, got: ${rounds}`) + process.exit(2) +} + +const dir = fileURLToPath(new URL("..", import.meta.url)) +const logsDir = fileURLToPath(new URL("../.build-race-logs", import.meta.url)) +mkdirSync(logsDir, { recursive: true }) + +const enoentSignature = /[Nn]o such file or directory/ +let failures = 0 + +for (let round = 1; round <= rounds; round++) { + const procs = [0, 1].map(() => + Bun.spawn(["bun", "./script/build.ts"], { + cwd: dir, + stdout: "inherit", + stderr: "pipe", + }), + ) + const results = await Promise.all( + procs.map(async (proc, slot) => { + const [exitCode, stderr] = await Promise.all([proc.exited, new Response(proc.stderr).text()]) + await Bun.write(`${logsDir}/r${round}-b${slot}.err`, stderr) + return { slot, exitCode, stderr } + }), + ) + for (const { slot, exitCode, stderr } of results) { + if (exitCode !== 0 || enoentSignature.test(stderr)) { + failures++ + console.error(`FAIL round ${round} build ${slot}: exit=${exitCode} enoent=${enoentSignature.test(stderr)}`) + } + } + console.log(`round ${round}/${rounds} done (failures so far: ${failures})`) +} + +if (failures > 0) { + console.error(`build-race-stress FAILED: ${failures} failing run(s) across ${rounds} rounds; stderr logs in ${logsDir}`) + process.exit(1) +} +console.log(`build-race-stress OK: ${rounds * 2} simultaneous builds, no ownership-race failures`) diff --git a/packages/sdk/js/script/build.ts b/packages/sdk/js/script/build.ts index 1bd5e2f2ca..a7cdd4c831 100755 --- a/packages/sdk/js/script/build.ts +++ b/packages/sdk/js/script/build.ts @@ -6,17 +6,31 @@ process.chdir(dir) import { $ } from "bun" import path from "path" +import os from "os" +import { copyFile, mkdir, mkdtemp, readdir, rm } from "fs/promises" import { createClient } from "@hey-api/openapi-ts" const opencode = path.resolve(dir, "../../opencode") -await $`bun dev generate > ${dir}/openapi.json`.cwd(opencode) +// The generated spec and the raw codegen output are per-run artifacts: +// overlapping builds in one checkout never share their lifecycle, and a run +// removes only its own mkdtemp directory (forced, so an already-gone own +// artifact is not an error). Publishing into the committed src/v2/gen goes +// through a deterministic mirror (copy-over + prune of entries the new +// generation dropped, preserving clean semantics) instead of hey-api's +// tree delete: `clean: true` on the shared path deletes the tree another +// concurrent run is reading. No locking is wanted here. +const openapiTmpDir = await mkdtemp(path.join(os.tmpdir(), "opencode-sdk-openapi-")) +const openapiPath = path.join(openapiTmpDir, "openapi.json") +const genDir = path.join(openapiTmpDir, "gen") + +await $`bun dev generate > ${openapiPath}`.cwd(opencode) await createClient({ - input: "./openapi.json", + input: openapiPath, output: { - path: "./src/v2/gen", + path: genDir, tsConfigPath: path.join(dir, "tsconfig.json"), clean: true, }, @@ -40,6 +54,8 @@ await createClient({ ], }) +await mirrorDir(genDir, path.join(dir, "src/v2/gen")) + // Patch a @hey-api/openapi-ts codegen bug: SseFn incorrectly passes the // endpoint's TError into the second generic of ServerSentEventsResult, which // is the AsyncGenerator's TReturn slot. Iterator return values have nothing @@ -64,4 +80,24 @@ await $`bun prettier --write src/gen` await $`bun prettier --write src/v2` await $`rm -rf dist` await $`bun tsc` -await $`rm openapi.json` +await $`rm -rf ${openapiTmpDir}` + +async function mirrorDir(source: string, target: string) { + await mkdir(target, { recursive: true }) + const entries = await readdir(source, { withFileTypes: true }) + for (const entry of entries) { + const sourcePath = path.join(source, entry.name) + const targetPath = path.join(target, entry.name) + if (entry.isDirectory()) { + await mirrorDir(sourcePath, targetPath) + } else { + await copyFile(sourcePath, targetPath) + } + } + const kept = new Set(entries.map((entry) => entry.name)) + for (const existing of await readdir(target)) { + if (!kept.has(existing)) { + await rm(path.join(target, existing), { recursive: true, force: true }) + } + } +} diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index c74b13d390..10d0a3a905 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -678,6 +678,7 @@ export type DagWorkflowSummary = { id: string title: string status: string + graphRev: number | "NaN" | "Infinity" | "-Infinity" | "Infinity" | "-Infinity" | "NaN" nodeCount: number | "NaN" | "Infinity" | "-Infinity" | "Infinity" | "-Infinity" | "NaN" completedNodes: number | "NaN" | "Infinity" | "-Infinity" | "Infinity" | "-Infinity" | "NaN" runningNodes: number | "NaN" | "Infinity" | "-Infinity" | "Infinity" | "-Infinity" | "NaN" @@ -2952,6 +2953,7 @@ export type DagWorkflowSummary1 = { id: string title: string status: string + graphRev: number | "NaN" | "Infinity" | "-Infinity" nodeCount: number | "NaN" | "Infinity" | "-Infinity" completedNodes: number | "NaN" | "Infinity" | "-Infinity" runningNodes: number | "NaN" | "Infinity" | "-Infinity" diff --git a/packages/tui/src/context/sync.tsx b/packages/tui/src/context/sync.tsx index 4fe07c284c..83b38dd38e 100644 --- a/packages/tui/src/context/sync.tsx +++ b/packages/tui/src/context/sync.tsx @@ -297,7 +297,10 @@ export const { // its visible progress. We just store it β€” no client-side aggregation. case "dag.workflow.summary.updated": if (workspace !== undefined && workspace !== project.workspace.current()) break - setStore("dag", event.properties.sessionID, event.properties.summaries) + // reconcile (like bootstrap/reconnect) so unchanged same-ID rows keep + // store-node identity: a no-op summary must not remount expanded + // sidebar rows and re-trigger their signature-guarded fetches. + setStore("dag", event.properties.sessionID, reconcile(event.properties.summaries)) break case "session.diff": diff --git a/packages/tui/src/feature-plugins/sidebar/dag-panel.tsx b/packages/tui/src/feature-plugins/sidebar/dag-panel.tsx index 52ab53156c..5f6a0e3d3e 100644 --- a/packages/tui/src/feature-plugins/sidebar/dag-panel.tsx +++ b/packages/tui/src/feature-plugins/sidebar/dag-panel.tsx @@ -33,7 +33,10 @@ function WorkflowRow(props: { const failed = () => Number(props.summary.failedNodes) const queued = () => Number(props.summary.queuedNodes) - const signature = () => `${total()}:${completed()}:${running()}:${failed()}:${queued()}` + // graphRev (topology revision) participates so an equal-count replan still + // changes the signature and triggers exactly one authoritative re-fetch. + const signature = () => + `${total()}:${completed()}:${running()}:${failed()}:${queued()}:${props.summary.graphRev}` const fetchNodes = async (dagID: string, sig: string) => { try { @@ -49,8 +52,9 @@ function WorkflowRow(props: { } // Signature-triggered fetch: the signature memo only changes value when a - // node count actually changes, so this effect re-runs (and re-fetches) only - // on real state changes β€” never on a no-op summary event. No polling. + // node count or the topology revision (graphRev) actually changes, so this + // effect re-runs (and re-fetches) only on real state changes β€” never on a + // no-op summary event. No polling. createEffect(() => { const sig = signature() if (!props.expanded) { diff --git a/packages/tui/src/feature-plugins/system/dag-inspector.tsx b/packages/tui/src/feature-plugins/system/dag-inspector.tsx index 5b8c7a9953..bbf3eae290 100644 --- a/packages/tui/src/feature-plugins/system/dag-inspector.tsx +++ b/packages/tui/src/feature-plugins/system/dag-inspector.tsx @@ -274,7 +274,9 @@ function DagInspector(props: { api: TuiPluginApi }) { } // Per-workflow summary signature for change detection. Only re-fetch nodes - // when the selected workflow's node-level state actually changes. + // when the selected workflow's node-level state or topology revision + // (graphRev) changes β€” an equal-count replan bumps graphRev alone, so it + // must participate in the signature for the refresh to fire. let lastSignature = "" const signatureFor = (wfId: string): string => { @@ -284,7 +286,7 @@ function DagInspector(props: { api: TuiPluginApi }) { 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}` + return `${wf.nodeCount}:${wf.completedNodes}:${wf.runningNodes}:${wf.failedNodes}:${wf.graphRev}` } createEffect(() => { diff --git a/packages/tui/test/cli/cmd/tui/sync-dag.test.tsx b/packages/tui/test/cli/cmd/tui/sync-dag.test.tsx index 421409d16c..07341e54cb 100644 --- a/packages/tui/test/cli/cmd/tui/sync-dag.test.tsx +++ b/packages/tui/test/cli/cmd/tui/sync-dag.test.tsx @@ -26,6 +26,7 @@ function summary(completed: number, total: number, running = 0, failed = 0): Dag id: "wf-1", title: "Test workflow", status: "running", + graphRev: 1, nodeCount: total, completedNodes: completed, runningNodes: running, @@ -50,7 +51,7 @@ describe("tui sync dag slice", () => { const stored = sync.data.dag[sid] expect(stored).toHaveLength(1) - expect(stored[0]).toMatchObject({ id: "wf-1", completedNodes: 2, nodeCount: 5, runningNodes: 1 }) + expect(stored[0]).toMatchObject({ id: "wf-1", graphRev: 1, completedNodes: 2, nodeCount: 5, runningNodes: 1 }) } finally { app.renderer.destroy() } 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 121827f6d6..dcdefacc1d 100644 --- a/packages/tui/test/feature-plugins/dag-inspector-utils.test.ts +++ b/packages/tui/test/feature-plugins/dag-inspector-utils.test.ts @@ -204,6 +204,7 @@ describe("mergeDagWorkflowSummaries", () => { skippedNodes: 0, queuedNodes: 0, escalatedNodes: 0, + graphRev: 1, }) test("orders merged rows by the project list and keeps the list as source of truth", () => { diff --git a/packages/tui/test/feature-plugins/dag-inspector.test.tsx b/packages/tui/test/feature-plugins/dag-inspector.test.tsx index c215905b70..149470bf2e 100644 --- a/packages/tui/test/feature-plugins/dag-inspector.test.tsx +++ b/packages/tui/test/feature-plugins/dag-inspector.test.tsx @@ -20,6 +20,7 @@ const wfSummary = (overrides: Partial<DagWorkflowSummary> = {}): DagWorkflowSumm id: "wf-1", title: "Test workflow", status: "running", + graphRev: 1, nodeCount: 2, completedNodes: 0, runningNodes: 0, @@ -77,6 +78,8 @@ async function renderDagInspector(opts: RenderOpts = {}) { // Updatable workflow state for change detection. let workflowsState = opts.workflows ?? [] + // Updatable node state so a replan can serve a fresh node set. + let nodesState = opts.nodes ?? [] // Trackable spies const nodesCalls: string[] = [] @@ -112,7 +115,7 @@ async function renderDagInspector(opts: RenderOpts = {}) { }, nodes: async (input: { dagID: string }) => { nodesCalls.push(input.dagID) - return { data: opts.nodes ?? [] } + return { data: nodesState } }, control: async (input: { dagID: string; operation: string }) => { controlCalls.push(input) @@ -203,6 +206,9 @@ async function renderDagInspector(opts: RenderOpts = {}) { setWorkflows: (wfs: DagWorkflowSummary[]) => { workflowsState = wfs }, + setNodes: (nodes: DagNode[]) => { + nodesState = nodes + }, emitSummaryUpdate: (sessionID: string = SESSION_ID) => { eventHandlers.get("dag.workflow.summary.updated")?.({ type: "dag.workflow.summary.updated", @@ -381,6 +387,28 @@ describe("DagInspector", () => { } }) + test("equal-count replan bumps graphRev alone and refetches exactly once with the replanned node set", async () => { + const viewer = await renderDagInspector({ + workflows: [wfSummary({ id: "wf-1", nodeCount: 2, completedNodes: 0, graphRev: 1 })], + nodes: [dagNode({ id: "n-1", name: "build-old", status: "running" })], + }) + try { + await viewer.app.waitForFrame((frame) => frame.includes("build-old")) + const before = viewer.nodesCalls().length + // Equal-count replan: identical counts/status, only the topology + // revision moved. The server now serves the replanned node set. + viewer.setNodes([dagNode({ id: "n-2", name: "build-new", status: "pending" })]) + viewer.setWorkflows([wfSummary({ id: "wf-1", nodeCount: 2, completedNodes: 0, graphRev: 2 })]) + viewer.emitSummaryUpdate() + await waitForCondition(() => viewer.nodesCalls().length === before + 1) + await Bun.sleep(50) + expect(viewer.nodesCalls().length).toBe(before + 1) + await viewer.app.waitForFrame((frame) => frame.includes("build-new") && !frame.includes("build-old")) + } finally { + viewer.app.renderer.destroy() + } + }) + test("summary for another session does not trigger a re-fetch", async () => { const viewer = await renderDagInspector({ workflows: [wfSummary({ id: "wf-1", completedNodes: 0 })], @@ -400,12 +428,18 @@ describe("DagInspector", () => { test("unchanged summary does not trigger a re-fetch", async () => { const viewer = await renderDagInspector({ - workflows: [wfSummary({ id: "wf-1", completedNodes: 0 })], + workflows: [wfSummary({ id: "wf-1", completedNodes: 0, graphRev: 1 })], nodes: [dagNode({ id: "n-1", name: "build", status: "running" })], }) try { const before = viewer.nodesCalls().length - // Don't change the workflow state β€” signature stays the same. + // Re-emit the exact same aggregates AND the same graphRev β€” a no-op + // summary event (server re-broadcasts identical state). Neither emit + // may refetch nodes. + viewer.setWorkflows([wfSummary({ id: "wf-1", completedNodes: 0, graphRev: 1 })]) + viewer.emitSummaryUpdate() + await Bun.sleep(50) + expect(viewer.nodesCalls().length).toBe(before) viewer.emitSummaryUpdate() await Bun.sleep(50) expect(viewer.nodesCalls().length).toBe(before) diff --git a/packages/tui/test/feature-plugins/dag-panel.test.tsx b/packages/tui/test/feature-plugins/dag-panel.test.tsx new file mode 100644 index 0000000000..43ccc3727b --- /dev/null +++ b/packages/tui/test/feature-plugins/dag-panel.test.tsx @@ -0,0 +1,148 @@ +/** @jsxImportSource @opentui/solid */ +import { describe, expect, test } from "bun:test" +import { testRender } from "@opentui/solid" +import type { JSX } from "solid-js" +import { createStore, reconcile } from "solid-js/store" +import type { TuiPluginApi } from "@opencode-ai/plugin/tui" +import type { DagNode, DagWorkflowSummary } from "@opencode-ai/sdk/v2" +import dagPanelPlugin from "../../src/feature-plugins/sidebar/dag-panel" +import { createTuiPluginApi } from "../fixture/tui-plugin" +import { TestTuiContexts } from "../fixture/tui-environment" + +const SESSION_ID = "ses_panel" + +const wfSummary = (overrides: Partial<DagWorkflowSummary> = {}): DagWorkflowSummary => ({ + id: "wf-1", + title: "Panel workflow", + status: "running", + graphRev: 1, + nodeCount: 2, + completedNodes: 1, + runningNodes: 1, + failedNodes: 0, + skippedNodes: 0, + queuedNodes: 0, + escalatedNodes: 0, + ...overrides, +}) + +function dagNode(overrides: Partial<DagNode> & { id: string }): DagNode { + return { + workflow_id: "wf-1", + name: overrides.id, + status: "pending", + worker_type: "build", + required: false, + depends_on: [], + replan_attempts: 0, + ...overrides, + } +} + +type RenderOpts = { + workflows?: DagWorkflowSummary[] + nodes?: DagNode[] +} + +/** Mirrors the production bridge: the plugin-facing dag(sessionID) accessor + * reads a Solid store slice that summary events replace wholesale. */ +async function renderDagPanel(opts: RenderOpts = {}) { + const nodesCalls: string[] = [] + let nodesState = opts.nodes ?? [] + const [store, setStore] = createStore<{ dag: Record<string, DagWorkflowSummary[]> }>({ + dag: { [SESSION_ID]: opts.workflows ?? [] }, + }) + + const base = createTuiPluginApi({ + client: { + dag: { + nodes: async (input: { dagID: string }) => { + nodesCalls.push(input.dagID) + return { data: nodesState } + }, + }, + } as unknown as TuiPluginApi["client"], + state: { session: { dag: (sessionID: string) => store.dag[sessionID] ?? [] } }, + }) + + let sidebar: ((props: { session_id: string }) => JSX.Element) | undefined + const api = { + ...base, + slots: { + register: (def: { slots: { sidebar_content: (ctx: never, props: { session_id: string }) => JSX.Element } }) => { + sidebar = (props) => def.slots.sidebar_content(undefined as never, props) + }, + }, + } as unknown as TuiPluginApi + + await dagPanelPlugin.tui(api, undefined, undefined as never) + + const app = await testRender(() => <TestTuiContexts>{sidebar?.({ session_id: SESSION_ID })}</TestTuiContexts>, { + width: 80, + height: 24, + }) + // The first active workflow auto-expands; let its initial fetch settle. + await waitForCondition(() => nodesCalls.length > 0) + + return { + app, + nodesCalls: () => nodesCalls, + setNodes: (nodes: DagNode[]) => { + nodesState = nodes + }, + setWorkflows: (wfs: DagWorkflowSummary[]) => setStore("dag", SESSION_ID, reconcile(wfs)), + } +} + +async function waitForCondition(fn: () => boolean, timeout = 2000) { + const start = Date.now() + while (!fn()) { + if (Date.now() - start > timeout) throw new Error("timed out waiting for condition") + await Bun.sleep(10) + } +} + +describe("DagPanel expanded sidebar", () => { + test("equal-count replan bumps graphRev alone and refetches exactly once with the current node set", async () => { + const panel = await renderDagPanel({ + workflows: [wfSummary({ id: "wf-1", graphRev: 1 })], + nodes: [dagNode({ id: "n-1", name: "build-old", status: "pending" })], + }) + try { + await panel.app.waitForFrame((frame) => frame.includes("build-old")) + const before = panel.nodesCalls().length + // Equal-count replan: identical counts/status, only the topology + // revision moved. The server now serves the replanned node set. + panel.setNodes([dagNode({ id: "n-2", name: "build-new", status: "pending" })]) + panel.setWorkflows([wfSummary({ id: "wf-1", graphRev: 2 })]) + await waitForCondition(() => panel.nodesCalls().length === before + 1) + await Bun.sleep(50) + expect(panel.nodesCalls().length).toBe(before + 1) + await panel.app.waitForFrame((frame) => frame.includes("build-new") && !frame.includes("build-old")) + } finally { + panel.app.renderer.destroy() + } + }) + + // R1 regression canary: a no-op summary event must not refetch the expanded + // row. Identity is preserved end-to-end because every summary writer β€” + // bootstrap, reconnect, and the event reducer in context/sync.tsx β€” uses + // reconcile(), which setWorkflows mirrors; unchanged same-ID rows keep their + // store-node identity, so <For> never remounts them. + test("no-op summary replacement (same graphRev and counts) does not refetch the expanded row", async () => { + const panel = await renderDagPanel({ + workflows: [wfSummary({ id: "wf-1", graphRev: 1 })], + nodes: [dagNode({ id: "n-1", name: "build", status: "pending" })], + }) + try { + const before = panel.nodesCalls().length + // A no-op summary event re-broadcasts identical state β€” same aggregates + // AND same graphRev. The signature must not move, so no refetch. + panel.setWorkflows([wfSummary({ id: "wf-1", graphRev: 1 })]) + await Bun.sleep(80) + expect(panel.nodesCalls().length).toBe(before) + } finally { + panel.app.renderer.destroy() + } + }) +})