diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index 9484aac7a..d801a2193 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -26,6 +26,11 @@ on: - main - dev pull_request: + # ready_for_review matters: the SpecGit Acceptance verdict requires + # required-check runs that started at/after the draft→ready transition + # (#315 anchor). Without this type a ready transition finds only stale + # pre-ready runs and the gate times out waiting for fresh ones. + types: [opened, synchronize, reopened, ready_for_review] branches: - main - dev diff --git a/.github/workflows/ci-typecheck.yml b/.github/workflows/ci-typecheck.yml index f8bd2341c..863d90751 100644 --- a/.github/workflows/ci-typecheck.yml +++ b/.github/workflows/ci-typecheck.yml @@ -20,6 +20,11 @@ on: - main - dev pull_request: + # ready_for_review matters: the SpecGit Acceptance verdict requires + # required-check runs that started at/after the draft→ready transition + # (#315 anchor). Without this type a ready transition finds only stale + # pre-ready runs and the gate times out waiting for fresh ones. + types: [opened, synchronize, reopened, ready_for_review] branches: - main - dev diff --git a/.github/workflows/specgit-accept.yml b/.github/workflows/specgit-accept.yml index c280272a4..93d6a6166 100644 --- a/.github/workflows/specgit-accept.yml +++ b/.github/workflows/specgit-accept.yml @@ -2,22 +2,42 @@ name: SpecGit Acceptance on: pull_request: - # Delivery PRs target dev (fast-integration layer); the acceptance - # verdict runs only on the dev→main promotion PR, where protect-main's - # checks apply. Keep the trigger main-only (d6ce53a83): running it on - # dev PRs duplicated the verdict against the lighter dev gate. branches: [main] + # A draft PR fails the verdict (pr_draft), so the draft→ready + # transition must re-verdict. Listing types replaces the defaults, + # so the default activity types are listed alongside. + types: [opened, synchronize, reopened, ready_for_review] + # No workflow_dispatch (local specialization): dispatch is the privileged + # context that fires CodeQL's cache-poisoning taint rule on the head_ref + # checkout (false positive: no cache use, read-only token, + # persist-credentials: false), and on dispatch events head_ref is empty so + # the verdict would evaluate the default branch — the wrong tree. Delivery + # here always goes through a PR. permissions: contents: read + issues: read + pull-requests: read + +# One verdict per head at a time (#319): a newer trigger event (a push +# after the draft opened, then ready_for_review) supersedes the older +# run of the same pull request instead of leaving parallel copies +# burning identical wait budgets. The surviving run re-verdicts fully. +concurrency: + group: specgit-accept-${{ github.ref }} + cancel-in-progress: true jobs: specgit-acceptance: name: SpecGit Acceptance + # Portable gate for any adopting repository: the published CLI is + # installed at the exact version `specgit init` pinned. The adopting + # project's own toolchain (package manager, lockfile, build, layout) + # is never assumed and never invoked. runs-on: ubuntu-latest - # Must exceed the slowest required sibling (Unit Tests (linux) runs - # ~28min on PRs): the verdict waits for every policy check to reach a - # terminal state before evaluating. + # Local specialization: must exceed the slowest required sibling + # (Unit Tests (linux) runs ~28min on PRs) — the verdict waits for every + # policy check to reach a terminal state before evaluating. timeout-minutes: 45 steps: - name: Checkout code @@ -35,13 +55,13 @@ jobs: with: node-version: '22' - # This repo is a bun workspace and does not vendor the SpecGit CLI; - # install the published CLI instead of building from source. Pinned - # with a caret floor (#366): the CLI releases multiple times a day and - # an unpinned install would let an unnoticed upstream change flip CI - # acceptance verdicts repo-wide. - - name: Install specgit CLI - run: npm install -g specgit@^0.5.0 + - name: Install pinned SpecGit CLI + # Local specialization — install GLOBALLY, not `npm install --no-save + # specgit@X`: a workspace-local install reads this bun workspace's + # package.json and dies on the `catalog:` protocol (EUNSUPPORTEDPROTOCOL, + # #434/#459). Exact pin on purpose: the gate must evaluate with the + # same CLI generation that wrote the binding (1.10.1 re-init). + run: npm install -g --no-audit --no-fund specgit@1.10.1 - name: Wait for sibling checks # The verdict must see the OTHER required checks in a terminal @@ -49,50 +69,240 @@ jobs: # their check-runs yet, so an empty poll is not "done": wait until # every name in spec_git/policy.yaml is present with a terminal # conclusion. This job is not in the policy, so no self-deadlock. + # #315: a terminal run only counts when it started at/after the + # delivery's ready-for-review transition — a stale green keeps + # waiting for the fresh run the transition triggers. + # All GitHub access goes through the authenticated gh CLI. env: GH_TOKEN: ${{ github.token }} WAIT_REPO: ${{ github.repository }} - WAIT_SHA: ${{ github.event.pull_request.head.sha }} + WAIT_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + WAIT_PR: ${{ github.event.pull_request.number || '' }} run: | node --input-type=module <<'EOF' - import { readFileSync } from 'node:fs'; - // Minimal parse of policy.yaml's required_checks block list — - // avoids a yaml dependency in this bun-based repo. + import { existsSync, readFileSync } from 'node:fs'; + import { execFileSync } from 'node:child_process'; + if (!existsSync('spec_git/policy.yaml')) { + console.error('spec_git/policy.yaml is absent at this head — an adoption PR carries no binding commit yet (expected once; merge it before enabling branch protection), and a delivery PR must carry it via specgit issue.'); + process.exit(1); + } + // Local specialization — minimal hand parse of policy.yaml's + // required_checks block: this bun-based repo does not expose a + // root-reachable `yaml` package (workspace catalog isolation), so + // `import { parse } from 'yaml'` would fail to resolve here. const policy = readFileSync('spec_git/policy.yaml', 'utf8'); const section = policy.slice(policy.indexOf('required_checks:')); const required = [...section.matchAll(/^\s*-\s*(.+)$/gm)].map((m) => m[1].trim()); - const headers = { - authorization: 'Bearer ' + process.env.GH_TOKEN, - accept: 'application/vnd.github+json', + // gh.cmd needs a shell on Windows; POSIX execs the binary + + // directly (shell stays off where it is not needed). The + + // query rides --field args (never a raw "?" URL): cmd.exe + + // treats a bare "&" as a command separator, so a URL query + + // would be split mid-parameter on Windows. + + const listChecks = (page) => + JSON.parse( + execFileSync( + 'gh', + [ + 'api', + 'repos/' + process.env.WAIT_REPO + '/commits/' + process.env.WAIT_SHA + '/check-runs', + '--method', 'GET', + '--field', 'per_page=' + PER_PAGE, + '--field', 'page=' + page, + ], + { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'pipe'], shell: process.platform === 'win32' } + ) + ); + // Transient API failures (5xx, 429, network) retry with bounded + + // exponential backoff — a platform blip must not fail the gate. + + const MAX_ATTEMPTS = 5; + const listChecksWithRetry = async (page) => { + for (let attempt = 1; ; attempt += 1) { + try { + return listChecks(page); + } catch (error) { + const text = String(error) + ' ' + String(error && error.stderr ? error.stderr : ''); + const transient = /HTTP 5\d\d|HTTP 429|ETIMEDOUT|ECONNRESET|ENOTFOUND|timed out/i.test(text); + if (attempt >= MAX_ATTEMPTS || !transient) throw error; + const backoff = Math.min(30000, 2000 * 2 ** (attempt - 1)); + console.log('Transient failure; retry ' + attempt + '/' + MAX_ATTEMPTS + ' in ' + backoff + 'ms'); + await new Promise((r) => setTimeout(r, backoff)); + } + } + }; + // #315: the ready-for-review anchor rides the issue-timeline + // endpoint through gh api --field args (GET, like the listing). + const fetchTimelinePage = (page) => + JSON.parse( + execFileSync( + 'gh', + [ + 'api', + 'repos/' + process.env.WAIT_REPO + '/issues/' + process.env.WAIT_PR + '/timeline', + '--method', 'GET', + '--field', 'per_page=' + PER_PAGE, + '--field', 'page=' + page, + ], + { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'pipe'], shell: process.platform === 'win32' } + ) + ); + const fetchTimelineWithRetry = async (page) => { + for (let attempt = 1; ; attempt += 1) { + try { + const payload = fetchTimelinePage(page); + if (!Array.isArray(payload)) throw new Error('GitHub returned a non-array timeline payload.'); + return payload; + } catch (error) { + if (error && error.message === 'GitHub returned a non-array timeline payload.') throw error; + const text = String(error) + ' ' + String(error && error.stderr ? error.stderr : ''); + const transient = /HTTP 5\d\d|HTTP 429|ETIMEDOUT|ECONNRESET|ENOTFOUND|timed out/i.test(text); + if (attempt >= MAX_ATTEMPTS || !transient) throw error; + const backoff = Math.min(30000, 2000 * 2 ** (attempt - 1)); + console.log('Transient failure; retry ' + attempt + '/' + MAX_ATTEMPTS + ' in ' + backoff + 'ms'); + await new Promise((r) => setTimeout(r, backoff)); + } + } }; - const url = 'https://api.github.com/repos/' + process.env.WAIT_REPO - + '/commits/' + process.env.WAIT_SHA + '/check-runs?per_page=100'; const terminal = new Set(['completed']); - const terminalHas = (byName, name) => { - if (byName.has(name)) return terminal.has(byName.get(name)); - const retried = [...byName.keys()].find((k) => k.startsWith(name + ' (')); - return retried !== undefined && terminal.has(byName.get(retried)); + const PER_PAGE = 100; + // #300: page the listing to exhaustion — a head with more than + + // PER_PAGE check-runs must still expose every required name. + + const fetchAllCheckRuns = async () => { + const runs = []; + for (let page = 1; ; page += 1) { + const payload = await listChecksWithRetry(page); + runs.push(...(payload.check_runs ?? [])); + if (!payload.check_runs || payload.check_runs.length < PER_PAGE) break; + } + return runs; + }; + // #315: the evidence anchor — created_at of the latest + + // ready_for_review event on the pull request's issue timeline, + + // paged to exhaustion through the same transport seam. Empty + + // WAIT_PR (a push or workflow_dispatch event) means no anchor + + // and no freshness bound; a fetch failure fails the step + + // loudly instead of silently unbounding freshness. + + const fetchAnchor = async () => { + if (!process.env.WAIT_PR) return null; + let anchor = null; + let anchorTime = null; + for (let page = 1; ; page += 1) { + const events = await fetchTimelineWithRetry(page); + if (!Array.isArray(events)) throw new Error('GitHub returned a non-array timeline payload.'); + for (const event of events) { + if (event && event.event === 'ready_for_review') { + if (typeof event.created_at !== 'string' || event.created_at === '' + || Number.isNaN(Date.parse(event.created_at))) { + throw new Error('GitHub returned a ready-for-review event without a valid timestamp.'); + } + const eventTime = Date.parse(event.created_at); + if (anchor === null || anchorTime === null || eventTime > anchorTime) { + anchor = event.created_at; + anchorTime = eventTime; + } + } + } + if (!Array.isArray(events) || events.length < PER_PAGE) return anchor; + } }; - // Must outlast the slowest required sibling (Unit Tests (linux) - // runs ~28min on PRs); the job timeout above bounds this too. + // Poll deadline sits BELOW the job's timeout-minutes (45) on + + // purpose: when the deadline loses the race against a slow + + // sibling, the script exits with its own diagnosis instead of + + // being killed by the job timeout mid-line. + + // Local specialization: 40min because the slowest required + + // sibling (Unit Tests (linux)) runs ~28min on PRs. + const deadline = Date.now() + 40 * 60 * 1000; while (Date.now() < deadline) { - const res = await fetch(url, { headers }); - if (!res.ok) throw new Error('check-runs API ' + res.status); - const payload = await res.json(); - const byName = new Map(payload.check_runs.map((r) => [r.name, r.status])); - const missing = required.filter((n) => !terminalHas(byName, n)); - if (missing.length === 0) { + // #315: re-read the anchor every cycle — the transition + + // event landing after this job started, or the fresh runs + + // registering late, self-heal on the next poll. + + let anchor; + try { + anchor = await fetchAnchor(); + } catch (error) { + console.error('Could not read the ready-for-review anchor: ' + + (error && error.message ? error.message : String(error))); + process.exit(1); + } + const runs = await fetchAllCheckRuns(); + // #119: re-runs keep every same-name run; terminality is + // decided on the truth run — latest started_at, ties broken + // by the higher check-run id (docs/reference.md) — never on + // response position. + const truth = new Map(); + const startedTime = (run) => { + if (typeof run.started_at !== 'string') return Number.NEGATIVE_INFINITY; + const parsed = Date.parse(run.started_at); + return Number.isNaN(parsed) ? Number.NEGATIVE_INFINITY : parsed; + }; + for (const r of runs) { + const cur = truth.get(r.name); + const runTime = startedTime(r); + const currentTime = cur === undefined ? Number.NEGATIVE_INFINITY : startedTime(cur); + const later = cur === undefined + || runTime > currentTime + || (runTime === currentTime && (r.id || 0) > (cur.id || 0)); + if (later) truth.set(r.name, r); + } + const truthRunFor = (name) => { + if (truth.has(name)) return truth.get(name); + const retried = [...truth.keys()].find((k) => k.startsWith(name + ' (')); + return retried === undefined ? undefined : truth.get(retried); + }; + // #315: a required check settles only when its truth run is + // terminal AND (when an anchor exists) started at/after the + // ready-for-review transition — a stale green keeps waiting. + const missing = []; + const stale = []; + const anchorTime = anchor === null ? null : Date.parse(anchor); + for (const name of required) { + const run = truthRunFor(name); + if (run === undefined || !terminal.has(run.status)) { + missing.push(name); + } else if (anchorTime !== null && (Number.isNaN(anchorTime) || startedTime(run) < anchorTime)) { + stale.push(name); + } + } + if (missing.length === 0 && stale.length === 0) { console.log('All required checks are in a terminal state.'); process.exit(0); } - console.log('Waiting for: ' + missing.join(', ')); + if (missing.length > 0) { + console.log('Waiting for: ' + missing.join(', ')); + } + if (stale.length > 0) { + console.log('Waiting for a fresh run after ready for review: ' + stale.join(', ')); + } await new Promise((r) => setTimeout(r, 10000)); } console.error('Timed out waiting for sibling checks.'); process.exit(1); EOF + - name: specgit finish run: specgit finish --json env: diff --git a/.specgit.yaml b/.specgit.yaml index bffc28295..8f391bf94 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,9 +1,17 @@ version: 1 -delivery: correct-v1-0 +delivery: accept-builtin-spec context: kind: branch - branch: docs/493-correct-v1-0 + branch: fix/506-accept-builtin-spec issues: - - 493 - - 494 -pr: 495 + - 506 + - 507 + - 508 +issueKinds: + - issue: 506 + kind: kind::fix + - issue: 507 + kind: kind::fix + - issue: 508 + kind: kind::docs +pr: 509 diff --git a/AGENTS.md b/AGENTS.md index 561824a59..3a6020512 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,7 @@ feat/**, fix/** ──PR(Typecheck + Unit Tests 门禁)──▶ dev ──push **CI 配置**: - `ci-typecheck.yml`:push 到 `main`/`dev` + PR → `main`/`dev` 时触发;除 lint + typecheck 外还跑 `test:dag-core` DAG 核心行为/覆盖率门禁(10min 超时) - `ci-test.yml`:push 到 `main`/`dev` + PR → `main`/`dev` 时触发全量测试(`cancel-in-progress: false` 保证跑完);Linux unit-tests job 额外校验生成物新鲜度(`packages/client` 与 `packages/sdk/js` 的 `check:generated`)并跑 HttpAPI 契约门禁(`test:httpapi:ci`) -- `specgit-accept.yml`:仅 PR → `main` 时触发;全局安装 `specgit@^0.5.0`(`npm install -g`;workspace 内安装会因 bun `catalog:` 协议失败),等 `spec_git/policy.yaml` `required_checks` 全部到终态后运行 `specgit finish --json` 产出 SpecGit Acceptance 裁决 +- `specgit-accept.yml`:仅 PR → `main` 时触发;全局安装 `specgit@1.10.1`(`npm install -g`;workspace 内安装会因 bun `catalog:` 协议失败),等 `spec_git/policy.yaml` `required_checks` 全部到终态(等待脚本手工解析 YAML,不依赖 `yaml` 包)后运行 `specgit finish --json` 产出 SpecGit Acceptance 裁决 - `release-fork.yml`:手动 `workflow_dispatch` 是唯一真实构建路径(push 到 `main`/`dev` 仅注册不构建);从 `dev` 发布自动产出 `X.Y.Z-dev.N` prerelease,从 `main` 发布 `X.Y.Z` 并标 Latest ## Standard Delivery Workflow (标准交付流程) @@ -267,7 +267,10 @@ This repository uses a multi-context domain-document layout rooted at `CONTEXT-M Kept OUTSIDE the managed block so `specgit init`/`--force` never rewrites them; re-apply each deviation after every re-init: - `specgit-accept.yml` drops the template's `workflow_dispatch` trigger. Dispatch is the privileged context that fires CodeQL's cache-poisoning taint rule on the `head_ref` checkout (false positive: no cache use, read-only token, `persist-credentials: false`), and on dispatch events `head_ref` is empty so the verdict would evaluate the default branch — the wrong tree. Delivery here always goes through a PR. The head-ref checkout itself must NOT be replaced with a SHA: `specgit finish` requires HEAD on the delivery branch (detached_head otherwise). -- `specgit-accept.yml` installs the CLI globally (`npm install -g specgit@^0.5.0`), NOT the template's `npm install --no-save specgit@X`: a workspace-local install reads this bun workspace's package.json and dies on the `catalog:` protocol (EUNSUPPORTEDPROTOCOL, #434, again on #459). Re-apply after every `specgit init --force`. +- `specgit-accept.yml` installs the CLI globally (`npm install -g specgit@1.10.1`), NOT the template's `npm install --no-save specgit@X`: a workspace-local install reads this bun workspace's package.json and dies on the `catalog:` protocol (EUNSUPPORTEDPROTOCOL, #434, again on #459). Re-apply after every `specgit init --force`. +- `specgit-accept.yml` runs the job with `timeout-minutes: 45` while the wait script's own poll deadline is 40 minutes (`Date.now() + 40 * 60 * 1000`): the deadline must stay BELOW the job timeout so a lost race against a slow sibling check exits with its own diagnosis instead of being killed mid-line by the job timeout. +- `specgit-accept.yml` pins `node-version: '22'` for the wait script and the CLI. +- The wait script hand-parses `spec_git/policy.yaml` (minimal line-based parse) instead of importing the `yaml` package: no root-reachable `yaml` exists under workspace catalog isolation, so `import { parse } from 'yaml'` would fail to resolve on the runner. - `spec_git/policy.yaml` `required_checks` uses the template's canonical check IDs (`unit-tests`, `e2e-tests`), not display names. diff --git a/packages/core/src/plugin/command/orchestration-policy.md b/packages/core/src/plugin/command/orchestration-policy.md index bf3a6a23f..4c04a1a67 100644 --- a/packages/core/src/plugin/command/orchestration-policy.md +++ b/packages/core/src/plugin/command/orchestration-policy.md @@ -333,6 +333,6 @@ A replan fragment takes real time to compose — template rendering, model reaso 1. On any user cancel/replan/model-change intent, IMMEDIATELY issue `control(pause)` in the same turn. Pause needs no fragment, applies in milliseconds, and stops new node spawns. 2. Pause does not interrupt nodes that are already running. Decide their disposition inside the fragment: `restart: true` re-spawns a running node with the new definition (its in-flight child session is hard-aborted at re-spawn), `cancel: true` terminates it, absence keeps it running to completion. 3. Compose the fragment, then issue `control(replan)` — replan is valid while paused. -4. Issue `control(resume)` to restore scheduling. +4. A successful replan auto-resumes the workflow. Issue `control(resume)` manually only when the replan output reports the automatic resume raced with another control op and the workflow is still paused; never resume a workflow the output says was already resumed. If the workflow terminalized before you paused, do not force the replan: start a new workflow carrying the updated definitions, and state which prior results are superseded. diff --git a/packages/core/src/plugin/command/workflow.md b/packages/core/src/plugin/command/workflow.md index ddb6ef955..f0602ea92 100644 --- a/packages/core/src/plugin/command/workflow.md +++ b/packages/core/src/plugin/command/workflow.md @@ -64,8 +64,11 @@ config: ## Saved workflows -A `spec_path` with no path separator and no `.yaml`/`.yml` extension is a -**name** resolved against the workflow library instead of the filesystem: +A `spec_path` with no path separator, no leading `.`, no control characters, +and no `.yaml`/`.yml` extension is a **name** resolved against the workflow +library instead of the filesystem (a leading dot or a recognized extension +means a filesystem path). The synthetic `builtin://` marker the `list` +output shows for builtin templates also resolves by name: 1. `.opencode/workflows/.yaml` — project scope, committed with the repo 2. `/workflows/.yaml` — global scope, available in every project @@ -340,7 +343,7 @@ Workflows are not static. After creating a workflow, use `extend` and `control(r - **Scale up**: a node reports the work is larger than expected → `extend` with additional parallel nodes to split the load. - **Cut short**: a node proves the remaining work is unnecessary → `control(complete)` to early-complete and skip pending nodes. -- **Redirect**: a gate or review reveals a wrong direction → `control(pause)` first to freeze scheduling, then `control(replan)` with `restart: true` on the affected nodes and `cancel: true` on their downstream dependents, then `control(resume)`. +- **Redirect**: a gate or review reveals a wrong direction → `control(pause)` first to freeze scheduling, then `control(replan)` with `restart: true` on the affected nodes and `cancel: true` on their downstream dependents. A successful replan auto-resumes a paused workflow; issue `control(resume)` manually only when the replan output reports the automatic resume raced with another control op. Only nodes with `report_to_parent: true` produce intermediate parent checkpoints, and those reports are delivered at the next actionable wake @@ -527,6 +530,24 @@ validation status. This lists reusable specs, not running workflows; use Pass `spec_path`, then retarget generic objectives and block instructions in the parent, write the edited result to YAML, and start that file by path. +**draft** — Render a structured graph `config` (same fields as the start +file's `config`) into a validated YAML spec and return its `spec_path`; no +workflow is created. Preferred over hand-writing YAML: the parameter schema +rejects unknown fields, eliminating serialization drift. When validation fails +the file is still on disk — fix it by calling draft again with corrected +fields, then start the returned `spec_path`. + +**guide** — Load on-demand authoring guidance. `topic` is one of `blocks` +(composable block schema), `interface` (low-level node fields), `policy` +(gates, admission, recovery), or `patterns` (cross-domain playbooks); omit it +for the compact index. Load only the topic needed for the current decision. + +**validate** — Pre-flight one spec without creating a workflow. Pass +`spec_path` and an optional `profile` (`portable` for distributable-template +checks, `environment` to additionally resolve prompts, workers, and models in +this project; builtin specs default to portable, everything else to +environment). Returns diagnostics with per-error paths, never a workflow ID. + **extend** — Add nodes to a running workflow. Existing nodes are unaffected; new nodes are immediately eligible for scheduling if their dependencies are met. It also accepts a genuinely additive wave after a reporting leaf @@ -547,9 +568,9 @@ omitted content from its preview. **control** — Control a running workflow: - `pause` — let running nodes finish, don't spawn new ones (pause does NOT stop nodes that are already running). On a cancel/replan intent, always pause FIRST: it needs no fragment and freezes scheduling while you compose the replan, so the graph cannot terminalize under you. -- `resume` — resume scheduling +- `resume` — resume scheduling. Unneeded after a successful replan or extend: both auto-resume a paused workflow; resume manually only when their output reports the automatic resume raced with another control op and the workflow is still paused. - `cancel` — cancel the entire workflow -- `replan` — put `fragment: { ... }` with the graph fields and node definitions in YAML and pass its `spec_path`; running nodes can be `restart: true` or `cancel: true`; pending nodes absent from the fragment are cancelled. Valid while paused — the pause → write file → replan → resume sequence is the safe path. +- `replan` — put `fragment: { ... }` with the graph fields and node definitions in YAML and pass its `spec_path`; running nodes can be `restart: true` or `cancel: true`; pending nodes absent from the fragment are cancelled. Valid while paused — the pause → write file → replan sequence is the safe path: a successful replan auto-resumes the workflow, an explicit resume belongs only in the rare case the replan output reports the automatic resume raced with another control op and the workflow is still paused. - `complete` — early-complete: remaining pending nodes are skipped (non-violation) - `step` — advance exactly one ready node (the first by node ID lexicographic order), then wait. Use for controlled debugging or staged verification of a critical path. Unlike `pause`, which freezes all scheduling, `step` advances one node and re-waits. A second `step` while the stepped node is still running is rejected. Use `resume` to return to full-speed scheduling. Nodes are selected in lexicographic ID order for determinism. @@ -566,10 +587,11 @@ omitted content from its preview. | `condition` | no | Expression evaluated before spawn; node is skipped if false | | `input_mapping` | no | Map upstream node outputs into template variables | | `report_to_parent` | no | If true, the parent agent is woken when this node completes or fails. The workflow's terminal status always wakes the parent regardless of this flag | -| `worker_config` | no | `{ timeout_ms }` — bounds node execution (defaults to 10 minutes if omitted) | +| `worker_config` | no | `{ timeout_ms }` — bounds the node from admission to completion (defaults to 10 minutes if omitted); queue wait counts toward the budget, an expired queued node fails without spawning, and a running node that exceeds it escalates to the parent for adjudication (capped deadline extensions) before failing | | `output_schema` | no | JSON Schema; when declared, the child agent must call `submit_result` to submit structured output — failure to submit results in node failure | | `restart` | no | (replan only) Re-spawn this running node with new prompt | | `cancel` | no | (replan only) Cancel this node | +| `review` | no | (deep review workers) `{ phase: "design" \| "diff" }`; a `diff` review must also declare `implementation_node_id` / `verification_node_id` and wire them: transitive review→verification→implementation dependencies, `input_mapping` for the diff artifact + fingerprint + verification output, a PASS-gated `condition`, and a `verdict`+`implementation_fingerprint` `output_schema`. Authoring rejects violations in deep mode and warns in standard | ### What NOT to expect diff --git a/packages/opencode/src/dag/blocks.ts b/packages/opencode/src/dag/blocks.ts index aa756a1be..859dd1347 100644 --- a/packages/opencode/src/dag/blocks.ts +++ b/packages/opencode/src/dag/blocks.ts @@ -36,7 +36,8 @@ export class WorkflowBlock extends Schema.Class("WorkflowBlock")( timeout_ms: Schema.Number, }), ).annotate({ - description: "{ timeout_ms } — bounds node execution; overrides config.node_defaults.worker_config", + description: + "{ timeout_ms } — bounds the node from admission to completion; queue wait counts toward the budget and an expired queued node fails without spawning. A running node that exceeds it escalates to the parent for adjudication (capped deadline extensions) before failing. Overrides config.node_defaults.worker_config", }), required: Schema.optional(Schema.Boolean).annotate({ description: diff --git a/packages/opencode/src/dag/validation.ts b/packages/opencode/src/dag/validation.ts index 46e988903..195027ee1 100644 --- a/packages/opencode/src/dag/validation.ts +++ b/packages/opencode/src/dag/validation.ts @@ -202,7 +202,10 @@ export const NodeSchema = Schema.Struct({ Schema.Struct({ timeout_ms: Schema.optional(Schema.Number), }), - ).annotate({ description: "{ timeout_ms } — bounds node execution. Inherits config.node_defaults.worker_config" }), + ).annotate({ + description: + "{ timeout_ms } — bounds the node from admission to completion; queue wait counts toward the budget and an expired queued node fails without spawning. A running node that exceeds it escalates to the parent for adjudication (capped deadline extensions) before failing. Inherits config.node_defaults.worker_config", + }), input_mapping: Schema.optional(Schema.Record(Schema.String, Schema.String)).annotate({ description: 'Optional variable-to-source map, e.g. { resultA: "node-a", count: "node-b.output.count" }. Omit to expose each direct dependency under its node ID', @@ -230,7 +233,7 @@ export const NodeSchema = Schema.Struct({ }), ).annotate({ description: - "(deep review workers) design reviews pre-implementation artifacts; diff reviews require implementation_node_id and verification_node_id", + "(deep review workers) design reviews pre-implementation artifacts; diff reviews require implementation_node_id, verification_node_id, and authoring-validated wiring: transitive review→verification→implementation dependencies, input_mapping for the diff artifact, fingerprint, and verification output, a PASS-gated condition, and a verdict+implementation_fingerprint output_schema; deep mode additionally requires the diff review to feed a required final gate conditioned on verdict ACCEPT. Violations are authoring errors in deep mode, warnings in standard mode.", }), }) diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index aa7b437f2..a2f40d7f0 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -77,6 +77,10 @@ type AgentCatalogOptions = { heading: string includeHidden: boolean includeModelState: boolean + /** Workflow worker_type defaults compile to the native build/plan primary + * agents, so the workflow catalog lists native primaries even though the + * task subagent catalog and user-defined primary modes stay filtered out. */ + includePrimary?: boolean } export interface Interface { @@ -278,7 +282,7 @@ export const layer = Layer.effect( options: AgentCatalogOptions, ) { const description = (yield* agents.list()) - .filter((item) => item.mode !== "primary") + .filter((item) => item.mode !== "primary" || (options.includePrimary && item.native)) .filter((item) => options.includeHidden || !item.hidden) .filter((item) => Permission.evaluate("task", item.name, caller.permission).action !== "deny") .toSorted((a, b) => a.name.localeCompare(b.name)) @@ -333,6 +337,7 @@ export const layer = Layer.effect( heading: "Available workflow worker_type values:", includeHidden: false, includeModelState: true, + includePrimary: true, }) : undefined, ] diff --git a/packages/opencode/src/tool/workflow.ts b/packages/opencode/src/tool/workflow.ts index 0204a1b43..e33a6cede 100644 --- a/packages/opencode/src/tool/workflow.ts +++ b/packages/opencode/src/tool/workflow.ts @@ -57,7 +57,7 @@ export { Parameters as WorkflowParameters } // ============================================================================ const specPathDescription = - '(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. Graph content belongs in that file; relative paths resolve from the session directory' + '(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. The `builtin://` path marker list shows for builtin templates also resolves, by name. Graph content belongs in that file; relative paths resolve from the session directory' const StartPath = Schema.Struct({ action: Schema.Literal("start").annotate({ description: "Create a workflow" }), @@ -913,19 +913,19 @@ function searchedScopes(directory: string) { function resolveSpecPath(specPath: string, directory: string, ctx: Tool.Context) { return Effect.gen(function* () { - // A bare name addresses the workflow library. Its project/global scopes - // are curated assets the user placed under `.opencode/` or the config - // dir — the same trust level as dag.jsonc — so a resolved name needs no - // external-directory prompt even when the global scope lands outside the - // session directory. Arbitrary paths below keep the prompt. - if (DagWorkflows.isName(specPath)) { - const entry = yield* DagWorkflows.resolve(specPath, directory) + // A bare name addresses the workflow library. The synthetic + // `builtin://name` marker list output advertises must round-trip the same + // way: strip the scheme and resolve by name instead of letting the path + // branch reject it with a cwd-joined extension error. Its project/global + // scopes are curated assets the user placed under `.opencode/` or the + // config dir — the same trust level as dag.jsonc — so a resolved name + // needs no external-directory prompt even when the global scope lands + // outside the session directory. Arbitrary paths below keep the prompt. + const name = DagWorkflows.isBuiltinPath(specPath) ? DagWorkflows.builtinName(specPath) : specPath + if (DagWorkflows.isName(name)) { + const entry = yield* DagWorkflows.resolve(name, directory) if (entry) return entry.path - return yield* Effect.fail( - new Error( - `Saved workflow not found: "${specPath}". Searched ${searchedScopes(directory)}. Run workflow(action: "list") to see what is available, or pass a path to a .yaml spec file.`, - ), - ) + return yield* Effect.fail(savedWorkflowNotFound(name, directory)) } const filepath = path.isAbsolute(specPath) ? path.normalize(specPath) : path.resolve(directory, specPath) if (![".yaml", ".yml"].includes(path.extname(filepath).toLowerCase())) { @@ -940,6 +940,12 @@ function resolveSpecPath(specPath: string, directory: string, ctx: Tool.Context) }) } +function savedWorkflowNotFound(name: string, directory: string) { + return new Error( + `Saved workflow not found: "${name}". Searched ${searchedScopes(directory)}. Run workflow(action: "list") to see what is available, or pass a path to a .yaml spec file.`, + ) +} + /** Terminal-workflow rejections surface as defects carrying recovery * guidance, not bare iron-law errors. Shared by the replan and extend paths. */ function withTerminalRecovery(effect: Effect.Effect, guidance: string) { diff --git a/packages/opencode/test/dag/workflow-tool.test.ts b/packages/opencode/test/dag/workflow-tool.test.ts index 5c564f178..aed9d638a 100644 --- a/packages/opencode/test/dag/workflow-tool.test.ts +++ b/packages/opencode/test/dag/workflow-tool.test.ts @@ -2560,6 +2560,52 @@ describe("workflow tool saved workflows", () => { ), ) + runtime.effect("builtin:// spec_path from list output round-trips by name", () => + withGlobalConfigDir((globalDir) => + Effect.gen(function* () { + const routeSpec = (name: string) => + `title: ${name} title\nconfig:\n name: ${name}\n objective: Route objective\n blocks:\n - id: plan\n kind: plan\n` + yield* Effect.promise(() => + Bun.write(path.join(globalDir, "workflows", "marker-route.yaml"), routeSpec("global-route")), + ) + const previousBuiltin = (globalThis as Record).OPENCODE_DAG_TEMPLATES + ;(globalThis as Record).OPENCODE_DAG_TEMPLATES = { + "marker-builtin": routeSpec("marker-builtin-route"), + } + try { + const info = yield* WorkflowTool + const workflow = yield* info.init() + + // the synthetic builtin:// marker resolves by name through the library + const builtinRead = yield* workflow.execute( + { params: { action: "read", spec_path: "builtin://marker-builtin" }}, + contextWith([]), + ) + expect(JSON.parse(builtinRead.output).spec.title).toContain("marker-builtin-route") + + // the marker resolves through the same shadowing chain as a bare name + const shadowed = yield* workflow.execute( + { params: { action: "validate", spec_path: "builtin://marker-route" }}, + contextWith([]), + ) + expect(JSON.parse(shadowed.output).source).toBe(path.join(globalDir, "workflows", "marker-route.yaml")) + + // unknown builtin names fail as a library lookup, not a path extension error + const missingExit = yield* Effect.exit( + workflow.execute({ params: { action: "read", spec_path: "builtin://missing-route" }}, contextWith([])), + ) + expect(Exit.isFailure(missingExit)).toBe(true) + if (Exit.isFailure(missingExit)) { + expect(Cause.pretty(missingExit.cause)).toContain('Saved workflow not found: "missing-route"') + } + } finally { + if (previousBuiltin === undefined) delete (globalThis as Record).OPENCODE_DAG_TEMPLATES + else (globalThis as Record).OPENCODE_DAG_TEMPLATES = previousBuiltin + } + }), + ), + ) + runtime.effect("list marks invalid templates without hiding them", () => withGlobalConfigDir((globalDir) => Effect.gen(function* () { diff --git a/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap b/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap index a01a903e3..9dbf9d6ff 100644 --- a/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap +++ b/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap @@ -464,7 +464,7 @@ exports[`tool parameters JSON Schema (wire shape) workflow 1`] = ` "type": "string", }, "spec_path": { - "description": "(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. Graph content belongs in that file; relative paths resolve from the session directory", + "description": "(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. The \`builtin://\` path marker list shows for builtin templates also resolves, by name. Graph content belongs in that file; relative paths resolve from the session directory", "type": "string", }, }, @@ -484,7 +484,7 @@ exports[`tool parameters JSON Schema (wire shape) workflow 1`] = ` "type": "string", }, "spec_path": { - "description": "(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. Graph content belongs in that file; relative paths resolve from the session directory", + "description": "(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. The \`builtin://\` path marker list shows for builtin templates also resolves, by name. Graph content belongs in that file; relative paths resolve from the session directory", "type": "string", }, "workflow_id": { @@ -517,7 +517,7 @@ exports[`tool parameters JSON Schema (wire shape) workflow 1`] = ` "type": "string", }, "spec_path": { - "description": "(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. Graph content belongs in that file; relative paths resolve from the session directory", + "description": "(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. The \`builtin://\` path marker list shows for builtin templates also resolves, by name. Graph content belongs in that file; relative paths resolve from the session directory", "type": "string", }, "workflow_id": { @@ -649,7 +649,7 @@ exports[`tool parameters JSON Schema (wire shape) workflow 1`] = ` "type": "string", }, "spec_path": { - "description": "(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. Graph content belongs in that file; relative paths resolve from the session directory", + "description": "(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. The \`builtin://\` path marker list shows for builtin templates also resolves, by name. Graph content belongs in that file; relative paths resolve from the session directory", "type": "string", }, }, @@ -739,7 +739,7 @@ exports[`tool parameters JSON Schema (wire shape) workflow 1`] = ` "type": "boolean", }, "worker_config": { - "description": "{ timeout_ms } — bounds node execution; overrides config.node_defaults.worker_config", + "description": "{ timeout_ms } — bounds the node from admission to completion; queue wait counts toward the budget and an expired queued node fails without spawning. A running node that exceeds it escalates to the parent for adjudication (capped deadline extensions) before failing. Overrides config.node_defaults.worker_config", "properties": { "timeout_ms": { "type": "number", @@ -935,7 +935,7 @@ exports[`tool parameters JSON Schema (wire shape) workflow 1`] = ` "type": "boolean", }, "review": { - "description": "(deep review workers) design reviews pre-implementation artifacts; diff reviews require implementation_node_id and verification_node_id", + "description": "(deep review workers) design reviews pre-implementation artifacts; diff reviews require implementation_node_id, verification_node_id, and authoring-validated wiring: transitive review→verification→implementation dependencies, input_mapping for the diff artifact, fingerprint, and verification output, a PASS-gated condition, and a verdict+implementation_fingerprint output_schema; deep mode additionally requires the diff review to feed a required final gate conditioned on verdict ACCEPT. Violations are authoring errors in deep mode, warnings in standard mode.", "properties": { "implementation_node_id": { "type": "string", @@ -957,7 +957,7 @@ exports[`tool parameters JSON Schema (wire shape) workflow 1`] = ` "type": "object", }, "worker_config": { - "description": "{ timeout_ms } — bounds node execution. Inherits config.node_defaults.worker_config", + "description": "{ timeout_ms } — bounds the node from admission to completion; queue wait counts toward the budget and an expired queued node fails without spawning. A running node that exceeds it escalates to the parent for adjudication (capped deadline extensions) before failing. Inherits config.node_defaults.worker_config", "properties": { "timeout_ms": { "type": "number", @@ -1020,7 +1020,7 @@ exports[`tool parameters JSON Schema (wire shape) workflow 1`] = ` "type": "string", }, "spec_path": { - "description": "(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. Graph content belongs in that file; relative paths resolve from the session directory", + "description": "(start/extend/control replan/read/validate) An exact saved workflow name returned by workflow(action="list"), or a path to a YAML workflow spec. The \`builtin://\` path marker list shows for builtin templates also resolves, by name. Graph content belongs in that file; relative paths resolve from the session directory", "type": "string", }, }, diff --git a/packages/opencode/test/tool/task.test.ts b/packages/opencode/test/tool/task.test.ts index 11257220b..187a0df94 100644 --- a/packages/opencode/test/tool/task.test.ts +++ b/packages/opencode/test/tool/task.test.ts @@ -363,6 +363,30 @@ describe("tool.task", () => { }, ) + it.instance( + "workflow worker_type catalog lists primary build and plan, task catalog does not", + () => + Effect.gen(function* () { + const agent = yield* Agent.Service + const build = yield* agent.get("build") + const registry = yield* ToolRegistry.Service + const tools = yield* registry.tools({ ...ref, agent: build }) + const taskDescription = tools.find((tool) => tool.id === TaskTool.id)?.description ?? "" + const workflowDescription = tools.find((tool) => tool.id === WorkflowTool.id)?.description ?? "" + + // blocks compile coding/prototype to build and plan blocks to plan, so + // the workflow catalog must let the model name them explicitly + expect(workflowDescription).toContain("- build:") + expect(workflowDescription).toContain("- plan:") + expect(workflowDescription).toContain("- general:") + expect(workflowDescription).toContain("- explore:") + // hidden primaries stay hidden and the task catalog stays subagent-only + expect(workflowDescription).not.toContain("- compaction:") + expect(taskDescription).not.toContain("- build:") + expect(taskDescription).not.toContain("- plan:") + }), + ) + it.instance( "description hides denied subagents for the caller", () =>