diff --git a/.github/workflows/specgit-accept.yml b/.github/workflows/specgit-accept.yml index 96f4d84cd6..52d791f50f 100644 --- a/.github/workflows/specgit-accept.yml +++ b/.github/workflows/specgit-accept.yml @@ -64,7 +64,7 @@ jobs: # same CLI generation that wrote the binding; upgrades are a # deliberate re-init. An isolated prefix avoids installing the # adopting project's dependencies or running its lifecycle scripts. - run: npm install --prefix "$RUNNER_TEMP/specgit-cli" --no-save --no-audit --no-fund specgit@1.13.1 + run: npm install --prefix "$RUNNER_TEMP/specgit-cli" --no-save --no-audit --no-fund specgit@1.14.0 - name: Prepare approved policy for acceptance env: diff --git a/.opencode/hooks/specgit-merge-guard.sh b/.opencode/hooks/specgit-merge-guard.sh index 01981b8098..49e34c573a 100755 --- a/.opencode/hooks/specgit-merge-guard.sh +++ b/.opencode/hooks/specgit-merge-guard.sh @@ -17,7 +17,7 @@ case "$tool" in # (branch "feat/1-a" must never satisfy a record for "feat/1-a2"). branch=$(git branch --show-current 2>/dev/null) if [ -z "$branch" ] || [ ! -f .specgit.yaml ] || ! grep -qFx " branch: $branch" .specgit.yaml; then - echo "specgit: start gate - this branch has no delivery binding. Start the delivery first: specgit issue \": \", then fill each issue body from the discussion, then edit files." >&2 + echo "specgit: start gate - this branch has no delivery binding. Start the delivery first according to the managed guidance: prepare any required body files, then run specgit issue \"<type>: <title>\" with them before editing files." >&2 exit 2 fi exit 0 @@ -25,19 +25,161 @@ case "$tool" in esac command=$(printf '%s' "$payload" | node -e "let s='';process.stdin.on('data',d=>s+=d).on('end',()=>{try{const j=JSON.parse(s);process.stdout.write((j.tool_input&&j.tool_input.command)||'')}catch{process.stdout.write('')}})") -case "$command" in - gh\ pr\ merge*|glab\ mr\ merge*) +# Classify statically visible forge-merge commands without executing or +# expanding shell input. The lexer understands quoting, command separators, +# environment assignments, env/command/exec wrappers, and forge-global repo +# selectors. It deliberately inspects only each simple command's executable +# and leading global options, so quoted prose such as echo "gh pr merge" does +# not trigger the gate. +merge_command=$(printf '%s' "$command" | node -e ' + const fs = require("fs"); + const source = fs.readFileSync(0, "utf8"); + const SQ = String.fromCharCode(39); + const DQ = String.fromCharCode(34); + const BS = String.fromCharCode(92); + + function tokenize(text) { + const tokens = []; + let word = ""; + let active = false; + let quote = 0; + const flush = () => { + if (!active) return; + tokens.push({ kind: "word", value: word }); + word = ""; + active = false; + }; + for (let index = 0; index < text.length; index += 1) { + const char = text[index]; + if (quote === 39) { + if (char === SQ) quote = 0; + else word += char; + continue; + } + if (quote === 34) { + if (char === DQ) { + quote = 0; + } else if (char === BS) { + if (index + 1 >= text.length) return null; + word += text[++index]; + } else { + word += char; + } + continue; + } + if (char === SQ || char === DQ) { + quote = char === SQ ? 39 : 34; + active = true; + } else if (char === BS) { + if (index + 1 >= text.length) return null; + const next = text[++index]; + if (next !== "\n") { + word += next; + active = true; + } + } else if (char === " " || char === "\t" || char === "\r") { + flush(); + } else if (char === "\n" || char === ";" || char === "|" || + char === "&" || char === "(" || char === ")") { + flush(); + if ((char === "|" || char === "&") && text[index + 1] === char) index += 1; + tokens.push({ kind: "boundary" }); + } else if (char === "#" && !active) { + while (index + 1 < text.length && text[index + 1] !== "\n") index += 1; + } else { + word += char; + active = true; + } + } + if (quote !== 0) return null; + flush(); + return tokens; + } + + const executable = (word) => { + const base = word.replace(/\\/g, "/").split("/").pop() || ""; + return base.toLowerCase().replace(/\.exe$/, ""); + }; + const assignment = /^[A-Za-z_][A-Za-z0-9_]*=/; + + function unwrap(words) { + let index = 0; + while (assignment.test(words[index] || "")) index += 1; + if (executable(words[index] || "") === "env") { + index += 1; + while (assignment.test(words[index] || "")) index += 1; + } + while (["command", "exec"].includes(executable(words[index] || ""))) { + index += 1; + } + return words.slice(index); + } + + function isForgeMerge(segment) { + const words = unwrap(segment); + const forge = executable(words[0] || ""); + if (forge !== "gh" && forge !== "glab") return false; + let index = 1; + while (index < words.length) { + const option = words[index]; + if (["-R", "--repo", "--hostname"].includes(option)) { + if (index + 1 >= words.length) return false; + index += 2; + } else if (option === "--" || option.startsWith("--repo=") || + option.startsWith("--hostname=") || + (option.startsWith("-R") && option.length > 2)) { + index += 1; + if (option === "--") break; + } else { + break; + } + } + return forge === "gh" + ? words[index] === "pr" && words[index + 1] === "merge" + : words[index] === "mr" && words[index + 1] === "merge"; + } + + const tokens = tokenize(source); + if (tokens === null) { + process.stdout.write("indeterminate"); + } else { + let segment = []; + for (const token of [...tokens, { kind: "boundary" }]) { + if (token.kind === "word") { + segment.push(token.value); + } else { + if (isForgeMerge(segment)) { + process.stdout.write("merge"); + process.exit(0); + } + segment = []; + } + } + } +') +classifier_status=$? +if [ "$classifier_status" -ne 0 ]; then + merge_command=indeterminate +fi + +case "$merge_command" in + indeterminate) + echo "specgit: command blocked - the merge guard could not safely classify the shell input. Retry with a direct gh pr merge or glab mr merge command." >&2 + exit 2 + ;; + merge) exec node -e ' const { spawn } = require("child_process"); const fs = require("fs"); const path = require("path"); - const ghMsRaw = parseInt(process.env.SPECGIT_GH_TIMEOUT_MS || "", 10); - const ghMs = Number.isFinite(ghMsRaw) && ghMsRaw > 0 ? ghMsRaw : 15000; - const ghS = Math.max(1, Math.floor(ghMs / 1000)); - let budgetS = Math.max(60, ghS * 8); + const timeoutMs = ["SPECGIT_GH_TIMEOUT_MS", "SPECGIT_GLAB_TIMEOUT_MS"] + .map((name) => parseInt(process.env[name] || "", 10)) + .map((value) => Number.isFinite(value) && value > 0 ? value : 15000); + const providerS = Math.max(1, Math.ceil(Math.max(...timeoutMs) / 1000)); + let budgetS = Math.max(60, providerS * 8); const overrideRaw = parseInt(process.env.SPECGIT_GUARD_BUDGET_S || "", 10); if (Number.isFinite(overrideRaw) && overrideRaw > 0) { - budgetS = Math.max(overrideRaw, ghS); + budgetS = Math.max(overrideRaw, providerS); } // The hook runner kills long hooks; surface the mismatch instead of // being cut off mid-verdict. @@ -135,7 +277,7 @@ case "$command" in ); } else { lines.push( - "specgit: merge blocked - no verdict possible (evidence incomplete, exit " + code + "). This is not a rejection: fix evidence gathering (network, gh auth), then retry." + "specgit: merge blocked - no verdict possible (evidence incomplete, exit " + code + "). This is not a rejection: follow errors[].fix in the specgit finish --json result first. Run specgit doctor --json only for git, repository, origin, configured provider CLI/auth, or policy probes, then retry." ); } if (pending.length > 0) { @@ -167,9 +309,33 @@ case "$command" in }); ' ;; - git\ push\ origin\ main*|git\ push\ origin\ +main*|git\ push\ origin\ HEAD:main*) - echo "specgit: direct push to main is not the delivery path. Deliveries go: specgit issue -> PR -> CI -> specgit finish (exit 0) -> merge." >&2 - exit 2 +esac +unset classifier_status merge_command + +specgit_default_ref() { + specgit_ref=$(git symbolic-ref --quiet refs/remotes/origin/HEAD 2>/dev/null) || return 1 + case "$specgit_ref" in + refs/remotes/origin/HEAD) return 1 ;; + refs/remotes/origin/?*) ;; + *) return 1 ;; + esac + git rev-parse --verify "$specgit_ref^{commit}" >/dev/null 2>&1 || return 1 + printf '%s' "$specgit_ref" +} + +case "$command" in + git\ push\ origin\ *) + default_ref=$(specgit_default_ref) || { + echo "specgit: cannot prove origin/HEAD. Run git fetch origin and git remote set-head origin -a before pushing." >&2 + exit 2 + } + default_branch=${default_ref#refs/remotes/origin/} + case "$command" in + git\ push\ origin\ "$default_branch"|git\ push\ origin\ "$default_branch"\ *|git\ push\ origin\ +"$default_branch"|git\ push\ origin\ +"$default_branch"\ *|git\ push\ origin\ HEAD:"$default_branch"|git\ push\ origin\ HEAD:"$default_branch"\ *) + echo "specgit: direct push to $default_branch is not the delivery path. Deliveries go: specgit issue -> PR/MR -> CI -> specgit finish (exit 0) -> merge." >&2 + exit 2 + ;; + esac ;; esac exit 0 diff --git a/.specgit.yaml b/.specgit.yaml index 69e622331b..2995f89572 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,12 +1,38 @@ version: 1 -delivery: dag-release +delivery: hooks-runtime-reliability context: kind: branch - branch: chore/554-dag-release + branch: fix/557-hooks-runtime-reliability issues: - - 554 - - 556 + - 557 + - 558 + - 559 + - 560 + - 561 + - 562 + - 563 + - 564 + - 565 + - 566 issueKinds: - - issue: 554 - kind: kind::chore -pr: 555 + - issue: 557 + kind: kind::fix + - issue: 558 + kind: kind::fix + - issue: 559 + kind: kind::fix + - issue: 560 + kind: kind::fix + - issue: 561 + kind: kind::fix + - issue: 562 + kind: kind::fix + - issue: 563 + kind: kind::fix + - issue: 564 + kind: kind::fix + - issue: 565 + kind: kind::fix + - issue: 566 + kind: kind::fix +pr: 567 diff --git a/AGENTS.md b/AGENTS.md index 65c4c4d797..a0b188bdf7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -290,21 +290,22 @@ already exists); keep manual guidance outside them. ### The delivery story - Start with `specgit issue <title-or-number>...`: 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. -- Use the issue/PR templates explicitly selected by policy. With + the issues, writes and pushes the initial binding on the delivery branch, + opens the draft pull or merge request with the supplied body, selected policy + template, or built-in scaffold, then records and pushes its number. Re-running + resumes; it is idempotent. +- Use the issue and PR/MR templates explicitly selected by policy. With `validation.bodies` or `required_sections`, prepare complete content from the discussion before bootstrap and supply `--body-file <path>` per new - title and `--pr-body-file <path>`. Without body rules, built-in scaffolds - can be filled after creation. Preserve every `Closes #n`; enabled body + title and `--pr-body-file <path>`. Without enforced body rules, the selected + policy template or built-in scaffold can be filled after creation. Preserve + every `Closes #n`; enabled body rules apply at creation and acceptance. Resume keeps existing remote bodies and user edits. Unselected repository templates are not silently loaded. -- A draft pull request always fails the verdict (`pr_draft`): before +- A draft PR/MR always fails the verdict (`pr_draft`): before `specgit finish`, mark it ready for review — `gh pr ready <number>` on GitHub, `glab mr update <number> --ready` on GitLab. -- `specgit finish` is read-only: its verdict comes from real git, PR, +- `specgit finish` is read-only: its verdict comes from real git, PR/MR, and CI evidence; exit 0 means accepted. With automation enabled, the trusted remote workflow continues after CI without another confirmation. `specgit pr --merge --json` is the recovery path: it verifies the approved @@ -314,7 +315,7 @@ already exists); keep manual guidance outside them. ### Issue tags -- Follow the project's `language` for issues and PRs. Enabled `validation` +- Follow the project's `language` for issues and PRs/MRs. Enabled `validation` rules check titles and labels before creation and during `finish`. `kind` mode requires one catalog kind and only declared extras; `project` mode selects only policy `tags`. Users choose rule changes with @@ -332,12 +333,12 @@ already exists); keep manual guidance outside them. ### 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 +- `specgit pr` repairs the PR/MR binding: with no arguments it + auto-discovers the 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. + origin. `specgit doctor` probes git, repository, origin, the configured + provider CLI (`gh`, or `glab` for a declared GitLab host), and policy. ### The command surface @@ -347,17 +348,23 @@ already exists); keep manual guidance outside them. - `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. -- Automation defaults to off (`--automation no`). Only when the user personally chooses - yes may `specgit init --automation yes --merge-target <branch>` enable it; - ordinary `init --force` preserves that choice and target. An agent must not answer yes for the user. +- Automation defaults to off (`--automation no`). For a fresh policy, only + when the user personally chooses yes may they enable it with + `specgit init --automation yes --merge-target <branch>`. To change an + existing policy, use + `specgit init --force --automation yes --merge-target <branch>`; plain + `init --force` preserves its current choice and target. An agent must not + answer yes for the user. ### 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 <n>`) — compare - the WHY, not just the wording. + similar open work through the authenticated session: on GitHub use + `gh issue list --state open --search "<keywords>"`; on GitLab use + `glab issue list --search "<keywords>" --in title`. Narrow + further with labels when useful. +- Open and read every plausible candidate with `gh issue view <n>` on GitHub + or `glab issue view <n>` on GitLab — 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 @@ -396,9 +403,14 @@ verified on its own evidence, split it before binding. artifacts. Trivial replies and read-only questions need none of this. - Local maintenance: installing or upgrading the CLI and running `init` / - `setup` to refresh local configuration and entry points need no issue, PR, + `setup` to refresh local configuration and entry points need no issue, PR/MR, product build, or release when no product or shared-rule change is intended - for commit. Review tracked diffs before choosing what to share; ignore rules + for commit. After a package upgrade, a human may run plain `specgit init` + and approve its guided refresh when it proves drift; non-interactive agents + run `specgit init --force --no-protect`, then `specgit setup --tool all`, + then verify `specgit status --json`. Append `--no-ignore` to init when + authoritative delivery files are intentionally tracked without the managed + ignore block; setup preserves that proven choice. Review tracked diffs before choosing what to share; ignore rules are never CI exemptions. Follow the host project's verification policy for the actual changed inputs; documentation may itself be a product input. Publishing requires explicit release intent within existing user authorization; @@ -406,18 +418,19 @@ verified on its own evidence, split it before binding. - `specgit finish` exit `0` means accepted. Report completed only after the configured target merge and every bound issue closure are confirmed. Never declare completion from task lists, file states, or tests alone. - Track a failed PR with a new repair issue; repeated causes reuse an open - repair issue and do not require abandoning the original PR. -- Use existing user authorization to complete issue bodies, the PR body + Track a failed PR/MR with a new repair issue; repeated causes reuse an open + repair issue and do not require abandoning the original PR/MR. +- Use existing user authorization to complete issue bodies, the PR/MR body and ready transition, CI repairs or retries, acceptance, and the authorized merge. When user authorization or platform permission is missing, present the prepared result and name the specific gap. Documentation and entry points do not grant permission themselves. - 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 + the gates named; `3` = evidence missing, so follow `errors[].fix` first. + Run `specgit doctor --json` only for git, repository, origin, configured + provider CLI/auth, or policy probes. Never present exit `3` as success. +- Keep the `Closes #n` references in the PR/MR body intact; after changing + the PR/MR 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. diff --git a/packages/core/src/plugin/skill/configure-hooks.md b/packages/core/src/plugin/skill/configure-hooks.md index 7718f64897..cf806c94f4 100644 --- a/packages/core/src/plugin/skill/configure-hooks.md +++ b/packages/core/src/plugin/skill/configure-hooks.md @@ -14,11 +14,11 @@ Config lives in dedicated `hooks.json` files (NOT `opencode.json`, NOT ## Where files live -| Scope | Path | Hot-reloaded? | -| ------- | ------------------------------------ | --------------------------------- | -| Global | `~/.config/opencode/hooks.json` | No — requires restart | -| Project | `.opencode/hooks.json` | Yes — polled every ~2s | -| Worktree| `<worktree>/.opencode/hooks.json` | Yes (when worktree ≠ project dir) | +| Scope | Path | Hot-reloaded? | +| -------- | --------------------------------- | --------------------------------- | +| Global | `~/.config/opencode/hooks.json` | Yes — polled every ~2s | +| Project | `.opencode/hooks.json` | Yes — polled every ~2s | +| Worktree | `<worktree>/.opencode/hooks.json` | Yes (when worktree ≠ project dir) | Layers concat-append (do NOT override by key): global hooks run, then project hooks are appended after, in file order. A single event can have hooks from @@ -86,25 +86,42 @@ this skill is a map, not the full schema. ## Hook types (all 5 implemented) -| `type` | What it does | -| --------- | ----------------------------------------------------------------------------- | +| `type` | What it does | +| --------- | ---------------------------------------------------------------------------------------------- | | `command` | Runs a shell command. Event data is piped to stdin as JSON; stdout/exit code drive the result. | -| `mcp` | Invokes an MCP tool, addressed as `mcp__<server>__<tool>`. | -| `http` | POSTs the event envelope to `url`; response body is parsed as JSON. | -| `prompt` | Sends the event to an LLM, constrained to structured JSON output. | -| `agent` | Runs an autonomous sub-agent loop (bash/read_file/list_dir/grep) to react to the event. | +| `mcp` | Invokes an MCP tool, addressed as `mcp__<server>__<tool>`. | +| `http` | POSTs the event envelope to `url`; response body is parsed as JSON. | +| `prompt` | Sends the event to an LLM, constrained to structured JSON output. | +| `agent` | Runs an autonomous sub-agent loop (bash/read_file/list_dir/grep) to react to the event. | ### `command` protocol - stdin: JSON envelope with event data - exit code `0`: success, stdout optionally parsed as `HookJSONOutput` JSON - exit code `2`: **block** — stderr becomes the block reason, shown to the agent -- any other exit code, or timeout: logged as a warning, does NOT abort the flow +- any other exit code, or timeout: logged as a warning; stdout control fields are ignored - `${CLAUDE_PLUGIN_ROOT}` / `${CLAUDE_PLUGIN_DATA}` expand to the directory the hook was declared in / its data dir — usable in `command` - `options` (fork-only field, no CC equivalent): exported as `CLAUDE_PLUGIN_OPTION_<KEY>` env vars in the subprocess +### Handler options and agent tools + +- `timeout`: positive seconds; cancellation reaches the running process, model request or MCP tool. +- `shell`: command hooks can explicitly select `bash` or `powershell`; that interpreter must be installed. Omit it for `/bin/sh` on POSIX or `cmd.exe` on Windows. +- `allowedEnvVars`: when supplied, HTTP hooks expand `$NAME` and `${NAME}` in headers only for names in this list. Unlisted or unset variables expand to an empty string. When omitted, headers remain literal. +- `statusMessage`: recorded in the hook execution log before dispatch; it does not create a UI progress indicator. +- Command-level `once`: runs once per session for the current loaded configuration entry, including concurrent/async triggers. Reloading the file creates fresh entries. Session registration-level `once` atomically claims the whole matching group; unmatched conditions leave it available. +- Dynamic session registration supports the same command fields, including `options`. + +Agent hooks provide `read_file`, `list_dir`, `grep`, and a restricted `bash` tool. +The latter directly executes installed POSIX system utilities with validated arguments; +it does not invoke a shell. It supports common read-only options for file inspection, +`find` predicates and `git status/log/diff/show`. `sed` is limited to +`-n '<line>[,<line>]p' <files>`. Interpreters such as `awk`, shell composition, +output-file options, external Git diff drivers and commands found through a +project-controlled PATH are unavailable. Use the three file tools on Windows. + ### Common output fields (`HookJSONOutput`, applies across types) ```json @@ -119,14 +136,26 @@ this skill is a map, not the full schema. } ``` -Not every field applies to every event — `hookSpecificOutput` shape varies per -event (see `HookSpecificOutput` in `settings.ts` for the exact per-event union). +`continue: false` stops prompt admission and wins over a Stop hook's request to +continue. Permission hooks can reject using `permissionDecision: "deny"`, a block +decision, or exit code 2. Post-tool block reasons and contexts are appended to +model-facing tool feedback after execution; they cannot undo a completed write. +`FileChanged` carries an absolute path and `add`, `change` or `delete`; patch +renames report both the removed path and the added destination. + +Some fields are retained only for schema compatibility. `initialUserMessage`, +`watchPaths`, `updatedMCPToolOutput`, `displayMessage`, `compactSummary` and +`customSummary` emit unsupported-output warnings and do not alter runtime state. +Use `additionalContext` for model feedback. `suppressOutput` remains a no-op: +hook stdout is not displayed directly in the UI. Prompt/agent hooks using OpenAI +OAuth are currently skipped with a warning; use a supported API-key provider. +Invalid output shapes are logged and ignored before aggregation. ## Applying changes -Global `hooks.json` loads once at startup — **restart required**. Project and -worktree `hooks.json` are polled and take effect within a few seconds without -a restart. +Global, project and worktree `hooks.json` files are polled every ~2 seconds, +with a 500 ms debounce. Changes take effect within a few seconds. Invalid +matcher/command entries are logged and skipped while valid siblings remain active. ## Migrating from Claude Code diff --git a/packages/opencode/src/hook/agent-tools.ts b/packages/opencode/src/hook/agent-tools.ts index 3a7f98a0f2..c4f2d1ca54 100644 --- a/packages/opencode/src/hook/agent-tools.ts +++ b/packages/opencode/src/hook/agent-tools.ts @@ -1,22 +1,4 @@ -/** - * WP-4D micro-WP-1 — agent-handler tool set (LLM-facing). - * - * Builds the 5-tool palette consumed by the WP-4D-2 agent loop: - * read_file / list_dir / grep / bash / synthetic_output - * - * Design contract: - * - All tools are pure ai-SDK `Tool` values; no Effect dependencies on the - * LLM-side execute path. Effect Services (spawner / fs) are pre-resolved - * by the caller and captured via closure. - * - Every `execute` is wrapped in try/catch. Errors return - * `{ output: "Error: <message>" }` and **never throw** — the agent loop - * must be able to keep running and let the model decide whether to retry. - * - bash uses a strict read-only whitelist. The token list and forbidden - * metachar regex are the v1 contract; expanding either requires a - * deliberate WP, not a one-off addition. - * - synthetic_output writes into the caller-owned `captured.value` slot; - * the loop polls it after each turn to decide termination. - */ +/** Read-only tools for model-driven hooks. Every operation observes the hook abort signal. */ import path from "path" import { Effect, Stream } from "effect" import { ChildProcess } from "effect/unstable/process" @@ -24,59 +6,10 @@ import type { ChildProcessSpawner } from "effect/unstable/process/ChildProcessSp import { type Tool, tool, jsonSchema } from "ai" import { FSUtil } from "@opencode-ai/core/fs-util" import type { HookJSONOutput } from "./settings" +import { HookOutputSchema } from "./schema" +import { FORBIDDEN_META, parseReadonlyCommand, readonlyExecutable, whitelistReject } from "./readonly-command" -// ── bash whitelist (read-only, POSIX-only) ────────────────────── - -const BASH_WHITELIST_SINGLE = new Set([ - "ls", - "cat", - "grep", - "find", - "test", - "wc", - "head", - "tail", - "sort", - "uniq", - "awk", - "echo", - "pwd", - "which", - "file", - "stat", -]) - -const BASH_WHITELIST_PAIR = new Set([ - "git status", - "git log", - "git diff", - "git show", - "sed -n", - "du -sh", -]) - -/** - * Reject metacharacters that enable composition / redirection / substitution. - * v1 only allows a single command invocation — no pipes, chains, redirects, - * background, command substitution, or backticks. - */ -const FORBIDDEN_META = /[|;&`$<>]|\$\(|\)\s*$/ - -function whitelistReject(cmd: string): string | null { - const trimmed = cmd.trim() - if (!trimmed) return "empty command" - if (FORBIDDEN_META.test(trimmed)) - return `compound/redirect not allowed in v1: ${trimmed.slice(0, 60)}` - const tokens = trimmed.split(/\s+/) - const first = tokens[0] - const pair = tokens.length >= 2 ? `${tokens[0]} ${tokens[1]}` : "" - if (BASH_WHITELIST_SINGLE.has(first)) return null - if (pair && BASH_WHITELIST_PAIR.has(pair)) return null - return `command "${first}" not in read-only whitelist` -} - -// Exported for unit tests only — not part of the runtime surface. -export const __test__ = { whitelistReject, BASH_WHITELIST_SINGLE, BASH_WHITELIST_PAIR, FORBIDDEN_META } +export const __test__ = { whitelistReject, FORBIDDEN_META } // ── helpers ───────────────────────────────────────────────────── @@ -88,38 +21,6 @@ const MAX_BASH_OUTPUT = 8000 const MAX_GREP_RESULTS_DEFAULT = 100 const MAX_READ_LINES_DEFAULT = 2000 -// ── synthetic_output schema (mirrors HookJSONOutput) ──────────── - -const HOOK_OUTPUT_SCHEMA = { - type: "object", - properties: { - continue: { type: "boolean" }, - stopReason: { type: "string" }, - suppressOutput: { type: "boolean" }, - systemMessage: { type: "string" }, - decision: { type: "string", enum: ["approve", "block"] }, - reason: { type: "string" }, - hookSpecificOutput: { - type: "object", - properties: { - hookEventName: { type: "string" }, - permissionDecision: { type: "string", enum: ["allow", "deny", "ask"] }, - permissionDecisionReason: { type: "string" }, - updatedInput: { type: "object" }, - additionalContext: { type: "string" }, - initialUserMessage: { type: "string" }, - updatedMCPToolOutput: {}, - }, - }, - }, -} as const - -// Compile-time guard: the synthetic_output schema must remain a structural -// subset of HookJSONOutput. If HookJSONOutput grows a required field, this -// assignment has to be updated alongside HOOK_OUTPUT_SCHEMA above. -const _schemaTypeCheck: (a: HookJSONOutput) => HookJSONOutput = (a) => a -void _schemaTypeCheck - // ── factory ───────────────────────────────────────────────────── export interface BuildAgentToolsDeps { @@ -148,7 +49,7 @@ export function buildAgentTools(deps: BuildAgentToolsDeps): Record<string, Tool> execute: async (args: any) => { try { const resolved = resolvePath(String(args.path), cwd) - const text = await Effect.runPromise(fs.readFileString(resolved) as Effect.Effect<string, unknown>) + const text = await Effect.runPromise(fs.readFileString(resolved), { signal }) const offset = typeof args.offset === "number" && args.offset > 0 ? args.offset - 1 : 0 const limit = typeof args.limit === "number" && args.limit > 0 ? args.limit : MAX_READ_LINES_DEFAULT const lines = text.split("\n").slice(offset, offset + limit) @@ -176,8 +77,9 @@ export function buildAgentTools(deps: BuildAgentToolsDeps): Record<string, Tool> const lines: string[] = [] const walk = async (dir: string, rel: string): Promise<void> => { - const entries = await Effect.runPromise(fs.readDirectoryEntries(dir) as Effect.Effect<FSUtil.DirEntry[], unknown>) + const entries = await Effect.runPromise(fs.readDirectoryEntries(dir), { signal }) for (const e of entries) { + signal.throwIfAborted() const display = (rel ? rel + "/" : "") + e.name + (e.type === "directory" ? "/" : "") lines.push(display) if (recursive && e.type === "directory") { @@ -211,7 +113,8 @@ export function buildAgentTools(deps: BuildAgentToolsDeps): Record<string, Tool> try { const re = new RegExp(String(args.pattern)) const root = resolvePath(String(args.path), cwd) - const max = typeof args.max_results === "number" && args.max_results > 0 ? args.max_results : MAX_GREP_RESULTS_DEFAULT + const max = + typeof args.max_results === "number" && args.max_results > 0 ? args.max_results : MAX_GREP_RESULTS_DEFAULT // Treat include as a suffix filter only — minimatch is not in the // hook subsystem's dep set and grep is best-effort here. Strip a // leading '*' so '*.ts' and '.ts' both work. @@ -225,8 +128,9 @@ export function buildAgentTools(deps: BuildAgentToolsDeps): Record<string, Tool> if (suffix && !filepath.endsWith(suffix)) return let content: string try { - content = await Effect.runPromise(fs.readFileString(filepath) as Effect.Effect<string, unknown>) + content = await Effect.runPromise(fs.readFileString(filepath), { signal }) } catch { + signal.throwIfAborted() return } const lines = content.split("\n") @@ -239,15 +143,16 @@ export function buildAgentTools(deps: BuildAgentToolsDeps): Record<string, Tool> } const walk = async (dir: string): Promise<void> => { - const entries = await Effect.runPromise(fs.readDirectoryEntries(dir) as Effect.Effect<FSUtil.DirEntry[], unknown>) + const entries = await Effect.runPromise(fs.readDirectoryEntries(dir), { signal }) for (const e of entries) { + signal.throwIfAborted() const child = path.join(dir, e.name) if (e.type === "directory") await walk(child) else if (e.type === "file") await scanFile(child) } } - const isDir = await Effect.runPromise(fs.isDir(root)) + const isDir = await Effect.runPromise(fs.isDir(root), { signal }) if (isDir) await walk(root) else await scanFile(root) @@ -262,7 +167,7 @@ export function buildAgentTools(deps: BuildAgentToolsDeps): Record<string, Tool> const bash = tool({ description: - "Run a single read-only shell command (whitelist enforced: ls/cat/grep/find/git status/git log/git diff/git show/sed -n/test/wc/head/tail/sort/uniq/awk/echo/pwd/which/file/stat/du -sh). No pipes, redirects, or substitution.", + "Run one POSIX read-only command with restricted options: ls/cat/grep/find/git status/log/diff/show/sed -n/test/wc/head/tail/sort/uniq/echo/pwd/which/file/stat/du. Quotes are supported; shell syntax, interpreters and output-file options are rejected.", inputSchema: jsonSchema({ type: "object", properties: { command: { type: "string" } }, @@ -270,22 +175,24 @@ export function buildAgentTools(deps: BuildAgentToolsDeps): Record<string, Tool> }), execute: async (args: any) => { const command = String(args?.command ?? "") - const reject = whitelistReject(command) - if (reject) return { output: `Error: ${reject}` } - - // Align with settings.ts execShell: use cmd.exe on Windows (no `sh`), - // POSIX `sh -c` elsewhere. Previously this bailed on win32 while command - // hooks ran fine via cmd.exe — asymmetric behavior across handler types. - const isWin = process.platform === "win32" - try { + signal.throwIfAborted() + const parsed = parseReadonlyCommand(command) + const executable = readonlyExecutable(parsed.name) const result = await Effect.runPromise( Effect.scoped( Effect.gen(function* () { const handle = yield* spawner.spawn( - ChildProcess.make(isWin ? "cmd.exe" : "sh", isWin ? ["/c", command] : ["-c", command], { + ChildProcess.make(executable, parsed.args, { cwd, extendEnv: true, + env: { + GIT_OPTIONAL_LOCKS: "0", + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: "/dev/null", + GIT_NO_LAZY_FETCH: "1", + GIT_TERMINAL_PROMPT: "0", + }, stdin: "ignore", stdout: "pipe", stderr: "pipe", @@ -301,7 +208,8 @@ export function buildAgentTools(deps: BuildAgentToolsDeps): Record<string, Tool> ) return { stdout, stderr, code } }), - ) as Effect.Effect<{ stdout: string; stderr: string; code: number }, unknown>, + ), + { signal }, ) const body = `exit=${result.code}\n${result.stdout}` + (result.stderr ? `\n[stderr]\n${result.stderr}` : "") @@ -314,10 +222,11 @@ export function buildAgentTools(deps: BuildAgentToolsDeps): Record<string, Tool> const synthetic_output = tool({ description: "Emit the final hook decision and stop. Call this exactly once when ready to terminate.", - inputSchema: jsonSchema(HOOK_OUTPUT_SCHEMA as Record<string, unknown>), + inputSchema: HookOutputSchema, execute: async (args: any) => { try { - captured.value = args as HookJSONOutput + signal.throwIfAborted() + captured.value = HookOutputSchema.parse(args) as HookJSONOutput return { output: "ok" } } catch (e: any) { return { output: `Error: ${e?.message ?? String(e)}` } @@ -325,13 +234,6 @@ export function buildAgentTools(deps: BuildAgentToolsDeps): Record<string, Tool> }, }) - // signal is captured for the loop's transport-level cancellation; the - // tool execute paths above don't directly consume it (Effect.scoped on - // the bash spawn unwinds child handles when the runtime is interrupted - // by the outer agent loop). Reference here is intentional to keep the - // dep contract honest without a noisy unused-param warning. - void signal - return { read_file, list_dir, grep, bash, synthetic_output } } diff --git a/packages/opencode/src/hook/extensions/hot-reload.ts b/packages/opencode/src/hook/extensions/hot-reload.ts index 48b0eebe9d..c9ded4fbc3 100644 --- a/packages/opencode/src/hook/extensions/hot-reload.ts +++ b/packages/opencode/src/hook/extensions/hot-reload.ts @@ -36,11 +36,7 @@ const POLL_INTERVAL_MS = 2000 * is never read. Global is included so editing `~/.config/opencode/hooks.json` * takes effect without a restart (previously startup-only). */ -function watchedFiles( - projectDir: string, - worktree: string | undefined, - opencodeGlobalConfig?: string, -): string[] { +function watchedFiles(projectDir: string, worktree: string | undefined, opencodeGlobalConfig?: string): string[] { const files: string[] = [] if (opencodeGlobalConfig) files.push(path.join(opencodeGlobalConfig, "hooks.json")) files.push(path.join(projectDir, ".opencode", "hooks.json")) @@ -111,13 +107,13 @@ export function watchSettings( const fireReload = (changedFile: string) => { log.info("hooks.json changed, reloading", { file: changedFile }) // Fire-and-forget: reload errors are logged but never crash - Effect.runPromise(reload()).then( - (settings) => { + Effect.runPromise(Effect.suspend(reload)) + .then((settings) => { + if (closed) return log.info("hooks hot-reloaded", { file: changedFile, hookCount: countHooks(settings) }) onReload(settings, changedFile) - }, - (err) => log.warn("hooks reload failed", { file: changedFile, error: String(err) }), - ) + }) + .catch((err) => log.warn("hooks reload failed", { file: changedFile, error: String(err) })) } // Debounce: 500ms. Min 1s between reloads. On min-interval block, reschedule diff --git a/packages/opencode/src/hook/file-changes.ts b/packages/opencode/src/hook/file-changes.ts new file mode 100644 index 0000000000..8d675e0527 --- /dev/null +++ b/packages/opencode/src/hook/file-changes.ts @@ -0,0 +1,37 @@ +import path from "node:path" +import { isRecord } from "@/util/record" + +const FILE_TOOLS = new Set(["write", "edit", "apply_patch", "multiedit", "patch"]) + +/** Prefer actual result metadata; input paths are a fallback for compatible tools. */ +export function toolFileChanges(tool: string, args: Record<string, unknown>, metadata: unknown, cwd: string) { + const changes = new Map<string, { path: string; changeType: "add" | "change" | "delete" }>() + if (!FILE_TOOLS.has(tool)) return [] + const add = (file: unknown, changeType: "add" | "change" | "delete") => { + if (typeof file !== "string" || !file.trim()) return + const resolved = path.resolve(cwd, file) + changes.set(resolved, { path: resolved, changeType }) + } + const data = isRecord(metadata) ? metadata : {} + if (Array.isArray(data.files)) { + for (const file of data.files) { + if (!isRecord(file)) continue + if (file.type === "move" && typeof file.movePath === "string") { + add(file.filePath, "delete") + add(file.movePath, "add") + } else { + add(file.filePath, file.type === "add" ? "add" : file.type === "delete" ? "delete" : "change") + } + } + } else { + const diff = isRecord(data.filediff) ? data.filediff : {} + const file = data.filepath ?? diff.file ?? args.filePath ?? args.file_path ?? args.path + add(file, tool === "write" && data.exists === false ? "add" : "change") + if (tool === "multiedit" && Array.isArray(args.edits)) { + for (const edit of args.edits) { + if (isRecord(edit)) add(edit.filePath ?? edit.file_path ?? edit.path, "change") + } + } + } + return [...changes.values()] +} diff --git a/packages/opencode/src/hook/readonly-command.ts b/packages/opencode/src/hook/readonly-command.ts new file mode 100644 index 0000000000..11cd7ca1ae --- /dev/null +++ b/packages/opencode/src/hook/readonly-command.ts @@ -0,0 +1,161 @@ +import { existsSync } from "node:fs" + +// This is an argument parser, never a shell. Expansion and command composition +// are deliberately unavailable, even inside quotes. +export const FORBIDDEN_META = /[\x00-\x1f\x7f|;&`$<>]/ + +function tokens(command: string): string[] { + if (FORBIDDEN_META.test(command)) throw new Error("shell syntax and control characters are not allowed") + const result: string[] = [] + let token = "" + let quote = "" + let started = false + for (let i = 0; i < command.length; i++) { + const c = command[i] + if (c === "\\" && quote !== "'") { + if (++i === command.length) throw new Error("unfinished escape") + token += command[i] + started = true + } else if (quote) { + if (c === quote) quote = "" + else token += c + } else if (c === "'" || c === '"') { + quote = c + started = true + } else if (c === " ") { + if (started) result.push(token) + token = "" + started = false + } else { + token += c + started = true + } + } + if (quote) throw new Error("unfinished quote") + if (started) result.push(token) + if (!result.length) throw new Error("empty command") + return result +} + +// Only known read-only options are accepted. A command-name allowlist alone +// admits output files, interpreters, external diff drivers and find actions. +const flags: Record<string, RegExp> = { + ls: /^(-[aAbBcCdDfFgGhHiIlLmMnNoOpPqQrRsStTuUvVwWxX1]+|--(all|almost-all|directory|human-readable|recursive))$/, + cat: /^(-[benstuvET]+|--(number|number-nonblank|show-ends|show-tabs|squeeze-blank))$/, + grep: /^(-[EFGivwxcLlnHhroqsaIR]+|--(line-number|ignore-case|files-with-matches|files-without-match|fixed-strings|extended-regexp))$/, + wc: /^(-[clmwL]+|--(bytes|chars|lines|words|max-line-length))$/, + head: /^(-[qv]+|-[0-9]+)$/, + tail: /^(-[qvfF]+|-[0-9]+)$/, + sort: /^(-[bdfghinMrsuV]+|--(numeric-sort|reverse|unique|ignore-case|stable|check))$/, + uniq: /^(-[cdiu]+|--(count|repeated|unique|ignore-case))$/, + echo: /^-[neE]+$/, + pwd: /^-[LP]+$/, + which: /^-a$/, + file: /^(-[bhiL]+|--(brief|mime|mime-type|mime-encoding))$/, + stat: /^(-[Lf]+|--(dereference|file-system|terse))$/, + du: /^(-[achHkLmsx]+|--(summarize|human-readable|total))$/, + test: /^(-[abcdefghkLmnoprSstuvwxzOGN]+|-eq|-ne|-gt|-ge|-lt|-le)$/, +} + +function simple(name: string, args: string[]) { + const pattern = flags[name] + if (!pattern) throw new Error(`command "${name}" not in read-only whitelist`) + let positional = 0 + let literal = false + for (let i = 0; i < args.length; i++) { + const arg = args[i] + if (literal || !arg.startsWith("-") || arg === "-") { + positional++ + continue + } + if (arg === "--") { + literal = true + continue + } + if ((name === "head" || name === "tail") && /^-(n|c)$/.test(arg)) { + if (!/^[+-]?\d+$/.test(args[++i] ?? "")) throw new Error("line/byte count must be numeric") + continue + } + if (!pattern.test(arg)) throw new Error(`option "${arg}" is not allowed for ${name}`) + } + // uniq's SECOND positional argument is an output file. + if (name === "uniq" && positional > 1) throw new Error("uniq output files are not allowed") +} + +function find(args: string[]) { + const predicates = new Set(["-name", "-iname", "-path", "-ipath", "-type", "-maxdepth", "-mindepth"]) + const operators = new Set(["-print", "-print0", "-empty", "-not", "!", "-a", "-and", "-o", "-or", "(", ")"]) + let expression = false + for (let i = 0; i < args.length; i++) { + const arg = args[i] + if (predicates.has(arg)) { + expression = true + if (args[++i] === undefined) throw new Error(`missing argument for ${arg}`) + } else if (operators.has(arg)) { + expression = true + } else if (expression || arg.startsWith("-")) { + throw new Error(`find action "${arg}" is not allowed`) + } + } +} + +function git(args: string[]) { + const sub = args[0] + if (!["status", "log", "diff", "show"].includes(sub)) throw new Error("git subcommand is not read-only") + const safe = + /^(--(short|branch|porcelain(?:=v?[12])?|oneline|stat|numstat|shortstat|summary|name-only|name-status|check|cached|staged|no-color|no-renames|no-patch|patch|reverse|all|first-parent|no-merges|merges|follow|graph|abbrev-commit|date-order|topo-order|full-history|relative|binary)|-[sbpwu]|-U\d+|-\d+|--(max-count|skip|unified)=\d+|--(format|pretty|date|since|until|author|grep|untracked-files|ignore-submodules)=[^-].*)$/ + let literal = false + for (let i = 1; i < args.length; i++) { + const arg = args[i] + if (literal) continue + if (arg === "--") { + literal = true + continue + } + if (arg === "-n") { + if (!/^\d+$/.test(args[++i] ?? "")) throw new Error("git count must be numeric") + continue + } + if (arg.startsWith("-") && !safe.test(arg)) throw new Error(`git option "${arg}" is not allowed`) + } + return [ + "--no-pager", + "-c", + "core.fsmonitor=false", + "-c", + "core.untrackedCache=false", + sub, + ...(sub === "status" ? [] : ["--no-ext-diff", "--no-textconv"]), + ...args.slice(1), + ] +} + +export function parseReadonlyCommand(command: string): { name: string; args: string[] } { + const [name, ...args] = tokens(command) + if (name === "git") return { name, args: git(args) } + if (name === "find") find(args) + else if (name === "sed") { + // Arbitrary sed programs can write files or execute commands on GNU sed. + if (args[0] !== "-n" || !/^\d+(,\d+)?p$/.test(args[1] ?? "") || args.slice(2).some((s) => s.startsWith("-"))) + throw new Error("sed only supports -n '<line>[,<line>]p' <files>") + } else simple(name, args) + return { name, args } +} + +export function readonlyExecutable(name: string): string { + if (process.platform === "win32") + throw new Error("Use read_file, list_dir or grep on Windows; bash requires POSIX utilities") + // Never resolve an executable from a project-controlled PATH entry. + const executable = [`/usr/bin/${name}`, `/bin/${name}`].find(existsSync) + if (!executable) throw new Error(`system utility ${name} is unavailable`) + return executable +} + +export function whitelistReject(command: string): string | null { + try { + parseReadonlyCommand(command) + return null + } catch (error) { + return error instanceof Error ? error.message : String(error) + } +} diff --git a/packages/opencode/src/hook/rewake.ts b/packages/opencode/src/hook/rewake.ts index fbee871d52..df4c2e7e2a 100644 --- a/packages/opencode/src/hook/rewake.ts +++ b/packages/opencode/src/hook/rewake.ts @@ -20,4 +20,18 @@ export interface Interface { export class Service extends Context.Service<Service, Interface>()("@opencode/HookRewake") {} +export function bind( + submit: (input: { sessionID: SessionID; text: string }) => Effect.Effect<unknown, unknown>, +): Interface { + return { + rewake: (input) => + submit(input).pipe( + Effect.catch((error) => + Effect.logWarning("hook rewake prompt failed", { sessionID: input.sessionID, error: String(error) }), + ), + Effect.asVoid, + ), + } +} + export * as HookRewake from "./rewake" diff --git a/packages/opencode/src/hook/schema.ts b/packages/opencode/src/hook/schema.ts new file mode 100644 index 0000000000..34470cebb0 --- /dev/null +++ b/packages/opencode/src/hook/schema.ts @@ -0,0 +1,54 @@ +import z from "zod" + +export const HookCommandSchema = z + .object({ + type: z.enum(["command", "mcp", "http", "prompt", "agent"]), + command: z.string().optional(), + url: z.string().optional(), + prompt: z.string().optional(), + headers: z.record(z.string(), z.string()).optional(), + allowedEnvVars: z.array(z.string()).optional(), + timeout: z.number().positive().finite().optional(), + statusMessage: z.string().optional(), + once: z.boolean().optional(), + shell: z.enum(["bash", "powershell"]).optional(), + if: z.string().optional(), + async: z.boolean().optional(), + asyncRewake: z.boolean().optional(), + options: z.record(z.string(), z.unknown()).optional(), + __sourceDir: z.string().optional(), + }) + .superRefine((entry, ctx) => { + const value = + entry.type === "http" + ? (entry.url ?? entry.command) + : entry.type === "prompt" || entry.type === "agent" + ? (entry.prompt ?? entry.command) + : entry.command + if (!value?.trim()) + ctx.addIssue({ code: "custom", message: `${entry.type} hook requires a nonempty command, url or prompt` }) + }) + +export const HookSpecificOutputSchema = z.object({ + hookEventName: z.string().optional(), + permissionDecision: z.enum(["allow", "deny", "ask"]).optional(), + permissionDecisionReason: z.string().optional(), + updatedInput: z.record(z.string(), z.unknown()).optional(), + additionalContext: z.string().optional(), + initialUserMessage: z.string().optional(), + updatedMCPToolOutput: z.unknown().optional(), + watchPaths: z.array(z.string()).optional(), + displayMessage: z.string().optional(), + compactSummary: z.string().optional(), + customSummary: z.string().optional(), +}) + +export const HookOutputSchema = z.object({ + continue: z.boolean().optional(), + stopReason: z.string().optional(), + suppressOutput: z.boolean().optional(), + systemMessage: z.string().optional(), + decision: z.enum(["approve", "block"]).optional(), + reason: z.string().optional(), + hookSpecificOutput: HookSpecificOutputSchema.optional(), +}) diff --git a/packages/opencode/src/hook/session-hooks.ts b/packages/opencode/src/hook/session-hooks.ts index 0e5189a1d5..07789f96ee 100644 --- a/packages/opencode/src/hook/session-hooks.ts +++ b/packages/opencode/src/hook/session-hooks.ts @@ -20,34 +20,18 @@ * Storage is process-local memory — entries do NOT survive a restart; users * wanting persistent hooks should use the on-disk hooks.json chain. */ -import { Context, Effect, Layer } from "effect" +import { Context, Effect, Layer, Schema } from "effect" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { SessionID } from "@/session/schema" import { InstanceState } from "@/effect/instance-state" -import type { HookEvent, HookJSONOutput } from "./settings" - -// Shape of the inner hooks array on a session entry. Mirrors the `hooks[]` -// array nested under each HookMatcher in the settings file format. We re-declare -// here rather than import HookCommand to avoid a settings.ts → session-hooks.ts -// import cycle (settings.ts already depends on session-hooks for the trigger merge). -export interface SessionHookCommand { - type: "command" | "mcp" | "http" | "prompt" | "agent" - command?: string - /** Claude Code `type:"http"` endpoint. Legacy configs may still use `command`. */ - url?: string - /** Claude Code `type:"prompt" | "agent"` prompt. Legacy configs may still use `command`. */ - prompt?: string - headers?: Record<string, string> - timeout?: number - shell?: "bash" | "powershell" - if?: string - /** Background execution — see HookCommand.async in settings.ts. */ - async?: boolean - /** Deliver async result to agent — see HookCommand.asyncRewake in settings.ts. */ - asyncRewake?: boolean - options?: Record<string, unknown> - __sourceDir?: string -} +import type { HookEvent, HookJSONOutput, HookCommand } from "./settings" +import { HookCommandSchema } from "./schema" + +export type SessionHookCommand = HookCommand + +export class InvalidHookError extends Schema.TaggedErrorClass<InvalidHookError>()("InvalidHookError", { + message: Schema.String, +}) {} export interface SessionHookEntryInput { event: HookEvent @@ -64,7 +48,9 @@ export interface SessionHookEntry extends SessionHookEntryInput { } export interface Interface { - readonly add: (sessionID: SessionID, entry: SessionHookEntryInput) => Effect.Effect<string> + readonly add: (sessionID: SessionID, entry: SessionHookEntryInput) => Effect.Effect<string, InvalidHookError> + /** Atomically remove a registration if still present; only one trigger can claim it. */ + readonly claim: (sessionID: SessionID, id: string) => Effect.Effect<boolean> readonly remove: (sessionID: SessionID, id: string) => Effect.Effect<void> readonly list: (sessionID: SessionID, event: HookEvent) => Effect.Effect<readonly SessionHookEntry[]> /** All entries for a session across every event (backs the HTTP GET endpoint). */ @@ -88,23 +74,32 @@ export const layer = Layer.effect( ) const add = Effect.fn("SessionHooks.add")(function* (sessionID: SessionID, entry: SessionHookEntryInput) { + const parsed = HookCommandSchema.array().min(1).safeParse(entry.hooks) + if (!parsed.success) return yield* new InvalidHookError({ message: parsed.error.message }) + if (entry.matcher !== undefined && typeof entry.matcher !== "string") + return yield* new InvalidHookError({ message: "matcher must be a string" }) const data = yield* InstanceState.get(state) const list = data.get(sessionID) ?? [] const id = crypto.randomUUID() - list.push({ id, ...entry }) + list.push({ ...entry, id, hooks: parsed.data }) data.set(sessionID, list) return id }) - const remove = Effect.fn("SessionHooks.remove")(function* (sessionID: SessionID, id: string) { + const claim = Effect.fn("SessionHooks.claim")(function* (sessionID: SessionID, id: string) { const data = yield* InstanceState.get(state) const list = data.get(sessionID) - if (!list) return - const next = list.filter((e) => e.id !== id) + if (!list?.some((entry) => entry.id === id)) return false + const next = list.filter((entry) => entry.id !== id) if (next.length === 0) data.delete(sessionID) else data.set(sessionID, next) + return true }) + const remove = Effect.fn("SessionHooks.remove")((sessionID: SessionID, id: string) => + claim(sessionID, id).pipe(Effect.asVoid), + ) + const list = Effect.fn("SessionHooks.list")(function* (sessionID: SessionID, event: HookEvent) { const data = yield* InstanceState.get(state) const arr = data.get(sessionID) ?? [] @@ -128,7 +123,7 @@ export const layer = Layer.effect( data.delete(sessionID) }) - return Service.of({ add, remove, list, listAll, hasForEvent, clear }) + return Service.of({ add, claim, remove, list, listAll, hasForEvent, clear }) }), ) diff --git a/packages/opencode/src/hook/settings.ts b/packages/opencode/src/hook/settings.ts index c9851b2575..5edd329006 100644 --- a/packages/opencode/src/hook/settings.ts +++ b/packages/opencode/src/hook/settings.ts @@ -1,5 +1,5 @@ /** - * Settings-based hook system — Claude Code protocol-level 1:1 compatible. + * Settings-based hooks with Claude Code-compatible envelopes and explicit runtime support. * * Reads hooks from a dedicated hooks.json chain (later layers concat-append on * top of earlier ones, mirroring Claude Code's merge semantics — hooks @@ -46,7 +46,7 @@ import { Effect, Layer, Context, Option, Scope, Exit } from "effect" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http" import { ChildProcessSpawner } from "effect/unstable/process/ChildProcessSpawner" -import z from "zod" +import { HookCommandSchema, HookOutputSchema } from "./schema" import { generateObject, generateText, type ModelMessage } from "ai" import { FSUtil } from "@opencode-ai/core/fs-util" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" @@ -69,7 +69,7 @@ import { isTrusted, trustFilePath } from "./workspace-trust" // [FORK:hook-ext] const log = Log.create({ service: "hook.settings" }) -// ── Types (Claude Code 1:1) ───────────────────────────────────── +// ── Hook protocol types ──────────────────────────────────────── export type HookEvent = | "PreToolUse" @@ -150,8 +150,8 @@ export interface HookCommand { statusMessage?: string once?: boolean /** - * Shell selector for `type:"command"`. CC honors `bash` (default on POSIX) and `powershell` - * (default on Windows). Currently a schema placeholder — execShell still picks based on platform. + * Explicit interpreter for command hooks. When omitted, preserve the platform + * default (/bin/sh on POSIX, cmd.exe on Windows). The interpreter must be installed. */ shell?: "bash" | "powershell" /** @@ -255,34 +255,6 @@ export interface HookJSONOutput { hookSpecificOutput?: HookSpecificOutput } -// Loose flat zod schema mirroring HookJSONOutput — used by the prompt handler to -// constrain LLM structured output. Intentionally NOT `.strict()`: lets the model -// emit unknown fields without failing parse. Single source of truth lives next -// to the HookJSONOutput interface; not exported (settings.ts internal only). -const HookSpecificOutputZodSchema = z.object({ - hookEventName: z.string().optional(), - permissionDecision: z.enum(["allow", "deny", "ask"]).optional(), - permissionDecisionReason: z.string().optional(), - updatedInput: z.record(z.string(), z.unknown()).optional(), - additionalContext: z.string().optional(), - initialUserMessage: z.string().optional(), - updatedMCPToolOutput: z.unknown().optional(), - watchPaths: z.array(z.string()).optional(), - displayMessage: z.string().optional(), - compactSummary: z.string().optional(), - customSummary: z.string().optional(), -}) - -const HookJSONOutputZodSchema = z.object({ - continue: z.boolean().optional(), - stopReason: z.string().optional(), - suppressOutput: z.boolean().optional(), - systemMessage: z.string().optional(), - decision: z.enum(["approve", "block"]).optional(), - reason: z.string().optional(), - hookSpecificOutput: HookSpecificOutputZodSchema.optional(), -}) - // ── Async rewake (hook-async-rewake) ─────────────────────────── // Sentinel prefix for rewake prompts. UserPromptSubmit hook processing skips // prompts whose text starts with this prefix, preventing hook → rewake → hook @@ -514,11 +486,7 @@ export interface ForkHooks { * INTENT: Centralized pre-dispatch filtering for condition-filter, * future rate-limiting, logging, etc. */ - readonly beforeRunEntry?: ( - entry: HookCommand, - envelope: Record<string, unknown>, - event: HookEvent, - ) => boolean + readonly beforeRunEntry?: (entry: HookCommand, envelope: Record<string, unknown>, event: HookEvent) => boolean /** * Called AFTER runEntry() for each executed hook entry. @@ -593,6 +561,20 @@ function httpUrl(entry: HookCommand): string { return entry.url ?? entry.command ?? "" } +function httpHeaders(entry: HookCommand): Record<string, string> { + if (entry.allowedEnvVars === undefined) return entry.headers ?? {} + const allowed = new Set(entry.allowedEnvVars ?? []) + return Object.fromEntries( + Object.entries(entry.headers ?? {}).map(([name, value]) => [ + name, + value.replace(/\$\{([A-Za-z_][A-Za-z0-9_]*)\}|\$([A-Za-z_][A-Za-z0-9_]*)/g, (_match, braced, bare) => { + const key = braced ?? bare + return allowed.has(key) ? (process.env[key] ?? "") : "" + }), + ]), + ) +} + function promptText(entry: HookCommand): string { return entry.prompt ?? entry.command ?? "" } @@ -659,35 +641,44 @@ export function readJSON(filepath: string): Settings | null { // hooks.json uses top-level event keys; a legacy {"hooks": {...}} wrapper is // tolerated (D1 graceful degradation). The wrapper wins when present. const obj = - parsed && typeof parsed === "object" && !Array.isArray(parsed) - ? (parsed as Record<string, unknown>) - : undefined - const rawHooks = obj && obj.hooks && typeof obj.hooks === "object" && !Array.isArray(obj.hooks) - ? obj.hooks as Record<string, unknown> - : obj - - // Filter to only valid HookEvent keys with array values (defends against - // non-event keys like "$schema" being treated as matchers) + parsed && typeof parsed === "object" && !Array.isArray(parsed) ? (parsed as Record<string, unknown>) : undefined + const rawHooks = + obj && obj.hooks && typeof obj.hooks === "object" && !Array.isArray(obj.hooks) + ? (obj.hooks as Record<string, unknown>) + : obj + const hooks: Settings["hooks"] = {} - if (rawHooks && typeof rawHooks === "object") { + const sourceDir = path.dirname(filepath) + if (rawHooks) { for (const [key, value] of Object.entries(rawHooks)) { - if (VALID_HOOK_EVENTS.has(key) && Array.isArray(value)) { - hooks[key as HookEvent] = value + if (!VALID_HOOK_EVENTS.has(key)) continue + if (!Array.isArray(value)) { + log.warn("invalid hook event configuration", { path: filepath, event: key }) + continue } - } - } - - // Stamp every HookCommand with the directory of the hooks.json file that - // declared it. execShell uses this to populate CLAUDE_PLUGIN_ROOT / - // CLAUDE_PLUGIN_DATA — now resolves to .opencode/ or ~/.config/opencode/ - // rather than .claude/. - const sourceDir = path.dirname(filepath) - if (hooks) { - for (const matchers of Object.values(hooks)) { - if (!matchers) continue - for (const m of matchers) { - for (const h of m.hooks ?? []) h.__sourceDir = sourceDir + const groups: HookMatcher[] = [] + for (const group of value) { + if ( + !group || + typeof group !== "object" || + !Array.isArray(group.hooks) || + (group.matcher !== undefined && typeof group.matcher !== "string") + ) { + log.warn("invalid hook matcher skipped", { path: filepath, event: key }) + continue + } + const commands: HookCommand[] = [] + for (const candidate of group.hooks) { + const parsed = HookCommandSchema.safeParse(candidate) + if (!parsed.success) { + log.warn("invalid hook command skipped", { path: filepath, event: key, error: parsed.error.message }) + continue + } + commands.push({ ...parsed.data, __sourceDir: sourceDir }) + } + if (commands.length) groups.push({ matcher: group.matcher, hooks: commands }) } + hooks[key as HookEvent] = groups } } log.info("loaded hook settings", { @@ -952,35 +943,18 @@ export function __resetDeprecatedWarnings(): void { warnedDeprecatedHooks.clear() } -/** - * Pure detection of HookCommand fields the fork has not yet implemented (`shell`). - * Exported for unit testing. `if` is fully implemented via condition-filter - * (`extensions/condition-filter.ts` evaluates it in `ForkHooks.beforeRunEntry`), - * and `async` / `asyncRewake` are fully implemented (hook-async-execution); all - * three are therefore excluded. `shell` has no runtime handler and MUST be flagged. - */ +/** Diagnose fields that have no effect for the selected handler type. */ export function detectUnsupportedFields( hooks: Settings["hooks"], ): Array<{ field: string; value: unknown; eventName: string }> { - if (!hooks) return [] const unsupported: Array<{ field: string; value: unknown; eventName: string }> = [] - for (const [eventName, matchers] of Object.entries(hooks)) { - if (!matchers) continue - for (const m of matchers) { - for (const h of m.hooks ?? []) { - if (h.shell !== undefined) unsupported.push({ field: "shell", value: h.shell, eventName }) - // issue #286 — schema-accepted but executor-dropped fields. Surfaced - // here instead of silently swallowed: allowedEnvVars/statusMessage have - // zero consumers anywhere; per-command `once` is never read (only the - // entry-level _sessionEntry?.once is consumed). `timeout` is NOT - // flagged — every handler type applies it (incl. prompt). - if (h.allowedEnvVars !== undefined) - unsupported.push({ field: "allowedEnvVars", value: h.allowedEnvVars, eventName }) - if (h.statusMessage !== undefined) - unsupported.push({ field: "statusMessage", value: h.statusMessage, eventName }) - if (h.once !== undefined) unsupported.push({ field: "once", value: h.once, eventName }) - // `if` is implemented (condition-filter); async/asyncRewake implemented. - // All 5 known types now have handlers; type-level unsupported set is empty by design. + for (const [eventName, matchers] of Object.entries(hooks ?? {})) { + for (const matcher of matchers ?? []) { + for (const entry of matcher.hooks) { + if (entry.shell !== undefined && entry.type !== "command") + unsupported.push({ field: "shell", value: entry.shell, eventName }) + if (entry.allowedEnvVars !== undefined && entry.type !== "http") + unsupported.push({ field: "allowedEnvVars", value: entry.allowedEnvVars, eventName }) } } } @@ -988,18 +962,12 @@ export function detectUnsupportedFields( } /** - * Internal: scan loaded settings for HookCommand fields the fork has not yet implemented - * (`shell`) and emit a single `log.warn` per settings file. Runtime still proceeds — - * this field is silently ignored. Exported for unit testing only; not part of the public - * surface. `if` / `async` / `asyncRewake` are fully implemented and excluded. + * Warn about fields supplied to a handler type that cannot use them. */ -export function warnUnsupportedFields( - hooks: Settings["hooks"], - sourceDir: string, -): void { +export function warnUnsupportedFields(hooks: Settings["hooks"], sourceDir: string): void { const unsupported = detectUnsupportedFields(hooks) if (unsupported.length > 0) { - log.warn("hook settings contains unsupported fields (will be ignored or fail at runtime)", { + log.warn("hook settings contains fields ignored by this handler type", { sourceDir, unsupported, }) @@ -1020,10 +988,26 @@ function execShell( entry: HookCommand, stdinJSON: string, cwd: string, + signal: AbortSignal, ): Promise<{ exitCode: number | null; stdout: string; stderr: string; spawnError?: string }> { return new Promise((resolve) => { + if (signal.aborted) { + resolve({ exitCode: null, stdout: "", stderr: "" }) + return + } const timeoutMs = entry.timeout ? entry.timeout * 1000 : DEFAULT_TIMEOUT_MS - const shell = process.platform === "win32" ? true : "/bin/sh" + const shell = + entry.shell === "powershell" + ? process.platform === "win32" + ? "powershell.exe" + : "pwsh" + : entry.shell === "bash" + ? process.platform === "win32" + ? "bash.exe" + : "/bin/bash" + : process.platform === "win32" + ? true + : "/bin/sh" const expandedCommand = expandCommand(entry) const command = commandText(entry) @@ -1066,7 +1050,6 @@ function execShell( shell, env: { ...process.env, ...extraEnv }, stdio: ["pipe", "pipe", "pipe"], - timeout: timeoutMs, detached: process.platform !== "win32", }) @@ -1095,6 +1078,7 @@ function execShell( // awaited as independent conditions, with a process-group SIGKILL as the // fallback that guarantees resolution. let exitCode: number | null = null + let timedOut = false let settled = false const timers = new Set<NodeJS.Timeout>() const arm = (fire: () => void, ms: number) => { @@ -1122,6 +1106,7 @@ function execShell( const finish = (spawnError?: string) => { if (settled) return settled = true + signal.removeEventListener("abort", afterKill) for (const timer of timers) clearTimeout(timer) timers.clear() child.stdout.destroy() @@ -1132,7 +1117,10 @@ function execShell( stdoutLen: stdout.length, stderrLen: stderr.length, }) - resolve(spawnError === undefined ? { exitCode, stdout, stderr } : { exitCode, stdout, stderr, spawnError }) + const code = timedOut || signal.aborted ? null : exitCode + resolve( + spawnError === undefined ? { exitCode: code, stdout, stderr } : { exitCode: code, stdout, stderr, spawnError }, + ) } child.on("error", (err) => { @@ -1154,6 +1142,9 @@ function execShell( void Promise.all([exited, Promise.race([streamsDone, drained])]).then(() => finish()) } + signal.addEventListener("abort", afterKill, { once: true }) + if (signal.aborted) afterKill() + void Promise.all([exited, streamsDone]).then(() => finish()) // Child exited but pipes are still open (grandchild holds them): kill the @@ -1162,7 +1153,14 @@ function execShell( // Child ignored the spawn-timeout SIGTERM and never exited: kill the group // at the absolute deadline, wait for the reap, then resolve. - arm(afterKill, timeoutMs + KILL_GRACE_MS) + arm(() => { + timedOut = true + if (child.pid !== undefined) + void Process.killGroupPid(child.pid, "SIGTERM").catch((error) => { + log.warn("hook process-group termination failed", { error: String(error) }) + }) + arm(afterKill, KILL_GRACE_MS) + }, timeoutMs) }) } @@ -1173,7 +1171,12 @@ function parseStdout(stdout: string, command: string): HookJSONOutput | undefine return undefined } try { - return JSON.parse(trimmed) as HookJSONOutput + const parsed = HookOutputSchema.safeParse(JSON.parse(trimmed)) + if (!parsed.success) { + log.warn("hook returned invalid output shape", { command, error: parsed.error.message }) + return undefined + } + return parsed.data as HookJSONOutput } catch { log.warn("hook returned invalid JSON", { command, output: trimmed.slice(0, 200) }) return undefined @@ -1250,18 +1253,14 @@ function buildStdinEnvelope(payload: HookPayload, ctx: TriggerContext, cwd: stri return { ...base, stop_hook_active: payload.stopHookActive, - ...(payload.lastAssistantMessage !== undefined - ? { last_assistant_message: payload.lastAssistantMessage } - : {}), + ...(payload.lastAssistantMessage !== undefined ? { last_assistant_message: payload.lastAssistantMessage } : {}), } case "StopFailure": return { ...base, stop_hook_active: payload.stopHookActive, error: payload.error, - ...(payload.lastAssistantMessage !== undefined - ? { last_assistant_message: payload.lastAssistantMessage } - : {}), + ...(payload.lastAssistantMessage !== undefined ? { last_assistant_message: payload.lastAssistantMessage } : {}), } case "SubagentStart": return { ...base, agent_id: payload.agentID, agent_type: payload.agentType } @@ -1270,13 +1269,9 @@ function buildStdinEnvelope(payload: HookPayload, ctx: TriggerContext, cwd: stri ...base, stop_hook_active: payload.stopHookActive, ...(payload.agentID !== undefined ? { agent_id: payload.agentID } : {}), - ...(payload.agentTranscriptPath !== undefined - ? { agent_transcript_path: payload.agentTranscriptPath } - : {}), + ...(payload.agentTranscriptPath !== undefined ? { agent_transcript_path: payload.agentTranscriptPath } : {}), ...(payload.agentType !== undefined ? { agent_type: payload.agentType } : {}), - ...(payload.lastAssistantMessage !== undefined - ? { last_assistant_message: payload.lastAssistantMessage } - : {}), + ...(payload.lastAssistantMessage !== undefined ? { last_assistant_message: payload.lastAssistantMessage } : {}), } case "PreCompact": return { @@ -1289,9 +1284,7 @@ function buildStdinEnvelope(payload: HookPayload, ctx: TriggerContext, cwd: stri ...base, ...(payload.trigger !== undefined ? { trigger: payload.trigger } : {}), ...(payload.compactSummary !== undefined ? { compact_summary: payload.compactSummary } : {}), - ...(payload.customInstructions !== undefined - ? { custom_instructions: payload.customInstructions } - : {}), + ...(payload.customInstructions !== undefined ? { custom_instructions: payload.customInstructions } : {}), } case "SessionStart": return { @@ -1396,6 +1389,7 @@ interface State { * the "" bucket, preserving the prior global-dedup behavior for those. */ seen: Map<string, Set<string>> + once: Map<string, WeakSet<HookCommand>> /** * Scope-tagged summaries of the currently-effective hooks, computed by * `summarizeChain` alongside `settings` (same closure, same hot-reload @@ -1405,10 +1399,7 @@ interface State { } export interface Interface { - readonly trigger: ( - payload: HookPayload, - ctx: TriggerContext, - ) => Effect.Effect<TriggerResult> + readonly trigger: (payload: HookPayload, ctx: TriggerContext) => Effect.Effect<TriggerResult> /** * Read-only view of the currently-effective hooks (merged global + project + * worktree chain), one entry per hook command tagged with its source layer. @@ -1450,8 +1441,8 @@ const commandHandler: HookHandler = { type: "command", run: Effect.fn("SettingsHook.handler.command")(function* (entry, envelope, cwd, _inHook) { const stdinJSON = JSON.stringify(envelope) - const { exitCode, stdout, stderr, spawnError } = yield* Effect.promise(() => - execShell(entry, stdinJSON, cwd), + const { exitCode, stdout, stderr, spawnError } = yield* Effect.promise((signal) => + execShell(entry, stdinJSON, cwd, signal), ) if (spawnError) { @@ -1461,7 +1452,7 @@ const commandHandler: HookHandler = { // Exit-code 2: block + stderr-as-reason (CC contract) if (exitCode === 2) { const reason = stderr.trim() || "Hook blocked execution" - return { json: parseStdout(stdout, commandText(entry)), exitBlock: reason, rawStdout: stdout, exitCode } + return { exitBlock: reason, exitCode } } // Other non-zero exits: log and continue (do not abort main flow) @@ -1483,7 +1474,9 @@ const commandHandler: HookHandler = { // trigger aggregator can inject it as additionalContext for // UserPromptSubmit / SessionStart (CC protocol). JSON stdout still parses // normally via parseStdout; rawStdout is only consumed when json is null. - return { json: parseStdout(stdout, commandText(entry)), exitBlock: undefined, rawStdout: stdout, exitCode } + return exitCode === 0 + ? { json: parseStdout(stdout, commandText(entry)), rawStdout: stdout, exitCode } + : { exitCode } }), } @@ -1496,10 +1489,7 @@ const mcpHandler: HookHandler = { return { json: undefined, exitBlock: undefined } } const timeoutMs = entry.timeout ? entry.timeout * 1000 : DEFAULT_TIMEOUT_MS - const exit = yield* invokeMcpHook(mcpSvc, commandText(entry), envelope).pipe( - Effect.timeout(timeoutMs), - Effect.exit, - ) + const exit = yield* invokeMcpHook(mcpSvc, commandText(entry), envelope).pipe(Effect.timeout(timeoutMs), Effect.exit) if (exit._tag === "Failure") { log.warn("mcp hook timed out or failed (non-blocking)", { command: commandText(entry), @@ -1536,7 +1526,7 @@ const httpHandler: HookHandler = { const url = httpUrl(entry) const exit = yield* HttpClientRequest.post(url).pipe( - HttpClientRequest.setHeaders(entry.headers ?? {}), + HttpClientRequest.setHeaders(httpHeaders(entry)), HttpClientRequest.bodyJson(envelope), Effect.flatMap((req) => httpRead.execute(req)), Effect.flatMap((res) => @@ -1571,7 +1561,7 @@ const httpHandler: HookHandler = { * * `entry.command` is interpreted as the system prompt template; the stdin envelope * (already shaped by buildStdinEnvelope) is JSON-stringified into the user message. - * The model returns structured output matching HookJSONOutputZodSchema (loose flat + * The model returns structured output matching HookOutputSchema (loose flat * shape; see definition near HookJSONOutput). * * Failure policy is **silent allow** — mirrors httpHandler's network-error path: @@ -1615,7 +1605,7 @@ const promptHandler: HookHandler = { { role: "system", content: prompt } as ModelMessage, { role: "user", content: JSON.stringify(envelope) } as ModelMessage, ], - schema: HookJSONOutputZodSchema, + schema: HookOutputSchema, } satisfies Parameters<typeof generateObject>[0] // issue #286 — the header doc promises `timeout` for every hook type, @@ -1625,12 +1615,9 @@ const promptHandler: HookHandler = { const timeoutMs = entry.timeout ? entry.timeout * 1000 : DEFAULT_TIMEOUT_MS const llmExit = yield* Effect.tryPromise({ - try: () => generateObject(params).then((r) => r.object), + try: (abortSignal) => generateObject({ ...params, abortSignal }).then((r) => r.object), catch: (e) => e, - }).pipe( - Effect.timeout(timeoutMs), - Effect.exit, - ) + }).pipe(Effect.timeout(timeoutMs), Effect.exit) if (llmExit._tag === "Failure") { log.warn("prompt hook failed (non-blocking)", { error: String(llmExit.cause) }) @@ -1690,45 +1677,40 @@ const agentHandler: HookHandler = { } const captured: { value: HookJSONOutput | null } = { value: null } - const ac = new AbortController() const timeoutMs = entry.timeout ? entry.timeout * 1000 : DEFAULT_AGENT_TIMEOUT_MS - const timer = setTimeout(() => ac.abort(), timeoutMs) const loopExit = yield* Effect.tryPromise({ - try: async () => { - try { - const tools = buildAgentTools({ spawner, fs, signal: ac.signal, cwd, captured }) - const messages: ModelMessage[] = [ - { role: "system", content: prompt }, - { role: "user", content: JSON.stringify(envelope) }, - ] - for (let turn = 0; turn < MAX_AGENT_TURNS; turn++) { - const result = await generateText({ - model: language, - messages, - tools, - toolChoice: "auto", - abortSignal: ac.signal, - maxOutputTokens: 4096, - allowSystemInMessages: true, - } as any) - if (captured.value) return captured.value - messages.push(...result.response.messages) - if ( - result.finishReason === "stop" || - result.finishReason === "length" || - result.finishReason === "content-filter" - ) - break - if (result.toolCalls.length === 0) break - } - return null - } finally { - clearTimeout(timer) + try: async (signal) => { + const tools = buildAgentTools({ spawner, fs, signal, cwd, captured }) + const messages: ModelMessage[] = [ + { role: "system", content: prompt }, + { role: "user", content: JSON.stringify(envelope) }, + ] + for (let turn = 0; turn < MAX_AGENT_TURNS; turn++) { + signal.throwIfAborted() + const result = await generateText({ + model: language, + messages, + tools, + toolChoice: "auto", + abortSignal: signal, + maxOutputTokens: 4096, + allowSystemInMessages: true, + } as any) + if (captured.value) return captured.value + messages.push(...result.response.messages) + if ( + result.finishReason === "stop" || + result.finishReason === "length" || + result.finishReason === "content-filter" + ) + break + if (result.toolCalls.length === 0) break } + return null }, catch: (e) => e, - }).pipe(Effect.exit) + }).pipe(Effect.timeout(timeoutMs), Effect.exit) if (loopExit._tag === "Failure") { const cause = String(loopExit.cause) @@ -1804,6 +1786,14 @@ export const layer = Layer.effect( Service, Effect.gen(function* () { const sessionHooks = yield* SessionHooks.Service + const handlerContext = Context.pick( + MCP.Service, + Provider.Service, + Auth.Service, + FSUtil.Service, + ChildProcessSpawner, + HttpClient.HttpClient, + )(yield* Effect.context<never>()) const state = yield* InstanceState.make( Effect.fn("SettingsHook.state")(function* (instCtx) { @@ -1813,6 +1803,7 @@ export const layer = Layer.effect( hooksList: chain.summaries, cwd: instCtx.directory, seen: new Map<string, Set<string>>(), + once: new Map<string, WeakSet<HookCommand>>(), } satisfies State // [FORK:hook-ext] Hot-reload settings files at runtime. The watcher @@ -1833,11 +1824,12 @@ export const layer = Layer.effect( const handle = watchSettings( instCtx.directory, instCtx.worktree, - () => Effect.sync(() => { - const reloaded = loadChainWithSummaries(instCtx.directory, instCtx.worktree) - lastSummaries = reloaded.summaries - return reloaded.settings - }), + () => + Effect.sync(() => { + const reloaded = loadChainWithSummaries(instCtx.directory, instCtx.worktree) + lastSummaries = reloaded.summaries + return reloaded.settings + }), (newSettings, changedFile) => { stateObj.settings = newSettings stateObj.hooksList = lastSummaries @@ -1870,9 +1862,7 @@ export const layer = Layer.effect( // [FORK:hook-ext] Assemble fork middleware — wired from hook/extensions/index.ts // When undefined, trigger() behaves identically to upstream. - const forkHooks: ForkHooks | undefined = buildForkHooks - ? buildForkHooks({ sessionHooks }) - : undefined + const forkHooks: ForkHooks | undefined = buildForkHooks ? buildForkHooks({ sessionHooks }) : undefined /** * Execute a single hook entry. Never throws. Returns the parsed JSON @@ -1898,7 +1888,23 @@ export const layer = Layer.effect( exitCode: undefined as number | null | undefined, } } - return yield* handler.run(entry as never, envelope, cwd, inHook) + const result = yield* handler + .run(entry as never, envelope, cwd, inHook) + .pipe(Effect.updateContext((current: Context.Context<never>) => Context.merge(handlerContext, current))) + const hso = result.json?.hookSpecificOutput + if (hso) { + for (const field of [ + "initialUserMessage", + "watchPaths", + "updatedMCPToolOutput", + "displayMessage", + "compactSummary", + "customSummary", + ] as const) { + if (field in hso) log.warn("hook output field is not supported", { event: envelope.hook_event_name, field }) + } + } + return result }) // Background scope for async hooks. Lives as long as the SettingsHook service @@ -1934,7 +1940,9 @@ export const layer = Layer.effect( return } if (!hookRewake) { - log.warn("async hook rewake skipped: HookRewake.Service unavailable", { command: commandText(entry).slice(0, 80) }) + log.warn("async hook rewake skipped: HookRewake.Service unavailable", { + command: commandText(entry).slice(0, 80), + }) return } @@ -1955,7 +1963,7 @@ export const layer = Layer.effect( (event === "UserPromptSubmit" || event === "SessionStart") ) { const text = result.rawStdout.trim() - if (text) parts.push(text) + if (text && !text.startsWith("{")) parts.push(text) } if (parts.length === 0) { log.debug("async hook completed: nothing rewake-worthy", { command: commandText(entry).slice(0, 80) }) @@ -1965,16 +1973,16 @@ export const layer = Layer.effect( const text = buildRewakePrompt(entry, event, parts.join("\n")) yield* hookRewake.rewake({ sessionID: SessionID.make(sessionID), text }).pipe( Effect.catchDefect((defect) => { - log.warn("async hook rewake defect swallowed", { command: commandText(entry).slice(0, 80), error: String(defect) }) + log.warn("async hook rewake defect swallowed", { + command: commandText(entry).slice(0, 80), + error: String(defect), + }) return Effect.void }), ) }) - const trigger = Effect.fn("SettingsHook.trigger")(function* ( - payload: HookPayload, - ctx: TriggerContext, - ) { + const trigger = Effect.fn("SettingsHook.trigger")(function* (payload: HookPayload, ctx: TriggerContext) { using _ = log.time("trigger", { event: payload.event, sessionID: ctx.sessionID }) const s = yield* InstanceState.get(state) const result: TriggerResult = { additionalContexts: [], systemMessages: [] } @@ -1995,6 +2003,7 @@ export const layer = Layer.effect( // session, which is harmless. if (payload.event === "SessionEnd" && ctx.sessionID) { s.seen.delete(ctx.sessionID) + s.once.delete(ctx.sessionID) // NOTE: sessionHooks.clear is deferred to after hook execution // (before each return point below) — clearing here would remove // session-registered SessionEnd hooks before the matcher can see them. @@ -2007,8 +2016,7 @@ export const layer = Layer.effect( // s.settings is already cached on the InstanceState, so the file-side // probe is a property access. The session probe is O(1) (Map.get + // .some over the session's own array, typically empty). - const sessionEvent: HookEvent = - ctx.isSubAgent && payload.event === "Stop" ? "SubagentStop" : payload.event + const sessionEvent: HookEvent = ctx.isSubAgent && payload.event === "Stop" ? "SubagentStop" : payload.event const hasFile = (s.settings.hooks?.[payload.event]?.length ?? 0) > 0 const hasSession = ctx.sessionID ? yield* sessionHooks.hasForEvent(SessionID.make(ctx.sessionID), sessionEvent) @@ -2017,6 +2025,8 @@ export const layer = Layer.effect( log.info("trigger short-circuit", { event: payload.event, reason: "no_matchers", hasFile, hasSession }) if (payload.event === "SessionEnd" && ctx.sessionID) { yield* sessionHooks.clear(SessionID.make(ctx.sessionID)) + s.seen.delete(ctx.sessionID) + s.once.delete(ctx.sessionID) } return result } @@ -2029,8 +2039,7 @@ export const layer = Layer.effect( // A layer may declare `allowUntrusted: true` to opt out of the gate. NEVER // deny / throw — a trust gate that throws becomes a denial vector. Default // (enforcement off) is byte-for-byte the prior behavior (zero gate). - const requireTrust = - s.settings.requireTrust === true || process.env.OPENCODE_HOOKS_REQUIRE_TRUST === "1" + const requireTrust = s.settings.requireTrust === true || process.env.OPENCODE_HOOKS_REQUIRE_TRUST === "1" if (requireTrust && !isTrusted(s.cwd) && s.settings.allowUntrusted !== true) { log.warn("hooks skipped: workspace not trusted", { cwd: s.cwd, @@ -2043,6 +2052,8 @@ export const layer = Layer.effect( // leaked in memory for the process lifetime. if (payload.event === "SessionEnd" && ctx.sessionID) { yield* sessionHooks.clear(SessionID.make(ctx.sessionID)) + s.seen.delete(ctx.sessionID) + s.once.delete(ctx.sessionID) } return result } @@ -2075,6 +2086,8 @@ export const layer = Layer.effect( log.info("trigger short-circuit", { event: payload.event, reason: "empty_matchers" }) if (payload.event === "SessionEnd" && ctx.sessionID) { yield* sessionHooks.clear(SessionID.make(ctx.sessionID)) + s.seen.delete(ctx.sessionID) + s.once.delete(ctx.sessionID) } return result } @@ -2085,6 +2098,7 @@ export const layer = Layer.effect( for (const group of matchers) { if (!matches(group.matcher, target)) continue + let claimed = false for (const entry of group.hooks) { // Forward-compat: skip truly unknown types so future schema additions don't crash // older handlers. Known types (command/mcp/http/prompt/agent) all flow into runEntry. @@ -2100,15 +2114,25 @@ export const layer = Layer.effect( // [FORK:hook-ext] Pre-dispatch filter — skip entry if condition not met if (forkHooks?.beforeRunEntry && !forkHooks.beforeRunEntry(entry, envelope, payload.event)) continue + const onceBucket = s.once.get(ctx.sessionID) ?? new WeakSet<HookCommand>() + if (entry.once && onceBucket.has(entry)) continue + + if (group._sessionEntry?.once && ctx.sessionID && !claimed) { + if (!(yield* sessionHooks.claim(SessionID.make(ctx.sessionID), group._sessionEntry.id))) break + claimed = true + } + if (entry.once) { + onceBucket.add(entry) + s.once.set(ctx.sessionID, onceBucket) + } + if (entry.statusMessage) log.info("hook status", { event: payload.event, message: entry.statusMessage }) + // ── Async fork (hook-async-rewake) ────────────────────── // async:true entries are forked into the background and do NOT // participate in the current TriggerResult aggregation. Their output // (when asyncRewake:true) is delivered back via onAsyncComplete → // Session.rewake once the background fiber settles. if (entry.async) { - if (group._sessionEntry?.once && ctx.sessionID) { - yield* sessionHooks.remove(SessionID.make(ctx.sessionID), group._sessionEntry.id) - } const hookRewake = Option.getOrUndefined(yield* Effect.serviceOption(HookRewake.Service)) const capturedEvent = payload.event const capturedSessionID = ctx.sessionID @@ -2152,7 +2176,12 @@ export const layer = Layer.effect( command: commandText(entry), error: String(defect), }) - return Effect.succeed({ json: undefined, exitBlock: undefined, rawStdout: undefined, exitCode: undefined }) + return Effect.succeed({ + json: undefined, + exitBlock: undefined, + rawStdout: undefined, + exitCode: undefined, + }) }), ) @@ -2176,14 +2205,10 @@ export const layer = Layer.effect( (payload.event === "UserPromptSubmit" || payload.event === "SessionStart") ) { const text = rawStdout.trim() - if (text && addSeen(s.seen, ctx.sessionID, text)) { + if (text && !text.startsWith("{") && addSeen(s.seen, ctx.sessionID, text)) { result.additionalContexts.push(text) } } - // once: true entries are cleared after running, regardless of result. - if (group._sessionEntry?.once && ctx.sessionID) { - yield* sessionHooks.remove(SessionID.make(ctx.sessionID), group._sessionEntry.id) - } continue } @@ -2217,9 +2242,7 @@ export const layer = Layer.effect( // Most-restrictive-wins: deny > ask > allow. A later hook cannot // relax an earlier hook's deny (Claude Code permission semantics). const moreRestrictive = - current === undefined || - incoming === "deny" || - (incoming === "ask" && current === "allow") + current === undefined || incoming === "deny" || (incoming === "ask" && current === "allow") if (moreRestrictive) { result.permissionDecision = incoming result.permissionDecisionReason = @@ -2230,12 +2253,6 @@ export const layer = Layer.effect( result.updatedInput = hso.updatedInput } - // once: true cleanup — runs after aggregating this entry's json so - // additionalContext etc. still surface on the first (and only) firing. - if (group._sessionEntry?.once && ctx.sessionID) { - yield* sessionHooks.remove(SessionID.make(ctx.sessionID), group._sessionEntry.id) - } - // CC contract: continue=false short-circuits remaining hooks in this // matcher (and below, in subsequent matchers). Aggregation for the // current entry's json has already happened above — break only after. @@ -2246,6 +2263,8 @@ export const layer = Layer.effect( if (payload.event === "SessionEnd" && ctx.sessionID) { yield* sessionHooks.clear(SessionID.make(ctx.sessionID)) + s.seen.delete(ctx.sessionID) + s.once.delete(ctx.sessionID) } return result }) @@ -2259,12 +2278,16 @@ export const layer = Layer.effect( }), ) -// Only provide deps needed at layer construction (SessionHooks — the sole -// service yielded in the layer body). Handler deps (MCP/Provider/Auth/FSUtil/ -// HttpClient/CrossSpawnSpawner/HookRewake) are resolved lazily at trigger time -// from whatever ambient context the Effect runs in. -export const defaultLayer = layer.pipe( - Layer.provide(SessionHooks.defaultLayer), +export const defaultLayer = Layer.suspend(() => + layer.pipe( + Layer.provide(SessionHooks.defaultLayer), + Layer.provide(MCP.defaultLayer), + Layer.provide(Provider.defaultLayer), + Layer.provide(Auth.defaultLayer), + Layer.provide(FSUtil.defaultLayer), + Layer.provide(CrossSpawnSpawner.defaultLayer), + Layer.provide(FetchHttpClient.layer), + ), ) // ── type:"mcp" hook execution ─────────────────────────────────── @@ -2285,11 +2308,7 @@ export const defaultLayer = layer.pipe( * tool with the hook envelope as `arguments`. Parses the first text content * item as JSON to obtain the standard hook control output. */ -function invokeMcpHook( - mcpSvc: MCP.Interface, - command: string, - envelope: Record<string, unknown>, -) { +function invokeMcpHook(mcpSvc: MCP.Interface, command: string, envelope: Record<string, unknown>) { return Effect.gen(function* () { if (!command.startsWith("mcp__")) { log.warn("mcp hook command must start with mcp__", { command }) @@ -2321,13 +2340,16 @@ function invokeMcpHook( return undefined } - const result = yield* Effect.promise(() => + const result = yield* Effect.promise((signal) => Promise.resolve( - tool.execute!(envelope as never, { - toolCallId: `hook-${Date.now()}`, - messages: [], - abortSignal: new AbortController().signal, - } as never), + tool.execute!( + envelope as never, + { + toolCallId: `hook-${Date.now()}`, + messages: [], + abortSignal: signal, + } as never, + ), ).catch((err) => { log.warn("mcp hook execution threw", { command, error: String(err) }) return undefined @@ -2337,13 +2359,29 @@ function invokeMcpHook( if (!result || typeof result !== "object" || !("content" in result)) return undefined const content = (result as { content: Array<{ type: string; text?: string }> }).content - const firstText = content.find((c) => c.type === "text" && typeof c.text === "string")?.text + if (!Array.isArray(content)) return undefined + const firstText = content.find((c) => c && c.type === "text" && typeof c.text === "string")?.text if (!firstText) return undefined return parseStdout(firstText, command) }) } -export const node = LayerNode.make(layer, [SessionHooks.node]) +// Resolve module references after initialization; provider/plugin imports can +// otherwise encounter this module while their own node exports are in the TDZ. +export const node = { + ...LayerNode.make(layer, [SessionHooks.node]), + get dependencies() { + return [ + SessionHooks.node, + MCP.node, + Provider.node, + Auth.node, + FSUtil.node, + CrossSpawnSpawner.node, + LayerNode.make(FetchHttpClient.layer, []), + ] + }, +} export * as SettingsHook from "./settings" diff --git a/packages/opencode/src/hook/trigger-result.ts b/packages/opencode/src/hook/trigger-result.ts index dae152bdae..b5eef5c7a8 100644 --- a/packages/opencode/src/hook/trigger-result.ts +++ b/packages/opencode/src/hook/trigger-result.ts @@ -33,6 +33,27 @@ export interface TriggerResult { updatedInput?: Record<string, unknown> } +/** Post hooks run after the side effect; report validation feedback to the model. */ +export function postHookFeedback(result: TriggerResult): string { + return [ + ...(result.additionalContexts ?? []), + ...(result.systemMessages ?? []), + ...(result.blocked ? [`[Post-tool hook blocked] ${result.blocked.reason}`] : []), + ...(result.preventContinuation ? [`[Hook stopped] ${result.stopReason ?? "Hook requested stop"}`] : []), + ].join("\n\n") +} + +export function withHookFeedback(output: string, result: TriggerResult): string { + const feedback = postHookFeedback(result) + return feedback ? `${output}\n\n${feedback}` : output +} + +export function withHookFailure(error: unknown, result: TriggerResult): unknown { + const feedback = postHookFeedback(result) + if (!feedback) return error + return new Error(`${error instanceof Error ? error.message : String(error)}\n\n${feedback}`, { cause: error }) +} + /** * Land a TriggerResult's `systemMessages` so they are never silently dropped. * diff --git a/packages/opencode/src/permission/index.ts b/packages/opencode/src/permission/index.ts index fa066d5a33..48afecf29e 100644 --- a/packages/opencode/src/permission/index.ts +++ b/packages/opencode/src/permission/index.ts @@ -7,6 +7,7 @@ import * as Option from "effect/Option" import os from "os" import { PermissionV1 } from "@opencode-ai/core/v1/permission" import { EventV2Bridge } from "@/event-v2-bridge" +import { type TriggerResult } from "@/hook/trigger-result" import { SettingsHook } from "@/hook/settings" import { Notification } from "@/notification" import { PermissionV1Event } from "@opencode-ai/schema/permission-v1" @@ -121,40 +122,34 @@ export const layer = Layer.effect( } yield* Effect.logInfo("asking", { id, permission: info.permission, patterns: info.patterns }) - const deferred = yield* Deferred.make<void, PermissionV1.RejectedError | PermissionV1.CorrectedError>() - pending.set(id, { info, deferred }) - let hookAutoDecided = false if (settingsHook) { - const hookResult = yield* settingsHook.trigger( - { - event: "PermissionRequest", - toolName: request.permission, - toolInput: { - permission: request.permission, - patterns: request.patterns, - metadata: request.metadata, - always: request.always, - }, - toolUseID: id, - } as any, - { sessionID: request.sessionID ?? "", transcriptPath: "" }, - ).pipe(Effect.catch(() => Effect.succeed({ permissionDecision: undefined, additionalContexts: [], systemMessages: [] } as any))) + const hookResult = yield* settingsHook + .trigger( + { + event: "PermissionRequest", + toolName: request.permission, + toolInput: { + permission: request.permission, + patterns: request.patterns, + metadata: request.metadata, + always: request.always, + }, + toolUseID: id, + } as any, + { sessionID: request.sessionID ?? "", transcriptPath: "" }, + ) + .pipe(Effect.catch(() => Effect.succeed({ additionalContexts: [], systemMessages: [] } as TriggerResult))) yield* SettingsHook.landSystemMessages(hookResult as any, { sessionID: request.sessionID ?? "" }) - // Auto-approve/deny based on hook decision - if ((hookResult as any).permissionDecision === "allow") { - hookAutoDecided = true - pending.delete(id) - yield* events.publish(Event.Asked, info) - yield* Deferred.succeed(deferred, undefined) - } else if ((hookResult as any).permissionDecision === "deny") { - hookAutoDecided = true - pending.delete(id) - yield* events.publish(Event.Asked, info) - yield* Deferred.fail(deferred, new PermissionV1.RejectedError({})) + if (hookResult.blocked || hookResult.preventContinuation || hookResult.permissionDecision === "deny") { + const reason = hookResult.blocked?.reason ?? hookResult.stopReason ?? hookResult.permissionDecisionReason + if (reason) return yield* new PermissionV1.CorrectedError({ feedback: `Permission hook: ${reason}` }) + return yield* new PermissionV1.RejectedError({}) } + if (hookResult.permissionDecision === "allow") return } - // Only publish Event.Asked if hook didn't already handle the decision - if (!hookAutoDecided) { + const deferred = yield* Deferred.make<void, PermissionV1.RejectedError | PermissionV1.CorrectedError>() + pending.set(id, { info, deferred }) + return yield* Effect.gen(function* () { yield* events.publish(Event.Asked, info) // Notification emitter — routes "agent needs attention" through the single // choke point (which fires the Notification hook). Resolved at call time so @@ -171,13 +166,8 @@ export const layer = Layer.effect( }) .pipe(Effect.ignore, Effect.forkIn(scope), Effect.asVoid) } - } - return yield* Effect.ensuring( - Deferred.await(deferred), - Effect.sync(() => { - pending.delete(id) - }), - ) + return yield* Deferred.await(deferred) + }).pipe(Effect.ensuring(Effect.sync(() => pending.delete(id)))) }) const reply = Effect.fn("Permission.reply")(function* (input: PermissionV1.ReplyInput) { @@ -289,6 +279,7 @@ export function disabled(tools: string[], ruleset: PermissionV1.Ruleset): Set<st export const defaultLayer = layer.pipe( Layer.provide(EventV2Bridge.defaultLayer), + Layer.provide(SettingsHook.defaultLayer), ) export const node = LayerNode.make(layer, [EventV2Bridge.node, SettingsHook.node, Notification.node]) diff --git a/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts b/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts index 2e15bf35a8..904da3293a 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts @@ -92,6 +92,11 @@ export const SessionHookCommandPayload = Schema.Struct({ if: Schema.optional(Schema.String), async: Schema.optional(Schema.Boolean), asyncRewake: Schema.optional(Schema.Boolean), + shell: Schema.optional(Schema.Literals(["bash", "powershell"])), + allowedEnvVars: Schema.optional(Schema.Array(Schema.String)), + statusMessage: Schema.optional(Schema.String), + once: Schema.optional(Schema.Boolean), + options: Schema.optional(Schema.Record(Schema.String, Schema.Unknown)), }) export const SessionHookAddPayload = Schema.Struct({ event: HookEventParam, diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts index 3c7cb06b0f..3e22950f59 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts @@ -491,12 +491,14 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", (hook.timeout !== undefined && hook.timeout <= 0), ) if (invalidEntry) return yield* new HttpApiError.BadRequest({}) - const id = yield* sessionHooks.add(ctx.params.sessionID, { - event: ctx.payload.event as HookEvent, - matcher: ctx.payload.matcher, - hooks: ctx.payload.hooks as SessionHookCommand[], - once: ctx.payload.once, - }) + const id = yield* sessionHooks + .add(ctx.params.sessionID, { + event: ctx.payload.event as HookEvent, + matcher: ctx.payload.matcher, + hooks: ctx.payload.hooks as SessionHookCommand[], + once: ctx.payload.once, + }) + .pipe(Effect.mapError(() => new HttpApiError.BadRequest({}))) return { id } }) diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 9074c71f20..540b9f3018 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -1,3 +1,4 @@ +import { withHookFeedback } from "@/hook/trigger-result" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { PermissionV1 } from "@opencode-ai/core/v1/permission" import path from "path" @@ -67,6 +68,7 @@ import { SettingsHook, HOOK_REWAKE_SENTINEL, type TriggerResult } from "@/hook/s import { applyPreHookDecision } from "@/hook/pre-hook-decision" import { dispatchTrust } from "@/hook/workspace-trust" import { HookStartContext } from "@/hook/start-context" +import { HookRewake } from "@/hook/rewake" import { Goal } from "@/goal/goal" import { KeyedMutex } from "@opencode-ai/core/effect/keyed-mutex" import { Memory } from "@/memory/memory" @@ -165,7 +167,18 @@ export const layer = Layer.effect( const flags = yield* RuntimeFlags.Service const database = yield* Database.Service const { db } = database - const settingsHook = Option.getOrUndefined(yield* Effect.serviceOption(SettingsHook.Service)) + const rawSettingsHook = Option.getOrUndefined(yield* Effect.serviceOption(SettingsHook.Service)) + const rewake = Context.make( + HookRewake.Service, + HookRewake.bind(({ sessionID, text }) => prompt({ sessionID, parts: [{ type: "text", text }] })), + ) + const settingsHook: SettingsHook.Interface | undefined = rawSettingsHook && { + ...rawSettingsHook, + trigger: (payload, context) => + rawSettingsHook + .trigger(payload, context) + .pipe(Effect.updateContext((current: Context.Context<never>) => Context.merge(rewake, current))), + } const startContext = Option.getOrUndefined(yield* Effect.serviceOption(HookStartContext.Service)) const goal = Option.getOrUndefined(yield* Effect.serviceOption(Goal.Service)) const promptLocks = KeyedMutex.makeUnsafe<SessionID>() @@ -350,24 +363,48 @@ export const layer = Layer.effect( // SettingsHook: PreToolUse for task tool if (settingsHook) { const preResult = yield* settingsHook - .trigger( - { event: "PreToolUse", toolName: TaskTool.id, toolInput: taskArgs, toolUseID: part.callID } as any, - { sessionID, transcriptPath: "" }, + .trigger({ event: "PreToolUse", toolName: TaskTool.id, toolInput: taskArgs, toolUseID: part.callID } as any, { + sessionID, + transcriptPath: "", + }) + .pipe( + Effect.catch(() => + Effect.succeed({ + blocked: undefined, + permissionDecision: undefined as "allow" | "deny" | "ask" | undefined, + permissionDecisionReason: undefined as string | undefined, + } as any), + ), ) - .pipe(Effect.catch(() => Effect.succeed({ blocked: undefined, permissionDecision: undefined as "allow" | "deny" | "ask" | undefined, permissionDecisionReason: undefined as string | undefined } as any))) yield* SettingsHook.landSystemMessages(preResult as TriggerResult, { sessionID }) const decision = applyPreHookDecision(taskArgs, preResult as any) // deny / blocked → error part if (decision.deniedReason) { if (part.state.status === "running") { - part = yield* sessions.updatePart({ ...part, state: { ...part.state, status: "error", error: decision.deniedReason, time: { ...part.state.time, end: Date.now() } } } satisfies SessionV1.ToolPart) + part = yield* sessions.updatePart({ + ...part, + state: { + ...part.state, + status: "error", + error: decision.deniedReason, + time: { ...part.state.time, end: Date.now() }, + }, + } satisfies SessionV1.ToolPart) } return { info: assistantMessage, parts: [part] } } // preventContinuation → stop; reflect the stop message as the part output so the agent sees it if (decision.stopReason) { if (part.state.status === "running") { - part = yield* sessions.updatePart({ ...part, state: { ...part.state, status: "error", error: `[Hook stopped] ${decision.stopReason}`, time: { ...part.state.time, end: Date.now() } } } satisfies SessionV1.ToolPart) + part = yield* sessions.updatePart({ + ...part, + state: { + ...part.state, + status: "error", + error: `[Hook stopped] ${decision.stopReason}`, + time: { ...part.state.time, end: Date.now() }, + }, + } satisfies SessionV1.ToolPart) } return { info: assistantMessage, parts: [part] } } @@ -380,7 +417,10 @@ export const layer = Layer.effect( hookReason: preResult.permissionDecisionReason, }) if (part.state.status === "running") { - part = yield* sessions.updatePart({ ...part, state: { ...part.state, status: "error", error: reason, time: { ...part.state.time, end: Date.now() } } } satisfies SessionV1.ToolPart) + part = yield* sessions.updatePart({ + ...part, + state: { ...part.state, status: "error", error: reason, time: { ...part.state.time, end: Date.now() } }, + } satisfies SessionV1.ToolPart) } return { info: assistantMessage, parts: [part] } } @@ -476,16 +516,18 @@ export const layer = Layer.effect( if (settingsHook) { const postResult: any = yield* settingsHook .trigger( - { event: "PostToolUse", toolName: TaskTool.id, toolInput: taskArgs, toolResponse: result?.output ?? "", toolUseID: part.callID } as any, + { + event: "PostToolUse", + toolName: TaskTool.id, + toolInput: taskArgs, + toolResponse: result?.output ?? "", + toolUseID: part.callID, + } as any, { sessionID, transcriptPath: "" }, ) .pipe(Effect.catch(() => Effect.succeed({ additionalContexts: [], systemMessages: [] } as TriggerResult))) yield* SettingsHook.landSystemMessages(postResult as TriggerResult, { sessionID }) - // PostToolUse preventContinuation: tool already executed, annotate its output. - if (postResult?.preventContinuation && result) { - const stopReason = postResult.stopReason ?? "Hook requested stop" - ;(result as any).output = `${result.output ?? ""}\n\n[Hook stopped] ${stopReason}` - } + if (result) result.output = withHookFeedback(result.output ?? "", postResult) } assistantMessage.finish = "tool-calls" @@ -680,10 +722,23 @@ export const layer = Layer.effect( if (settingsHook) { const preResult = yield* settingsHook .trigger( - { event: "PreToolUse", toolName: "bash", toolInput: { command: input.command }, toolUseID: mutablePart.callID } as any, + { + event: "PreToolUse", + toolName: "bash", + toolInput: { command: input.command }, + toolUseID: mutablePart.callID, + } as any, { sessionID: input.sessionID, transcriptPath: "" }, ) - .pipe(Effect.catch(() => Effect.succeed({ blocked: undefined, permissionDecision: undefined as "allow" | "deny" | "ask" | undefined, permissionDecisionReason: undefined as string | undefined } as any))) + .pipe( + Effect.catch(() => + Effect.succeed({ + blocked: undefined, + permissionDecision: undefined as "allow" | "deny" | "ask" | undefined, + permissionDecisionReason: undefined as string | undefined, + } as any), + ), + ) yield* SettingsHook.landSystemMessages(preResult as TriggerResult, { sessionID: input.sessionID }) const decision = applyPreHookDecision({ command: input.command }, preResult as any) // deny / blocked / stop / ask-degrade all skip execution; each surfaces its own message. @@ -704,9 +759,20 @@ export const layer = Layer.effect( }) } if (skipReason !== undefined) { - const errorState = { status: "error" as const, error: skipReason, time: { start: (mutablePart.state as any).time?.start ?? Date.now(), end: Date.now() }, input: mutablePart.state.input } - mutablePart = yield* sessions.updatePart({ ...mutablePart, state: errorState as any } satisfies SessionV1.ToolPart) - } else if (decision.effectiveArgs.command !== undefined && decision.effectiveArgs.command !== input.command) { + const errorState = { + status: "error" as const, + error: skipReason, + time: { start: (mutablePart.state as any).time?.start ?? Date.now(), end: Date.now() }, + input: mutablePart.state.input, + } + mutablePart = yield* sessions.updatePart({ + ...mutablePart, + state: errorState as any, + } satisfies SessionV1.ToolPart) + } else if ( + decision.effectiveArgs.command !== undefined && + decision.effectiveArgs.command !== input.command + ) { // updatedInput rewrote the command — sync execution (args), TUI display (part.state.input), // and PostToolUse toolInput (which reads effectiveCommand below). `!== undefined` (not // truthiness) so a hook that clears the command to "" is honored rather than ignored. @@ -717,24 +783,24 @@ export const layer = Layer.effect( } } if (!shellHookDenied) { - const cmd = ChildProcess.make(sh, args, { - cwd, - extendEnv: true, - env: { ...shellEnv.env, TERM: "dumb" }, - stdin: "ignore", - forceKillAfter: "3 seconds", - }) - const handle = yield* spawner.spawn(cmd) - yield* Stream.runForEach(Stream.decodeText(handle.all), (chunk) => - Effect.gen(function* () { - output += chunk - if (mutablePart.state.status === "running") { - mutablePart.state.metadata = { output } - yield* sessions.updatePart(mutablePart) - } - }), - ) - yield* handle.exitCode + const cmd = ChildProcess.make(sh, args, { + cwd, + extendEnv: true, + env: { ...shellEnv.env, TERM: "dumb" }, + stdin: "ignore", + forceKillAfter: "3 seconds", + }) + const handle = yield* spawner.spawn(cmd) + yield* Stream.runForEach(Stream.decodeText(handle.all), (chunk) => + Effect.gen(function* () { + output += chunk + if (mutablePart.state.status === "running") { + mutablePart.state.metadata = { output } + yield* sessions.updatePart(mutablePart) + } + }), + ) + yield* handle.exitCode } // end if (!shellHookDenied) }).pipe(Effect.scoped, Effect.orDie), ).pipe(Effect.exit) @@ -748,17 +814,22 @@ export const layer = Layer.effect( if (settingsHook && !shellHookDenied) { const postResult: any = yield* settingsHook .trigger( - { event: "PostToolUse", toolName: "bash", toolInput: { command: effectiveCommand }, toolResponse: output, toolUseID: mutablePart.callID } as any, + { + event: "PostToolUse", + toolName: "bash", + toolInput: { command: effectiveCommand }, + toolResponse: output, + toolUseID: mutablePart.callID, + } as any, { sessionID: input.sessionID, transcriptPath: "" }, ) .pipe(Effect.catch(() => Effect.succeed({ additionalContexts: [], systemMessages: [] } as TriggerResult))) yield* SettingsHook.landSystemMessages(postResult as TriggerResult, { sessionID: input.sessionID }) - // PostToolUse preventContinuation: annotate output (command already ran). - if (postResult?.preventContinuation) { - const stopReason = postResult.stopReason ?? "Hook requested stop" - output += `\n\n[Hook stopped] ${stopReason}` + const annotated = withHookFeedback(output, postResult) + if (annotated !== output) { + output = annotated if (mutablePart.state.status === "completed") { - mutablePart.state = { ...mutablePart.state, output, metadata: { output } } as any + mutablePart.state = { ...mutablePart.state, output, metadata: { output } } yield* sessions.updatePart(mutablePart) } } @@ -1332,7 +1403,7 @@ export const layer = Layer.effect( // Loop guard (hook-async-rewake): skip hooks for rewake prompts (those whose // text starts with HOOK_REWAKE_SENTINEL) to prevent hook → rewake → hook loops. let hookAdditionalContexts: string[] = [] - const promptText = input.parts.map((p: any) => p.type === "text" ? p.text : "").join("\n") + const promptText = input.parts.map((p: any) => (p.type === "text" ? p.text : "")).join("\n") const isRewake = promptText.startsWith(HOOK_REWAKE_SENTINEL) if (settingsHook && !isRewake) { const hookResult = yield* settingsHook @@ -1346,19 +1417,32 @@ export const layer = Layer.effect( // parts when the turn proceeds so the model sees them — no silent drop). yield* SettingsHook.landSystemMessages(hookResult, { sessionID: input.sessionID, - inject: hookResult.blocked - ? undefined - : (text) => - sessions.updatePart({ - id: PartID.ascending(), - messageID: message.info.id, - sessionID: input.sessionID, - type: "text", - text, - synthetic: true, - } satisfies SessionV1.TextPart), + inject: + hookResult.blocked || hookResult.preventContinuation + ? undefined + : (text) => + sessions.updatePart({ + id: PartID.ascending(), + messageID: message.info.id, + sessionID: input.sessionID, + type: "text", + text, + synthetic: true, + } satisfies SessionV1.TextPart), }) - if (hookResult.blocked) return { message, run: false as const } + if (hookResult.blocked || hookResult.preventContinuation) { + const reason = hookResult.stopReason ?? hookResult.blocked?.reason ?? "Hook requested stop" + const part = yield* sessions.updatePart({ + id: PartID.ascending(), + messageID: message.info.id, + sessionID: input.sessionID, + type: "text", + text: `[Hook stopped] ${reason}`, + synthetic: true, + } satisfies SessionV1.TextPart) + message.parts.push(part) + return { message, run: false as const } + } } // SettingsHook: drain HookStartContext queued by SessionStart hooks (only if not blocked) @@ -1407,55 +1491,55 @@ export const layer = Layer.effect( return yield* wait }) - const prepareIfIdle: Interface["prepareIfIdle"] = Effect.fn("SessionPrompt.prepareIfIdle")( - function* (input: PromptInput) { - return yield* promptLocks.withLock(input.sessionID)( - Effect.uninterruptibleMask((restore) => - Effect.gen(function* () { - const activation = yield* Deferred.make<void>() - const admission = yield* Deferred.make< + const prepareIfIdle: Interface["prepareIfIdle"] = Effect.fn("SessionPrompt.prepareIfIdle")(function* ( + input: PromptInput, + ) { + return yield* promptLocks.withLock(input.sessionID)( + Effect.uninterruptibleMask((restore) => + Effect.gen(function* () { + const activation = yield* Deferred.make<void>() + const admission = + yield* Deferred.make< Exit.Exit<{ readonly message: SessionV1.WithParts; readonly run: boolean }, Image.Error> >() - const wait = yield* state.startIfIdle( - input.sessionID, - lastAssistant(input.sessionID), - Effect.gen(function* () { - yield* Deferred.await(activation) - const admitted = yield* Deferred.await(admission) - if (Exit.isFailure(admitted)) return yield* Effect.failCause(admitted.cause) - if (!admitted.value.run) return admitted.value.message - return yield* runLoop(input.sessionID) - }).pipe(Effect.orDie), - ) - if (Option.isNone(wait)) return Option.none<IdleAdmission>() - - const admitted = yield* restore(admitPrompt(input)).pipe(Effect.exit) - yield* Deferred.succeed(admission, admitted) - if (Exit.isFailure(admitted)) { - yield* Deferred.succeed(activation, undefined) - return yield* Effect.failCause(admitted.cause) - } - return Option.some({ - activate: Deferred.succeed(activation, undefined).pipe(Effect.asVoid), - result: wait.value, - abort: state.cancel(input.sessionID), - }) - }), - ), - ) - }, - ) + const wait = yield* state.startIfIdle( + input.sessionID, + lastAssistant(input.sessionID), + Effect.gen(function* () { + yield* Deferred.await(activation) + const admitted = yield* Deferred.await(admission) + if (Exit.isFailure(admitted)) return yield* Effect.failCause(admitted.cause) + if (!admitted.value.run) return admitted.value.message + return yield* runLoop(input.sessionID) + }).pipe(Effect.orDie), + ) + if (Option.isNone(wait)) return Option.none<IdleAdmission>() - const promptIfIdle: Interface["promptIfIdle"] = Effect.fn("SessionPrompt.promptIfIdle")( - (input: PromptInput) => - Effect.uninterruptibleMask((restore) => - Effect.gen(function* () { - const prepared = yield* restore(prepareIfIdle(input)) - if (Option.isNone(prepared)) return Option.none() - yield* prepared.value.activate.pipe(Effect.onError(() => prepared.value.abort)) - return Option.some(yield* restore(prepared.value.result)) + const admitted = yield* restore(admitPrompt(input)).pipe(Effect.exit) + yield* Deferred.succeed(admission, admitted) + if (Exit.isFailure(admitted)) { + yield* Deferred.succeed(activation, undefined) + return yield* Effect.failCause(admitted.cause) + } + return Option.some({ + activate: Deferred.succeed(activation, undefined).pipe(Effect.asVoid), + result: wait.value, + abort: state.cancel(input.sessionID), + }) }), ), + ) + }) + + const promptIfIdle: Interface["promptIfIdle"] = Effect.fn("SessionPrompt.promptIfIdle")((input: PromptInput) => + Effect.uninterruptibleMask((restore) => + Effect.gen(function* () { + const prepared = yield* restore(prepareIfIdle(input)) + if (Option.isNone(prepared)) return Option.none() + yield* prepared.value.activate.pipe(Effect.onError(() => prepared.value.abort)) + return Option.some(yield* restore(prepared.value.result)) + }), + ), ) const lastAssistant = Effect.fnUntraced(function* (sessionID: SessionID) { @@ -1566,9 +1650,7 @@ export const layer = Layer.effect( const stopResult = yield* settingsHook .trigger(stopPayload, { sessionID, transcriptPath: "" }) .pipe( - Effect.catch(() => - Effect.succeed({ additionalContexts: [], systemMessages: [] } as TriggerResult), - ), + Effect.catch(() => Effect.succeed({ additionalContexts: [], systemMessages: [] } as TriggerResult)), ) // Consume Stop-hook outputs so nothing is silently dropped: inject // additionalContexts as synthetic text parts (model-visible on the @@ -1600,7 +1682,7 @@ export const layer = Layer.effect( // stop_hook_active=true so a well-behaved hook stops blocking (anti-loop). // Capped by MAX_STOP_CONTINUATIONS so a hook that ignores the signal // can't loop forever; at the limit we log.warn and force a normal exit. - if (!turnError && stopResult.blocked) { + if (!turnError && stopResult.blocked && !stopResult.preventContinuation) { if (stopContinuationCount < SettingsHook.MAX_STOP_CONTINUATIONS) { stopContinuationCount++ stopHookBlocked = true @@ -1743,6 +1825,7 @@ export const layer = Layer.effect( bypassAgentCheck, messages: msgs, promptOps, + hooks: settingsHook, }).pipe( Effect.provideService(Plugin.Service, plugin), Effect.provideService(Permission.Service, permission), @@ -2270,6 +2353,7 @@ export const defaultLayer = Layer.suspend(() => RuntimeFlags.defaultLayer, EventV2Bridge.defaultLayer, HookStartContext.defaultLayer, + SettingsHook.defaultLayer, Todo.defaultLayer, ), ), @@ -2427,7 +2511,9 @@ export const node = LayerNode.make(layer, [ Database.node, Memory.node, Todo.node, - HookStartContext.node, SettingsHook.node, Goal.node, + HookStartContext.node, + SettingsHook.node, + Goal.node, ]) export function admitIfIdle( diff --git a/packages/opencode/src/session/tools.ts b/packages/opencode/src/session/tools.ts index c0aa7494de..3e31a54749 100644 --- a/packages/opencode/src/session/tools.ts +++ b/packages/opencode/src/session/tools.ts @@ -14,18 +14,18 @@ import { Truncate } from "@/tool/truncate" import { Plugin } from "@/plugin" import { TaskTool, type TaskPromptOps } from "@/tool/task" import { SettingsHook, type TriggerResult } from "@/hook/settings" +import { withHookFeedback, withHookFailure } from "@/hook/trigger-result" +import { toolFileChanges } from "@/hook/file-changes" import { applyPreHookDecision, classifyPermissionAsk } from "@/hook/pre-hook-decision" import { type Tool as AITool, tool, jsonSchema, type ToolExecutionOptions, asSchema } from "ai" -import { Effect } from "effect" +import { Cause, Effect } from "effect" import * as Option from "effect/Option" -import { MessageV2 } from "./message-v2" import { Session } from "./session" import { SessionProcessor } from "./processor" import { PartID } from "./schema" import { TodoReminders } from "./todo-reminders" import { EffectBridge } from "@/effect/bridge" import { SessionContext } from "@/effect/session-context" -import { ProviderV2 } from "@opencode-ai/core/provider" import { ModelV2 } from "@opencode-ai/core/model" import { isRecord } from "@/util/record" @@ -42,8 +42,6 @@ const SUPPORTED_MCP_RESOURCE_ATTACHMENT_MIMES = new Set([ "image/png", "image/webp", ]) -// Tools that modify files on disk — trigger FileChanged hook after execution -const FILE_CHANGING_TOOLS = new Set(["edit", "write", "apply_patch", "multiedit", "patch"]) const ROOT_ONLY_TOOLS = new Set([MemorySearch.MemorySearchTool.id, TaskTool.id, "workflow"]) export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { @@ -54,6 +52,7 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { bypassAgentCheck: boolean messages: SessionV1.WithParts[] promptOps: TaskPromptOps + hooks?: SettingsHook.Interface }) { const tools: Record<string, AITool> = {} const run = yield* EffectBridge.make() @@ -62,7 +61,7 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { const registry = yield* ToolRegistry.Service const mcp = yield* MCP.Service const truncate = yield* Truncate.Service - const settingsHook = Option.getOrUndefined(yield* Effect.serviceOption(SettingsHook.Service)) + const hooks = input.hooks ?? Option.getOrUndefined(yield* Effect.serviceOption(SettingsHook.Service)) const context = (args: Record<string, unknown>, options: ToolExecutionOptions): Tool.Context => ({ sessionID: input.session.id, @@ -116,147 +115,184 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { description: item.description, inputSchema: jsonSchema(schema), execute(args, options) { + const settingsHook = withHookCancellation(hooks, options.abortSignal) return run.promise( // Set the active session for server-initiated MCP reverse requests // (elicitation) so the handler can route the Question to this session. SessionContext.run(context(args, options).sessionID, () => Effect.gen(function* () { const ctx = context(args, options) - yield* plugin.trigger( - "tool.execute.before", - { tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID }, - { args }, - ) - // SettingsHook PreToolUse - let preContexts: string[] = [] - // Native todo surfacing (#429): once per assistant turn, before - // any non-todowrite tool result, re-show the uncompleted list. - const todoReminder = yield* TodoReminders.preToolCall({ - sessionID: ctx.sessionID, - messageID: input.processor.message.id, - tool: item.id, - }) - if (settingsHook) { - const preResult = yield* settingsHook - .trigger( - { event: "PreToolUse", toolName: item.id, toolInput: toRecord(args), toolUseID: ctx.callID }, - { sessionID: ctx.sessionID, transcriptPath: "" }, - ) - .pipe(Effect.catch(() => Effect.succeed<TriggerResult>({ additionalContexts: [], systemMessages: [] }))) - yield* SettingsHook.landSystemMessages(preResult, { sessionID: ctx.sessionID }) - const decision = applyPreHookDecision(toRecord(args), preResult) - if (decision.deniedReason) { - return { output: `[Tool denied by hook] ${decision.deniedReason}`, attachments: [], metadata: { hookDenied: true } } as any - } - if (decision.stopReason) { - return { output: `[Hook stopped] ${decision.stopReason}`, attachments: [], metadata: { hookStopped: true } } as any - } - // permissionDecision:"ask" — invoke the confirmation dialog. We call - // permission.ask directly (NOT the orDie-piped ctx.ask) and classify the - // outcome: typed rejections become a denied result, while interrupts - // (session abort mid-dialog) and defects propagate instead of being - // masked as a denial. - if (preResult.permissionDecision === "ask") { - const askReason = preResult.permissionDecisionReason - const verdict = yield* permission - .ask({ - permission: item.id, - sessionID: ctx.sessionID, - patterns: [item.id], - always: [], - metadata: { hookAsk: true, ...(askReason ? { reason: askReason } : {}) }, - tool: { messageID: input.processor.message.id, callID: options.toolCallId }, - ruleset: [], - }) - .pipe(Effect.exit) - const outcome = classifyPermissionAsk(verdict) - if (outcome !== "approved" && outcome !== "denied") return yield* Effect.failCause(outcome.propagate as never) - if (outcome === "denied") { - const reason = askReason ?? "Denied by user in hook confirmation" - return { output: `[Tool denied by hook] ${reason}`, attachments: [], metadata: { hookDenied: true } } as any - } - } - preContexts = preResult.additionalContexts ?? [] - // effectiveArgs reflects any PreToolUse updatedInput rewrite (shallow merge). - args = decision.effectiveArgs - } - const result = yield* Effect.suspend(() => { - const cleanup = setActiveElicitationSession(ctx.sessionID) - return item.execute(args, ctx).pipe(Effect.ensuring(Effect.sync(cleanup))) - }) - const output = { - ...result, - attachments: result.attachments?.map((attachment) => ({ - ...attachment, - id: PartID.ascending(), + yield* plugin.trigger( + "tool.execute.before", + { tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID }, + { args }, + ) + // SettingsHook PreToolUse + let preContexts: string[] = [] + // Native todo surfacing (#429): once per assistant turn, before + // any non-todowrite tool result, re-show the uncompleted list. + const todoReminder = yield* TodoReminders.preToolCall({ sessionID: ctx.sessionID, messageID: input.processor.message.id, - })), - } - // PreToolUse additionalContexts: prepend so the model sees any hook-injected - // gate/reminder before the tool result (mirrors PostToolUse surfacing below). - const preLines = [todoReminder, ...preContexts].filter((line): line is string => Boolean(line)) - if (preLines.length) { - output.output = `${preLines.join("\n\n")}\n\n${output.output ?? ""}` - } - yield* plugin.trigger( - "tool.execute.after", - { tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID, args }, - output, - ) - // SettingsHook PostToolUse - if (settingsHook) { - const postResult = yield* settingsHook - .trigger( - { event: "PostToolUse", toolName: item.id, toolInput: toRecord(args), toolResponse: output.output, toolUseID: ctx.callID } as any, - { sessionID: ctx.sessionID, transcriptPath: "" }, - ) - .pipe(Effect.catch(() => Effect.succeed({ additionalContexts: [] as string[] } as any))) - yield* SettingsHook.landSystemMessages(postResult as TriggerResult, { sessionID: ctx.sessionID }) - // Inject additionalContext into tool output so model sees it - if ((postResult as any).additionalContexts?.length) { - output.output += "\n\n" + (postResult as any).additionalContexts.join("\n") + tool: item.id, + }) + if (settingsHook) { + const preResult = yield* settingsHook + .trigger( + { event: "PreToolUse", toolName: item.id, toolInput: toRecord(args), toolUseID: ctx.callID }, + { sessionID: ctx.sessionID, transcriptPath: "" }, + ) + .pipe( + Effect.catch(() => Effect.succeed<TriggerResult>({ additionalContexts: [], systemMessages: [] })), + ) + yield* SettingsHook.landSystemMessages(preResult, { sessionID: ctx.sessionID }) + const decision = applyPreHookDecision(toRecord(args), preResult) + if (decision.deniedReason) { + return { + output: `[Tool denied by hook] ${decision.deniedReason}`, + attachments: [], + metadata: { hookDenied: true }, + } as any + } + if (decision.stopReason) { + return { + output: `[Hook stopped] ${decision.stopReason}`, + attachments: [], + metadata: { hookStopped: true }, + } as any + } + // permissionDecision:"ask" — invoke the confirmation dialog. We call + // permission.ask directly (NOT the orDie-piped ctx.ask) and classify the + // outcome: typed rejections become a denied result, while interrupts + // (session abort mid-dialog) and defects propagate instead of being + // masked as a denial. + if (preResult.permissionDecision === "ask") { + const askReason = preResult.permissionDecisionReason + const verdict = yield* permission + .ask({ + permission: item.id, + sessionID: ctx.sessionID, + patterns: [item.id], + always: [], + metadata: { hookAsk: true, ...(askReason ? { reason: askReason } : {}) }, + tool: { messageID: input.processor.message.id, callID: options.toolCallId }, + ruleset: [], + }) + .pipe(Effect.exit) + const outcome = classifyPermissionAsk(verdict) + if (outcome !== "approved" && outcome !== "denied") + return yield* Effect.failCause(outcome.propagate as never) + if (outcome === "denied") { + const reason = askReason ?? "Denied by user in hook confirmation" + return { + output: `[Tool denied by hook] ${reason}`, + attachments: [], + metadata: { hookDenied: true }, + } as any + } + } + preContexts = preResult.additionalContexts ?? [] + // effectiveArgs reflects any PreToolUse updatedInput rewrite (shallow merge). + args = decision.effectiveArgs } - // PostToolUse preventContinuation: tool already executed, so annotate - // the output rather than skipping. Soft signal, mirrors CC semantics. - if ((postResult as any).preventContinuation) { - const stopReason = (postResult as any).stopReason ?? "Hook requested stop" - output.output += `\n\n[Hook stopped] ${stopReason}` + if (options.abortSignal?.aborted) return yield* Effect.interrupt + const result = yield* Effect.suspend(() => { + const cleanup = setActiveElicitationSession(ctx.sessionID) + return item.execute(args, ctx).pipe(Effect.ensuring(Effect.sync(cleanup))) + }) + const output = { + ...result, + attachments: result.attachments?.map((attachment) => ({ + ...attachment, + id: PartID.ascending(), + sessionID: ctx.sessionID, + messageID: input.processor.message.id, + })), } - } - // SettingsHook FileChanged for file-modifying tools - if (settingsHook && FILE_CHANGING_TOOLS.has(item.id)) { - const fileResult = yield* settingsHook - .trigger( - { event: "FileChanged", path: (toRecord(args))["file_path"] ?? (toRecord(args))["path"], changeType: item.id } as any, - { sessionID: ctx.sessionID, transcriptPath: "" }, - ) - .pipe(Effect.catch(() => Effect.succeed({ additionalContexts: [], systemMessages: [] } as TriggerResult))) - yield* SettingsHook.landSystemMessages(fileResult, { sessionID: ctx.sessionID }) - } - if (options.abortSignal?.aborted) { - yield* input.processor.completeToolCall(options.toolCallId, output) - } - return output - }).pipe( - Effect.catch((error: unknown) => - Effect.gen(function* () { - // SettingsHook PostToolUseFailure + // PreToolUse additionalContexts: prepend so the model sees any hook-injected + // gate/reminder before the tool result (mirrors PostToolUse surfacing below). + const preLines = [todoReminder, ...preContexts].filter((line): line is string => Boolean(line)) + if (preLines.length) { + output.output = `${preLines.join("\n\n")}\n\n${output.output ?? ""}` + } + yield* plugin.trigger( + "tool.execute.after", + { tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID, args }, + output, + ) + // SettingsHook PostToolUse if (settingsHook) { - const failResult = yield* settingsHook + const postResult = yield* settingsHook .trigger( - { event: "PostToolUseFailure", toolName: item.id, toolInput: toRecord(args), error: String(error), toolUseID: options.toolCallId } as any, - { sessionID: input.session.id, transcriptPath: "" }, + { + event: "PostToolUse", + toolName: item.id, + toolInput: toRecord(args), + toolResponse: output.output, + toolUseID: ctx.callID, + }, + { sessionID: ctx.sessionID, transcriptPath: "" }, ) - .pipe(Effect.catch(() => Effect.succeed({ additionalContexts: [], systemMessages: [] } as TriggerResult))) - yield* SettingsHook.landSystemMessages(failResult, { sessionID: input.session.id }) + .pipe( + Effect.catch(() => Effect.succeed<TriggerResult>({ additionalContexts: [], systemMessages: [] })), + ) + yield* SettingsHook.landSystemMessages(postResult, { sessionID: ctx.sessionID }) + output.output = withHookFeedback(output.output ?? "", postResult) + } + if (settingsHook) { + for (const change of toolFileChanges( + item.id, + toRecord(args), + result.metadata, + input.session.directory, + )) { + const fileResult = yield* settingsHook + .trigger({ event: "FileChanged", ...change }, { sessionID: ctx.sessionID, transcriptPath: "" }) + .pipe( + Effect.catch(() => Effect.succeed<TriggerResult>({ additionalContexts: [], systemMessages: [] })), + ) + yield* SettingsHook.landSystemMessages(fileResult, { sessionID: ctx.sessionID }) + output.output = withHookFeedback(output.output ?? "", fileResult) + } + } + if (options.abortSignal?.aborted) { + yield* input.processor.completeToolCall(options.toolCallId, output) } - return yield* Effect.fail(error) - }), + return output + }).pipe( + Effect.catchCause((cause) => + Effect.gen(function* () { + if (Cause.hasInterrupts(cause)) return yield* Effect.failCause(cause) + const error = Cause.squash(cause) + // SettingsHook PostToolUseFailure + if (settingsHook) { + const failResult = yield* settingsHook + .trigger( + { + event: "PostToolUseFailure", + toolName: item.id, + toolInput: toRecord(args), + error: String(error), + toolUseID: options.toolCallId, + }, + { sessionID: input.session.id, transcriptPath: "" }, + ) + .pipe( + Effect.catch(() => + Effect.succeed({ additionalContexts: [], systemMessages: [] } as TriggerResult), + ), + ) + yield* SettingsHook.landSystemMessages(failResult, { sessionID: input.session.id }) + const failure = withHookFailure(error, failResult) + if (failure !== error) + return yield* Cause.hasDies(cause) ? Effect.die(failure) : Effect.fail(failure) + } + return yield* Effect.failCause(cause) + }), + ), ), ), - ), - ) + ) }, }) } @@ -520,8 +556,9 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { const schema = yield* Effect.promise(() => Promise.resolve(asSchema(item.inputSchema).jsonSchema)) const transformed = ProviderTransform.schema(input.model, { ...schema, properties: schema.properties ?? {} }) item.inputSchema = jsonSchema(transformed) - item.execute = (args, opts) => - run.promise( + item.execute = (args, opts) => { + const settingsHook = withHookCancellation(hooks, opts.abortSignal) + return run.promise( Effect.gen(function* () { const ctx = context(args, opts) yield* plugin.trigger( @@ -543,12 +580,12 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { { event: "PreToolUse", toolName: key, toolInput: toRecord(args), toolUseID: opts.toolCallId }, { sessionID: ctx.sessionID, transcriptPath: "" }, ) - .pipe(Effect.catch(() => Effect.succeed<TriggerResult>({ additionalContexts: [], systemMessages: [] }))) - yield* SettingsHook.landSystemMessages(preResult, { sessionID: ctx.sessionID }) - const decision = applyPreHookDecision(toRecord(args), preResult) - if (decision.deniedReason) { - return { content: [{ type: "text", text: `[Tool denied by hook] ${decision.deniedReason}` }] } as any - } + .pipe(Effect.catch(() => Effect.succeed<TriggerResult>({ additionalContexts: [], systemMessages: [] }))) + yield* SettingsHook.landSystemMessages(preResult, { sessionID: ctx.sessionID }) + const decision = applyPreHookDecision(toRecord(args), preResult) + if (decision.deniedReason) { + return { content: [{ type: "text", text: `[Tool denied by hook] ${decision.deniedReason}` }] } as any + } if (decision.stopReason) { return { content: [{ type: "text", text: `[Hook stopped] ${decision.stopReason}` }] } as any } @@ -568,7 +605,8 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { }) .pipe(Effect.exit) const outcome = classifyPermissionAsk(verdict) - if (outcome !== "approved" && outcome !== "denied") return yield* Effect.failCause(outcome.propagate as never) + if (outcome !== "approved" && outcome !== "denied") + return yield* Effect.failCause(outcome.propagate as never) if (outcome === "denied") { const reason = askReason ?? "Denied by user in hook confirmation" return { content: [{ type: "text", text: `[Tool denied by hook] ${reason}` }] } as any @@ -577,6 +615,7 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { preContexts = preResult.additionalContexts ?? [] args = decision.effectiveArgs } + if (opts.abortSignal?.aborted) return yield* Effect.interrupt const result: Awaited<ReturnType<NonNullable<typeof execute>>> = yield* Effect.gen(function* () { yield* ctx.ask({ permission: key, metadata: {}, patterns: ["*"], always: ["*"] }) return yield* Effect.suspend(() => { @@ -666,47 +705,81 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { if (settingsHook) { const postResult = yield* settingsHook .trigger( - { event: "PostToolUse", toolName: key, toolInput: toRecord(args), toolResponse: output.output, toolUseID: opts.toolCallId } as any, + { + event: "PostToolUse", + toolName: key, + toolInput: toRecord(args), + toolResponse: output.output, + toolUseID: opts.toolCallId, + }, { sessionID: ctx.sessionID, transcriptPath: "" }, ) - .pipe(Effect.catch(() => Effect.succeed({ additionalContexts: [] as string[] } as any))) - yield* SettingsHook.landSystemMessages(postResult as TriggerResult, { sessionID: ctx.sessionID }) - if ((postResult as any).additionalContexts?.length) { - output.output += "\n\n" + (postResult as any).additionalContexts.join("\n") - } - // PostToolUse preventContinuation: annotate output (tool already ran). - if ((postResult as any).preventContinuation) { - const stopReason = (postResult as any).stopReason ?? "Hook requested stop" - output.output += `\n\n[Hook stopped] ${stopReason}` - } + .pipe(Effect.catch(() => Effect.succeed<TriggerResult>({ additionalContexts: [], systemMessages: [] }))) + yield* SettingsHook.landSystemMessages(postResult, { sessionID: ctx.sessionID }) + output.output = withHookFeedback(output.output ?? "", postResult) } if (opts.abortSignal?.aborted) { yield* input.processor.completeToolCall(opts.toolCallId, output) } return output }).pipe( - Effect.catch((error: unknown) => + Effect.catchCause((cause) => Effect.gen(function* () { - // SettingsHook PostToolUseFailure + if (Cause.hasInterrupts(cause)) return yield* Effect.failCause(cause) + const error = Cause.squash(cause) if (settingsHook) { - yield* settingsHook + const failResult = yield* settingsHook .trigger( - { event: "PostToolUseFailure", toolName: key, toolInput: toRecord(args), error: String(error), toolUseID: opts.toolCallId } as any, + { + event: "PostToolUseFailure", + toolName: key, + toolInput: toRecord(args), + error: String(error), + toolUseID: opts.toolCallId, + }, { sessionID: input.session.id, transcriptPath: "" }, ) - .pipe(Effect.catch(() => Effect.succeed(undefined as any))) + .pipe( + Effect.catch(() => Effect.succeed<TriggerResult>({ additionalContexts: [], systemMessages: [] })), + ) + yield* SettingsHook.landSystemMessages(failResult, { sessionID: input.session.id }) + const failure = withHookFailure(error, failResult) + if (failure !== error) return yield* Cause.hasDies(cause) ? Effect.die(failure) : Effect.fail(failure) } - return yield* Effect.fail(error) + return yield* Effect.failCause(cause) }), ), ), ) + } tools[key] = item } return tools }) +// Cancel hooks independently so tools that finalize partial output on abort can +// still persist that output through completeToolCall. +function withHookCancellation(hooks: SettingsHook.Interface | undefined, signal: AbortSignal | undefined) { + if (!hooks || !signal) return hooks + return { + ...hooks, + trigger: (payload: SettingsHook.HookPayload, ctx: SettingsHook.TriggerContext) => { + const empty = Effect.succeed<TriggerResult>({ additionalContexts: [], systemMessages: [] }) + if (signal.aborted) return empty + return Effect.raceFirst( + hooks.trigger(payload, ctx), + Effect.callback<TriggerResult>((resume) => { + const abort = () => resume(empty) + signal.addEventListener("abort", abort, { once: true }) + if (signal.aborted) abort() + return Effect.sync(() => signal.removeEventListener("abort", abort)) + }), + ) + }, + } satisfies SettingsHook.Interface +} + function toRecord(value: unknown) { if (isRecord(value)) return value return {} diff --git a/packages/opencode/src/tool/task.ts b/packages/opencode/src/tool/task.ts index d08a5edefd..2d76b55185 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -404,7 +404,7 @@ export const TaskTool = Tool.define( .pipe(Effect.catch(() => Effect.succeed({ additionalContexts: [], systemMessages: [] } as TriggerResult))) // Land any hook systemMessages so they're never silently dropped. yield* SettingsHook.landSystemMessages(stopResult, { sessionID: ctx.sessionID }) - if (!stopResult.blocked) { + if (!stopResult.blocked || stopResult.preventContinuation) { lastStillBlocked = false break } diff --git a/packages/opencode/test/hook/handler-cancellation.test.ts b/packages/opencode/test/hook/handler-cancellation.test.ts new file mode 100644 index 0000000000..b3a4e707a0 --- /dev/null +++ b/packages/opencode/test/hook/handler-cancellation.test.ts @@ -0,0 +1,159 @@ +import { expect, describe } from "bun:test" +import { Effect, Layer } from "effect" +import { MockLanguageModelV3 } from "ai/test" +import { SettingsHook, type HookCommand } from "@/hook/settings" +import { SessionHooks } from "@/hook/session-hooks" +import { EventV2Bridge } from "@/event-v2-bridge" +import { Database } from "@opencode-ai/core/database/database" +import { SessionID } from "@/session/schema" +import { Provider } from "@/provider/provider" +import { ProviderTest } from "../fake/provider" +import { Auth } from "@/auth" +import { MCP } from "@/mcp" +import { FSUtil } from "@opencode-ai/core/fs-util" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { testEffect } from "../lib/effect" + +const base = SettingsHook.layer.pipe( + Layer.provide(EventV2Bridge.defaultLayer), + Layer.provide(Database.defaultLayer), + Layer.provideMerge(SessionHooks.defaultLayer), +) +const it = testEffect(Layer.mergeAll(base, CrossSpawnSpawner.defaultLayer, FSUtil.defaultLayer)) +const run = (entry: HookCommand) => + Effect.gen(function* () { + const store = yield* SessionHooks.Service + const settings = yield* SettingsHook.Service + const id = SessionID.descending() + yield* store.add(id, { event: "PreToolUse", hooks: [entry] }) + return yield* settings.trigger( + { event: "PreToolUse", toolName: "bash", toolInput: {} }, + { sessionID: id, transcriptPath: "" }, + ) + }) +const usage = { + inputTokens: { total: 1, noCache: 1, cacheRead: 0, cacheWrite: 0 }, + outputTokens: { total: 1, text: 1, reasoning: 0 }, +} +const generated = (text: string) => ({ + content: [{ type: "text" as const, text }], + finishReason: { unified: "stop" as const, raw: "stop" }, + usage, + warnings: [], +}) +const providerLayer = (language: any) => + Layer.mergeAll( + Layer.mock(Provider.Service, { + defaultModel: () => Effect.succeed({ providerID: "audit" as any, modelID: "test" as any }), + getModel: () => Effect.succeed(ProviderTest.model()), + getLanguage: () => Effect.succeed(language), + }), + Layer.mock(Auth.Service, { get: () => Effect.succeed({ type: "api", key: "test-only" } as any) }), + ) + +describe("hook handlers through real AI SDK and MCP invocation adapter", () => { + it.instance("prompt handler accepts a model-produced block decision", () => + Effect.gen(function* () { + const model = new MockLanguageModelV3({ + doGenerate: generated(JSON.stringify({ decision: "block", reason: "prompt-real-handler" })), + }) + const result = yield* run({ type: "prompt", prompt: "audit decision" }).pipe(Effect.provide(providerLayer(model))) + expect(result.blocked?.reason).toBe("prompt-real-handler") + }), + ) + + it.instance("agent handler runs synthetic_output and accepts the result", () => + Effect.gen(function* () { + const model = new MockLanguageModelV3({ + doGenerate: { + content: [ + { + type: "tool-call", + toolCallId: "audit-output", + toolName: "synthetic_output", + input: JSON.stringify({ decision: "block", reason: "agent-real-handler" }), + }, + ], + finishReason: { unified: "tool-calls", raw: "tool_calls" }, + usage, + warnings: [], + }, + }) + const result = yield* run({ type: "agent", prompt: "emit audit decision" }).pipe( + Effect.provide(providerLayer(model)), + ) + expect(result.blocked?.reason).toBe("agent-real-handler") + }), + ) + + it.instance("mcp handler maps its tool name and accepts the decision", () => + Effect.gen(function* () { + let envelope: any + const mcp = Layer.mock(MCP.Service, { + tools: () => + Effect.succeed({ + audit_check: { + execute: async (input: any) => { + envelope = input + return { + content: [{ type: "text", text: JSON.stringify({ decision: "block", reason: "mcp-real-handler" }) }], + } + }, + }, + } as any), + }) + const result = yield* run({ type: "mcp", command: "mcp__audit__check" }).pipe(Effect.provide(mcp)) + expect(envelope.hook_event_name).toBe("PreToolUse") + expect(result.blocked?.reason).toBe("mcp-real-handler") + }), + ) + + it.instance("prompt timeout must abort the underlying model request", () => + Effect.gen(function* () { + let release!: () => void + let signal: AbortSignal | undefined + const gate = new Promise<void>((resolve) => (release = resolve)) + const model = new MockLanguageModelV3({ + doGenerate: async (input) => { + signal = input.abortSignal + await gate + return generated("{}") + }, + }) + const result = yield* run({ type: "prompt", prompt: "audit timeout", timeout: 0.03 }).pipe( + Effect.provide(providerLayer(model)), + ) + const aborted = signal?.aborted ?? false + release() + expect(result.blocked).toBeUndefined() + expect(model.doGenerateCalls.length).toBe(1) + expect(aborted).toBe(true) + }), + ) + + it.instance("mcp timeout must abort the underlying tool request", () => + Effect.gen(function* () { + let release!: () => void + let signal: AbortSignal | undefined + const gate = new Promise<void>((resolve) => (release = resolve)) + const mcp = Layer.mock(MCP.Service, { + tools: () => + Effect.succeed({ + audit_check: { + execute: async (_input: any, options: any) => { + signal = options.abortSignal + await gate + return { content: [] } + }, + }, + } as any), + }) + const result = yield* run({ type: "mcp", command: "mcp__audit__check", timeout: 0.03 }).pipe(Effect.provide(mcp)) + const aborted = signal?.aborted ?? false + release() + expect(result.blocked).toBeUndefined() + expect(signal).toBeDefined() + expect(aborted).toBe(true) + }), + ) +}) diff --git a/packages/opencode/test/hook/http-handler.test.ts b/packages/opencode/test/hook/http-handler.test.ts index ade67bdcba..7d0882d2b9 100644 --- a/packages/opencode/test/hook/http-handler.test.ts +++ b/packages/opencode/test/hook/http-handler.test.ts @@ -32,6 +32,51 @@ const withFetch = <A, E, R>( ) describe("SettingsHook http handler", () => { + it.instance("expands only allowed header environment variables on the wire", () => + Effect.gen(function* () { + const store = yield* SessionHooks.Service + const hook = yield* SettingsHook.Service + const id = SessionID.descending() + const previous = process.env.OPENCODE_HOOK_HEADER_TEST + process.env.OPENCODE_HOOK_HEADER_TEST = "fixture-value" + yield* Effect.addFinalizer(() => + Effect.sync(() => { + if (previous === undefined) delete process.env.OPENCODE_HOOK_HEADER_TEST + else process.env.OPENCODE_HOOK_HEADER_TEST = previous + }), + ) + let seen: Headers | undefined + yield* withFetch( + (request) => { + seen = request.headers + return Response.json({}) + }, + (url) => + Effect.gen(function* () { + yield* store.add(id, { + event: "UserPromptSubmit", + hooks: [ + { + type: "http", + url, + allowedEnvVars: ["OPENCODE_HOOK_HEADER_TEST"], + headers: { + "x-allowed": "Bearer ${OPENCODE_HOOK_HEADER_TEST}", + "x-bare": "$OPENCODE_HOOK_HEADER_TEST", + "x-denied": "value:${HOME}", + }, + }, + ], + }) + yield* hook.trigger({ event: "UserPromptSubmit", prompt: "hello" }, { sessionID: id, transcriptPath: "" }) + expect(seen?.get("x-allowed")).toBe("Bearer fixture-value") + expect(seen?.get("x-bare")).toBe("fixture-value") + expect(seen?.get("x-denied")).toBe("value:") + }), + ) + }), + ) + it.instance("applies configured entry.headers to the outbound POST", () => Effect.gen(function* () { const sessionHooks = yield* SessionHooks.Service diff --git a/packages/opencode/test/hook/prompt-admission.test.ts b/packages/opencode/test/hook/prompt-admission.test.ts new file mode 100644 index 0000000000..7dd4708067 --- /dev/null +++ b/packages/opencode/test/hook/prompt-admission.test.ts @@ -0,0 +1,425 @@ +import { SessionHooks } from "@/hook/session-hooks" +import { expect } from "bun:test" +import fs from "node:fs/promises" +import path from "node:path" +import { Effect, Exit, Fiber, Layer } from "effect" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { Session } from "@/session/session" +import { SessionPrompt } from "@/session/prompt" +import { SessionSummary } from "@/session/summary" +import { Database } from "@opencode-ai/core/database/database" +import { SessionProjector } from "@opencode-ai/core/session/projector" +import { provideTmpdirServer } from "../fixture/fixture" +import { testEffect, pollWithTimeout } from "../lib/effect" +import { TestLLMServer } from "../lib/llm-server" + +import { LSP } from "@/lsp/lsp" +import { MCP } from "@/mcp" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { RuntimeFlags } from "@/effect/runtime-flags" +import { ProviderV2 } from "@opencode-ai/core/provider" +import { ModelV2 } from "@opencode-ai/core/model" + +const mcp = Layer.succeed( + MCP.Service, + MCP.Service.of({ + status: () => Effect.succeed({}), + clients: () => Effect.succeed({}), + instructions: () => Effect.succeed([]), + tools: () => Effect.succeed({}), + prompts: () => Effect.succeed({}), + resources: () => Effect.succeed({}), + resourceTemplates: () => Effect.succeed({}), + add: () => Effect.succeed({ status: { status: "disabled" as const } }), + connect: () => Effect.void, + disconnect: () => Effect.void, + getPrompt: () => Effect.succeed(undefined), + readResource: () => Effect.succeed(undefined), + startAuth: () => Effect.die("unexpected MCP auth"), + authenticate: () => Effect.die("unexpected MCP auth"), + finishAuth: () => Effect.die("unexpected MCP auth"), + removeAuth: () => Effect.void, + supportsOAuth: () => Effect.succeed(false), + hasStoredTokens: () => Effect.succeed(false), + getAuthStatus: () => Effect.succeed("not_authenticated" as const), + }), +) + +const lsp = Layer.succeed( + LSP.Service, + LSP.Service.of({ + init: () => Effect.void, + status: () => Effect.succeed([]), + hasClients: () => Effect.succeed(false), + touchFile: () => Effect.void, + diagnostics: () => Effect.succeed({}), + hover: () => Effect.succeed(undefined), + definition: () => Effect.succeed([]), + references: () => Effect.succeed([]), + implementation: () => Effect.succeed([]), + documentSymbol: () => Effect.succeed([]), + workspaceSymbol: () => Effect.succeed([]), + prepareCallHierarchy: () => Effect.succeed([]), + incomingCalls: () => Effect.succeed([]), + outgoingCalls: () => Effect.succeed([]), + }), +) + +const root = LayerNode.group([ + SessionPrompt.node, + SessionHooks.node, + Session.node, + SessionProjector.node, + SessionSummary.node, + Database.node, + CrossSpawnSpawner.node, + LayerNode.make(TestLLMServer.layer, []), +]) +const it = testEffect( + LayerNode.buildLayer(root, { + replacements: [ + LayerNode.replace(MCP.node, mcp), + LayerNode.replace(LSP.node, lsp), + LayerNode.replace(RuntimeFlags.node, RuntimeFlags.layer({ experimentalEventSystem: true })), + ], + }), +) + +const providerCfg = (url: string) => ({ + provider: { + test: { + name: "Test", + id: "test", + env: [], + npm: "@ai-sdk/openai-compatible", + models: { + "test-model": { + id: "test-model", + name: "Test Model", + attachment: false, + reasoning: false, + temperature: false, + tool_call: true, + release_date: "2025-01-01", + limit: { context: 100000, output: 10000 }, + cost: { input: 0, output: 0 }, + options: {}, + }, + }, + options: { + apiKey: "test-key", + baseURL: url, + }, + }, + }, +}) + +for (const variant of ["block", "continue-false"] as const) { + it.live(`UserPromptSubmit ${variant} must stop before invoking the model`, () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ llm }) { + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const hooks = yield* SessionHooks.Service + const session = yield* sessions.create({ title: "Audit prompt hook control", permission: [] }) + const output = + variant === "block" + ? { decision: "block", reason: "audit-block" } + : { continue: false, stopReason: "audit-stop" } + yield* hooks.add(session.id, { + event: "UserPromptSubmit", + hooks: [{ type: "command", command: "printf '%s' '" + JSON.stringify(output) + "'" }], + }) + yield* llm.text("audit-model-was-called") + const result = yield* prompt.prompt({ + sessionID: session.id, + agent: "build", + parts: [{ type: "text", text: "audit user prompt stop signal" }], + }) + const calls = yield* llm.calls + expect(calls).toBe(0) + expect(result.info.role).toBe("user") + }), + { git: true, config: providerCfg }, + ), + ) +} + +it.live("async hooks re-enter the real session after their caller has completed", () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ llm, dir }) { + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const hooks = yield* SessionHooks.Service + const session = yield* sessions.create({ title: "Async hook callback", permission: [] }) + const release = path.join(dir, "release") + const quote = (value: string) => "'" + value.replaceAll("'", "'\\''") + "'" + yield* hooks.add(session.id, { + event: "UserPromptSubmit", + hooks: [ + { + type: "command", + async: true, + asyncRewake: true, + once: true, + command: `while [ ! -f ${quote(release)} ]; do sleep 0.01; done; printf '%s' '{"systemMessage":"async finished"}'`, + }, + ], + }) + yield* llm.text("first response") + yield* llm.text("rewake response") + yield* prompt.prompt({ + sessionID: session.id, + agent: "build", + parts: [{ type: "text", text: "start async hook" }], + }) + expect(yield* llm.calls).toBe(1) + yield* Effect.promise(() => fs.writeFile(release, "go")) + yield* pollWithTimeout( + llm.calls.pipe(Effect.map((calls) => (calls === 2 ? true : undefined))), + "async hook did not invoke the model again", + ) + const messages = yield* sessions.messages({ sessionID: session.id }) + expect( + messages.some((message) => + message.parts.some((part) => part.type === "text" && part.text.includes("Async hook completed")), + ), + ).toBe(true) + }), + { git: true, config: providerCfg }, + ), +) + +it.live("session cancellation aborts an executing pre-tool hook", () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ llm, dir }) { + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const hooks = yield* SessionHooks.Service + const session = yield* sessions.create({ + title: "Cancel running hook", + permission: [{ permission: "*", pattern: "*", action: "allow" }], + }) + const ready = path.join(dir, "hook.pid") + const target = path.join(dir, "must-not-write.txt") + const quote = (value: string) => "'" + value.replaceAll("'", "'\\''") + "'" + yield* hooks.add(session.id, { + event: "PreToolUse", + hooks: [{ type: "command", command: `echo $$ > ${quote(ready)}; exec sleep 30` }], + }) + yield* llm.tool("write", { filePath: target, content: "should be cancelled" }) + yield* prompt + .prompt({ sessionID: session.id, agent: "build", parts: [{ type: "text", text: "cancel this operation" }] }) + .pipe(Effect.forkChild) + const pid = yield* pollWithTimeout( + Effect.promise(async () => { + try { + return Number(await fs.readFile(ready, "utf8")) || undefined + } catch { + return undefined + } + }), + "pre-tool hook never started", + ) + yield* prompt.cancel(session.id) + yield* pollWithTimeout( + Effect.sync(() => { + try { + process.kill(pid, 0) + return undefined + } catch { + return true + } + }), + "pre-tool hook survived session cancellation", + 2000, + ) + expect( + yield* Effect.promise(() => + fs.access(target).then( + () => true, + () => false, + ), + ), + ).toBe(false) + }), + { git: true, config: providerCfg }, + ), +) + +it.live("session cancellation aborts a post-tool hook and preserves the completed tool", () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ llm, dir }) { + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const hooks = yield* SessionHooks.Service + const session = yield* sessions.create({ + title: "Cancel post-tool hook", + permission: [{ permission: "*", pattern: "*", action: "allow" }], + }) + const ready = path.join(dir, "post-hook.pid") + const target = path.join(dir, "written.txt") + const quote = (value: string) => "'" + value.replaceAll("'", "'\\''") + "'" + yield* hooks.add(session.id, { + event: "PostToolUse", + hooks: [{ type: "command", command: `echo $$ > ${quote(ready)}; exec sleep 30` }], + }) + yield* llm.tool("write", { filePath: target, content: "completed before cancellation" }) + const running = yield* prompt + .prompt({ + sessionID: session.id, + agent: "build", + parts: [{ type: "text", text: "write the file" }], + }) + .pipe(Effect.forkChild) + const pid = yield* pollWithTimeout( + Effect.promise(async () => { + try { + return Number(await fs.readFile(ready, "utf8")) || undefined + } catch { + return undefined + } + }), + "post-tool hook never started", + ) + yield* prompt.cancel(session.id) + yield* pollWithTimeout( + Effect.sync(() => { + try { + process.kill(pid, 0) + return undefined + } catch { + return true + } + }), + "post-tool hook survived cancellation", + 2000, + ) + const result = yield* Fiber.await(running) + expect(Exit.isSuccess(result)).toBe(true) + if (Exit.isFailure(result)) return + const tool = result.value.parts.find((part) => part.type === "tool") + expect(tool?.state.status).toBe("completed") + expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("completed before cancellation") + }), + { git: true, config: providerCfg }, + ), +) + +it.live("native tool failures run failure hooks and preserve their feedback", () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ llm, dir }) { + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const hooks = yield* SessionHooks.Service + const session = yield* sessions.create({ + title: "Native failure hook", + permission: [{ permission: "*", pattern: "*", action: "allow" }], + }) + yield* hooks.add(session.id, { + event: "PostToolUseFailure", + hooks: [ + { + type: "command", + command: `printf '%s' '{"hookSpecificOutput":{"additionalContext":"failure-feedback: retry an existing file"}}'`, + }, + ], + }) + yield* llm.tool("read", { filePath: path.join(dir, "missing.txt") }) + yield* llm.text("I will retry a file that exists.") + yield* prompt.prompt({ + sessionID: session.id, + agent: "build", + parts: [{ type: "text", text: "read the missing file" }], + }) + const parts = (yield* sessions.messages({ sessionID: session.id })).flatMap((message) => message.parts) + const failed = parts.find((part) => part.type === "tool" && part.tool === "read") + expect(failed?.type).toBe("tool") + if (failed?.type !== "tool") return + expect(failed?.state.status).toBe("error") + if (failed?.state.status !== "error") return + expect(failed.state.error).toContain("failure-feedback") + expect(JSON.stringify(yield* llm.inputs)).toContain("failure-feedback") + }), + { git: true, config: providerCfg }, + ), +) + +it.live("native write, edit and multi-file patch emit actual FileChanged paths", () => + provideTmpdirServer( + Effect.fnUntraced(function* ({ llm, dir }) { + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const hooks = yield* SessionHooks.Service + const session = yield* sessions.create({ + title: "Hook file events", + permission: [{ permission: "*", pattern: "*", action: "allow" }], + }) + const capture = path.join(dir, "events.jsonl") + const original = path.join(dir, "source.txt") + const moved = path.join(dir, "moved.txt") + const second = path.join(dir, "second.txt") + const quote = (value: string) => "'" + value.replaceAll("'", "'\\''") + "'" + yield* hooks.add(session.id, { + event: "FileChanged", + hooks: [{ type: "command", command: "cat >> " + quote(capture) }], + }) + yield* llm.tool("write", { filePath: original, content: "before\n" }) + yield* llm.tool("edit", { filePath: original, oldString: "before", newString: "edited" }) + yield* llm.text("edited") + yield* prompt.prompt({ + sessionID: session.id, + agent: "build", + model: { providerID: ProviderV2.ID.make("test"), modelID: ModelV2.ID.make("test-model") }, + parts: [{ type: "text", text: "exercise write and edit" }], + }) + expect(yield* Effect.promise(() => fs.readFile(original, "utf8"))).toBe("edited\n") + yield* llm.tool("apply_patch", { + patchText: `*** Begin Patch\n*** Update File: ${original}\n*** Move to: ${moved}\n@@\n-edited\n+after\n*** Add File: ${second}\n+second\n*** End Patch`, + }) + yield* llm.text("done") + yield* prompt.prompt({ + sessionID: session.id, + agent: "build", + model: { providerID: ProviderV2.ID.make("test"), modelID: ModelV2.ID.make("gpt-hook-test") }, + parts: [{ type: "text", text: "exercise patch" }], + }) + expect(yield* Effect.promise(() => fs.readFile(moved, "utf8"))).toBe("after\n") + expect(yield* Effect.promise(() => fs.readFile(second, "utf8"))).toBe("second\n") + expect( + yield* Effect.promise(() => + fs.access(original).then( + () => true, + () => false, + ), + ), + ).toBe(false) + const records = (yield* Effect.promise(() => fs.readFile(capture, "utf8"))) + .trim() + .split("\n") + .map((line) => JSON.parse(line)) + expect(records.map((record) => [record.path, record.change_type])).toEqual([ + [original, "add"], + [original, "change"], + [original, "delete"], + [moved, "add"], + [second, "add"], + ]) + }), + { + git: true, + config: (url) => { + const config = providerCfg(url) + const model = config.provider.test.models["test-model"] + return { + provider: { + test: { + ...config.provider.test, + models: { ...config.provider.test.models, "gpt-hook-test": { ...model, id: "gpt-hook-test" } }, + }, + }, + } + }, + }, + ), +) diff --git a/packages/opencode/test/hook/readonly-command.test.ts b/packages/opencode/test/hook/readonly-command.test.ts new file mode 100644 index 0000000000..0f290af3d0 --- /dev/null +++ b/packages/opencode/test/hook/readonly-command.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, test } from "bun:test" +import { parseReadonlyCommand, whitelistReject } from "@/hook/readonly-command" + +describe("agent read-only command boundary", () => { + for (const command of [ + "find . -delete", + "find . -exec touch marker", + "find . -fprint marker", + "sort input -o marker", + "sort --output=marker input", + "sort --compress-program=sh input", + "file -z archive.gz", + "uniq input marker", + "uniq -- input marker", + "git diff --output=marker", + "git diff --ext-diff", + "git show --textconv", + "git -c alias.status=evil status", + "git log --format=x --output=marker", + "sed -n '1w marker' input", + "sed -n '1e touch marker' input", + "awk 'BEGIN {system(\"touch marker\")}'", + "echo safe\ntouch marker", + "echo safe\rtouch marker", + "echo safe; touch marker", + "echo $(touch marker)", + "echo `touch marker`", + "cat input > marker", + "/tmp/cat input", + "./git status", + "echo 'unterminated", + ]) { + test(`rejects ${JSON.stringify(command)}`, () => expect(whitelistReject(command)).not.toBeNull()) + } + + for (const command of [ + "ls -la", + "cat 'file with spaces.txt'", + "grep -n needle src/file.ts", + "find . -name '*.ts' -type f", + "git status --short", + "git log -n 2 --oneline", + "git diff --stat", + "git show HEAD -- src/file.ts", + "head -n 10 input", + "tail -f input", + "sort -nu input", + "uniq -c input", + "sed -n '1,10p' input", + ]) { + test(`accepts ${command}`, () => expect(whitelistReject(command)).toBeNull()) + } + + test("preserves quoted arguments without shell expansion", () => { + expect(parseReadonlyCommand("cat 'a b' \"c d\" e\\ f")).toEqual({ name: "cat", args: ["a b", "c d", "e f"] }) + }) +}) diff --git a/packages/opencode/test/hook/runtime-boundaries.test.ts b/packages/opencode/test/hook/runtime-boundaries.test.ts new file mode 100644 index 0000000000..9c0e109d34 --- /dev/null +++ b/packages/opencode/test/hook/runtime-boundaries.test.ts @@ -0,0 +1,292 @@ +import { describe, expect, test } from "bun:test" +import { Effect, Layer, Fiber } from "effect" +import fs from "node:fs/promises" +import path from "node:path" +import { SettingsHook, type HookCommand } from "@/hook/settings" +import { SessionHooks } from "@/hook/session-hooks" +import { EventV2Bridge } from "@/event-v2-bridge" +import { Database } from "@opencode-ai/core/database/database" +import { SessionID } from "@/session/schema" +import { testEffect, pollWithTimeout } from "../lib/effect" +import { TestInstance } from "../fixture/fixture" +import { __test__ as agentToolsTest } from "@/hook/agent-tools" + +const it = testEffect( + SettingsHook.layer.pipe( + Layer.provide(EventV2Bridge.defaultLayer), + Layer.provide(Database.defaultLayer), + Layer.provideMerge(SessionHooks.defaultLayer), + ), +) +const quote = (text: string) => "'" + text.replaceAll("'", "'\\''") + "'" +const command = (json: unknown, exit = 0) => `printf '%s' ${quote(JSON.stringify(json))}; exit ${exit}` +const runHook = (hook: HookCommand) => + Effect.gen(function* () { + const store = yield* SessionHooks.Service + const settings = yield* SettingsHook.Service + const id = SessionID.descending() + yield* store.add(id, { event: "PreToolUse", hooks: [hook] }) + return yield* settings.trigger( + { event: "PreToolUse", toolName: "bash", toolInput: { command: "echo safe" } }, + { sessionID: id, transcriptPath: "" }, + ) + }) + +describe("hooks audit - actual runtime boundary", () => { + it.instance("valid siblings survive invalid matcher and command shapes", () => + Effect.gen(function* () { + const instance = yield* TestInstance + yield* Effect.promise(async () => { + await fs.mkdir(path.join(instance.directory, ".opencode"), { recursive: true }) + await fs.writeFile( + path.join(instance.directory, ".opencode/hooks.json"), + JSON.stringify({ + PreToolUse: [ + null, + { matcher: 42, hooks: [] }, + { + hooks: [ + null, + { type: "http" }, + { type: "command", command: command({ decision: "block", reason: "valid sibling" }) }, + ], + }, + ], + }), + ) + }) + const settings = yield* SettingsHook.Service + const result = yield* settings.trigger( + { event: "PreToolUse", toolName: "write", toolInput: {} }, + { sessionID: SessionID.descending(), transcriptPath: "" }, + ) + expect(result.blocked?.reason).toBe("valid sibling") + }), + ) + + it.instance("invalid JSON-shaped output cannot become prompt context", () => + Effect.gen(function* () { + const settings = yield* SettingsHook.Service + const store = yield* SessionHooks.Service + const id = SessionID.descending() + yield* store.add(id, { + event: "UserPromptSubmit", + hooks: [{ type: "command", command: command({ hookSpecificOutput: "broken" }) }], + }) + const result = yield* settings.trigger( + { event: "UserPromptSubmit", prompt: "hello" }, + { sessionID: id, transcriptPath: "" }, + ) + expect(result.additionalContexts).toEqual([]) + }), + ) + + it.instance("explicit bash interpreter handles bash syntax", () => + Effect.gen(function* () { + const result = yield* runHook({ + type: "command", + shell: "bash", + command: "[[ -n $BASH_VERSION ]] && " + command({ decision: "block", reason: "bash selected" }), + }) + expect(result.blocked?.reason).toBe("bash selected") + }), + ) + + it.instance("timeout ignores JSON even when a TERM handler exits successfully", () => + Effect.gen(function* () { + const result = yield* runHook({ + type: "command", + timeout: 0.15, + command: + "trap 'exit 0' TERM; " + + command({ decision: "block", reason: "expired" }).replace("; exit 0", "") + + "; while :; do sleep 1; done", + }) + expect(result.blocked).toBeUndefined() + }), + ) + + it.instance("caller interruption kills a command hook process", () => + Effect.gen(function* () { + const instance = yield* TestInstance + const pidFile = path.join(instance.directory, "hook.pid") + const fiber = yield* runHook({ type: "command", command: `echo $$ > ${quote(pidFile)}; exec sleep 30` }).pipe( + Effect.forkChild, + ) + const pid = yield* pollWithTimeout( + Effect.promise(async () => { + try { + return Number(await fs.readFile(pidFile, "utf8")) || undefined + } catch { + return undefined + } + }), + "hook never started", + ) + yield* Fiber.interrupt(fiber) + yield* pollWithTimeout( + Effect.sync(() => { + try { + process.kill(pid, 0) + return undefined + } catch { + return true + } + }), + "hook process survived caller cancellation", + ) + }), + ) + + it.instance("a once group stays eligible until a condition matches, then all commands run once", () => + Effect.gen(function* () { + const instance = yield* TestInstance + const store = yield* SessionHooks.Service + const settings = yield* SettingsHook.Service + const id = SessionID.descending() + const output = path.join(instance.directory, "group-once.txt") + yield* store.add(id, { + event: "PreToolUse", + once: true, + hooks: ["first", "second"].map((label) => ({ + type: "command", + if: "Bash(match*)", + command: `printf '${label}\\n' >> ${quote(output)}`, + })), + }) + const trigger = (value: string) => + settings.trigger( + { event: "PreToolUse", toolName: "bash", toolInput: { command: value } }, + { sessionID: id, transcriptPath: "" }, + ) + yield* trigger("unmatched") + expect(yield* store.listAll(id)).toHaveLength(1) + yield* Effect.all([trigger("match1"), trigger("match2")], { concurrency: "unbounded" }) + expect((yield* Effect.promise(() => fs.readFile(output, "utf8"))).trim().split("\n")).toEqual(["first", "second"]) + expect(yield* store.listAll(id)).toHaveLength(0) + }), + ) + + for (const async of [false, true]) { + it.instance(`command once is atomic with async=${async}`, () => + Effect.gen(function* () { + const instance = yield* TestInstance + const store = yield* SessionHooks.Service + const settings = yield* SettingsHook.Service + const id = SessionID.descending() + const output = path.join(instance.directory, "command-once.txt") + yield* store.add(id, { + event: "PreToolUse", + hooks: [{ type: "command", once: true, async, command: `printf 'hit\\n' >> ${quote(output)}` }], + }) + const trigger = () => + settings.trigger( + { event: "PreToolUse", toolName: "bash", toolInput: {} }, + { sessionID: id, transcriptPath: "" }, + ) + yield* Effect.all([trigger(), trigger(), trigger()], { concurrency: "unbounded" }) + const content = yield* pollWithTimeout( + Effect.promise(() => fs.readFile(output, "utf8").catch(() => undefined)), + "once command did not finish", + ) + expect(content).toBe("hit\n") + yield* trigger() + expect(yield* Effect.promise(() => fs.readFile(output, "utf8"))).toBe("hit\n") + }), + ) + } + + it.instance("control: exit 0 accepts block output", () => + Effect.gen(function* () { + const result = yield* runHook({ type: "command", command: command({ decision: "block", reason: "audit-block" }) }) + expect(result.blocked?.reason).toBe("audit-block") + }), + ) + + it.instance("control: exit 2 blocks using stderr", () => + Effect.gen(function* () { + const result = yield* runHook({ type: "command", command: "printf '%s' 'audit-exit2' >&2; exit 2" }) + expect(result.blocked?.reason).toBe("audit-exit2") + }), + ) + + it.instance("exit 1 must not apply stdout control decisions", () => + Effect.gen(function* () { + const result = yield* runHook({ + type: "command", + command: command({ decision: "block", reason: "audit-invalid-exit1" }, 1), + }) + expect(result.blocked).toBeUndefined() + }), + ) + + it.instance("malformed hookSpecificOutput must not crash the trigger", () => + Effect.gen(function* () { + const exit = yield* runHook({ + type: "command", + command: command({ hookSpecificOutput: "bad-output-type" }), + }).pipe(Effect.exit) + expect(exit._tag).toBe("Success") + }), + ) + + it.instance("malformed matcher group must not crash the trigger", () => + Effect.gen(function* () { + const instance = yield* TestInstance + yield* Effect.promise(async () => { + await fs.mkdir(path.join(instance.directory, ".opencode"), { recursive: true }) + await fs.writeFile( + path.join(instance.directory, ".opencode/hooks.json"), + JSON.stringify({ PreToolUse: [{ matcher: "Bash" }] }), + ) + }) + const settings = yield* SettingsHook.Service + const exit = yield* settings + .trigger( + { event: "PreToolUse", toolName: "bash", toolInput: {} }, + { sessionID: SessionID.descending(), transcriptPath: "" }, + ) + .pipe(Effect.exit) + expect(exit._tag).toBe("Success") + }), + ) + + it.instance("once hook must execute only once across concurrent triggers", () => + Effect.gen(function* () { + const instance = yield* TestInstance + const store = yield* SessionHooks.Service + const settings = yield* SettingsHook.Service + const id = SessionID.descending() + const output = path.join(instance.directory, "once-runs.txt") + yield* store.add(id, { + event: "PreToolUse", + once: true, + hooks: [{ type: "command", command: `sleep 0.1; printf 'hit\n' >> ${quote(output)}` }], + }) + yield* Effect.all( + [1, 2].map(() => + settings.trigger( + { event: "PreToolUse", toolName: "bash", toolInput: {} }, + { sessionID: id, transcriptPath: "" }, + ), + ), + { concurrency: "unbounded" }, + ) + const lines = (yield* Effect.promise(() => fs.readFile(output, "utf8"))).trim().split("\n") + expect(lines).toHaveLength(1) + }), + ) + + for (const candidate of [ + "find . -delete", + "sort /dev/null -o audit-output", + "git diff --output=audit-output", + "awk 'BEGIN {system(\"touch audit-output\")}'", + "echo safe\ntouch audit-output", + ]) { + test(`read-only agent whitelist must reject: ${candidate}`, () => { + const rejection = agentToolsTest.whitelistReject(candidate) + expect(rejection).not.toBeNull() + }) + } +}) diff --git a/packages/opencode/test/hook/tool-boundaries.test.ts b/packages/opencode/test/hook/tool-boundaries.test.ts new file mode 100644 index 0000000000..385655b57e --- /dev/null +++ b/packages/opencode/test/hook/tool-boundaries.test.ts @@ -0,0 +1,283 @@ +import { describe, expect } from "bun:test" +import { Effect, Layer, Schema, Cause } from "effect" +import fs from "node:fs/promises" +import path from "node:path" +import { SessionTools } from "@/session/tools" +import { SessionHooks } from "@/hook/session-hooks" +import { SettingsHook } from "@/hook/settings" +import { EventV2Bridge } from "@/event-v2-bridge" +import { Database } from "@opencode-ai/core/database/database" +import { ToolRegistry } from "@/tool/registry" +import { Plugin } from "@/plugin" +import { MCP } from "@/mcp" +import { Permission } from "@/permission" +import { Truncate } from "@/tool/truncate" +import { SessionID, MessageID } from "@/session/schema" +import { TestInstance } from "../fixture/fixture" +import { testEffect } from "../lib/effect" +import { ProviderTest } from "../fake/provider" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { FSUtil } from "@opencode-ai/core/fs-util" +import { ChildProcessSpawner } from "effect/unstable/process/ChildProcessSpawner" +import { buildAgentTools } from "@/hook/agent-tools" +import { jsonSchema, tool } from "ai" + +const quote = (text: string) => "'" + text.replaceAll("'", "'\\''") + "'" +const jsonCommand = (json: unknown) => "printf '%s' " + quote(JSON.stringify(json)) +const hookLayer = SettingsHook.layer.pipe( + Layer.provide(EventV2Bridge.defaultLayer), + Layer.provide(Database.defaultLayer), + Layer.provideMerge(SessionHooks.defaultLayer), +) +const writeDefinition = { + id: "write", + description: "Fixture leaf writer with the real write tool parameter shape", + parameters: Schema.Struct({ filePath: Schema.String, content: Schema.String }), + execute: (args: { filePath: string; content: string }) => + Effect.promise(async () => { + await fs.writeFile(args.filePath, args.content) + return { title: "written", output: "WRITE_COMPLETE", metadata: {} } + }), +} +const layers = Layer.mergeAll( + hookLayer, + Layer.mock(Plugin.Service, { trigger: (_name, _input, output) => Effect.succeed(output) }), + Layer.mock(Permission.Service, { ask: () => Effect.void }), + Layer.mock(MCP.Service, { clients: () => Effect.succeed({}), tools: () => Effect.succeed({}) }), + Layer.mock(Truncate.Service, {}), + Layer.mock(ToolRegistry.Service, { tools: () => Effect.succeed([writeDefinition]) }), +) +const it = testEffect(layers) +const resolve = (id: any, dir: string) => + SessionTools.resolve({ + agent: { name: "build", permission: [], options: {} } as any, + model: ProviderTest.model(), + session: { id, directory: dir, permission: [] } as any, + processor: { + message: { id: MessageID.ascending(), sessionID: id }, + updateToolCall: () => Effect.succeed(undefined), + completeToolCall: () => Effect.void, + } as any, + bypassAgentCheck: false, + messages: [], + promptOps: {} as any, + }) +const execute = (tools: any, filePath: string) => + Effect.promise(() => + tools.write.execute( + { filePath, content: "audit-safe-fixture" }, + { toolCallId: "audit-write", messages: [], abortSignal: new AbortController().signal }, + ), + ) + +describe("real SessionTools call path", () => { + it.instance("FileChanged envelope must contain native filePath", () => + Effect.gen(function* () { + const instance = yield* TestInstance + const store = yield* SessionHooks.Service + const id = SessionID.descending() + const capture = path.join(instance.directory, "filechanged.json") + const target = path.join(instance.directory, "changed.txt") + yield* store.add(id, { event: "FileChanged", hooks: [{ type: "command", command: "cat > " + quote(capture) }] }) + const tools = yield* resolve(id, instance.directory) + yield* execute(tools, target) + expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("audit-safe-fixture") + const envelope = JSON.parse(yield* Effect.promise(() => fs.readFile(capture, "utf8"))) + expect(envelope.path).toBe(target) + }), + ) + + it.instance("PostToolUse block reason must be visible to the model", () => + Effect.gen(function* () { + const instance = yield* TestInstance + const store = yield* SessionHooks.Service + const id = SessionID.descending() + yield* store.add(id, { + event: "PostToolUse", + hooks: [{ type: "command", command: jsonCommand({ decision: "block", reason: "AUDIT_POST_REJECT" }) }], + }) + const tools = yield* resolve(id, instance.directory) + const result = yield* execute(tools, path.join(instance.directory, "changed.txt")) + expect(JSON.stringify(result)).toContain("AUDIT_POST_REJECT") + }), + ) + + it.instance("malformed hook output must not fail the tool execution", () => + Effect.gen(function* () { + const instance = yield* TestInstance + const store = yield* SessionHooks.Service + const id = SessionID.descending() + yield* store.add(id, { + event: "PreToolUse", + hooks: [{ type: "command", command: jsonCommand({ hookSpecificOutput: "broken" }) }], + }) + const tools = yield* resolve(id, instance.directory) + const result = yield* execute(tools, path.join(instance.directory, "changed.txt")).pipe(Effect.exit) + expect(result._tag).toBe("Success") + }), + ) + + it.instance("rejected MCP calls run failure hooks and keep feedback", () => + Effect.gen(function* () { + const instance = yield* TestInstance + const store = yield* SessionHooks.Service + const mcp = yield* MCP.Service + const id = SessionID.descending() + yield* store.add(id, { + event: "PostToolUseFailure", + hooks: [{ type: "command", command: jsonCommand({ decision: "block", reason: "MCP_FAILURE_FEEDBACK" }) }], + }) + const tools = yield* resolve(id, instance.directory).pipe( + Effect.provideService(MCP.Service, { + ...mcp, + tools: () => + Effect.succeed({ + audit_failure: tool({ + inputSchema: jsonSchema({ type: "object", properties: {} }), + execute: async (): Promise<{ content: { type: "text"; text: string }[] }> => { + throw new Error("MCP transport rejected") + }, + }), + }), + }), + ) + const result = yield* Effect.tryPromise({ + try: () => + Promise.resolve( + tools.audit_failure.execute!( + {}, + { + toolCallId: "audit-failure", + messages: [], + abortSignal: new AbortController().signal, + }, + ), + ), + catch: (error) => error, + }).pipe(Effect.exit) + expect(result._tag).toBe("Failure") + if (result._tag !== "Failure") return + const failure = String(Cause.squash(result.cause)) + expect(failure).toContain("MCP transport rejected") + expect(failure).toContain("MCP_FAILURE_FEEDBACK") + }), + ) +}) + +const agent = testEffect(Layer.mergeAll(CrossSpawnSpawner.defaultLayer, FSUtil.defaultLayer)) +describe("agent tool runtime boundary", () => { + agent.instance("read-only agent bash must not create a file using a newline", () => + Effect.gen(function* () { + const instance = yield* TestInstance + const spawner = yield* ChildProcessSpawner + const filesystem = yield* FSUtil.Service + const tools = buildAgentTools({ + spawner, + fs: filesystem, + cwd: instance.directory, + signal: new AbortController().signal, + captured: { value: null }, + }) + const marker = path.join(instance.directory, "unauthorized-marker") + const result = yield* Effect.promise(() => + Promise.resolve( + tools.bash.execute!({ command: "echo harmless\ntouch " + quote(marker) }, { + toolCallId: "audit", + messages: [], + } as any), + ), + ) + const exists = yield* Effect.promise(() => + fs.access(marker).then( + () => true, + () => false, + ), + ) + expect(JSON.stringify(result)).toContain("Error:") + expect(exists).toBe(false) + }), + ) + + agent.instance("agent bash must stop after its abort signal", () => + Effect.gen(function* () { + const instance = yield* TestInstance + const spawner = yield* ChildProcessSpawner + const filesystem = yield* FSUtil.Service + const ac = new AbortController() + const input = path.join(instance.directory, "follow.txt") + yield* Effect.promise(() => fs.writeFile(input, "ready\n")) + let pid: number | undefined + let ready!: () => void + const started = new Promise<void>((resolve) => { + ready = resolve + }) + const observed = { + ...spawner, + spawn: (...args: Parameters<typeof spawner.spawn>) => + spawner.spawn(...args).pipe( + Effect.tap((handle) => + Effect.sync(() => { + pid = Number(handle.pid) + ready() + }), + ), + ), + } + const tools = buildAgentTools({ + spawner: observed, + fs: filesystem, + cwd: instance.directory, + signal: ac.signal, + captured: { value: null }, + }) + yield* Effect.addFinalizer(() => Effect.sync(() => ac.abort())) + const task = Promise.resolve( + tools.bash.execute!( + { command: "tail -f " + quote(input) }, + { toolCallId: "cancel", messages: [], abortSignal: ac.signal }, + ), + ) + yield* Effect.promise(() => started).pipe(Effect.timeout(2000)) + ac.abort() + const result = yield* Effect.promise(() => task).pipe(Effect.timeout(3000)) + expect(JSON.stringify(result)).toContain("Error:") + expect(pid).toBeDefined() + expect(() => process.kill(pid!, 0)).toThrow() + }), + ) +}) + +const permission = testEffect( + Permission.layer.pipe(Layer.provide(EventV2Bridge.defaultLayer), Layer.provideMerge(hookLayer)), +) +describe("permission hook call path", () => { + for (const variant of ["deny", "exit2"] as const) { + permission.instance(`PermissionRequest ${variant} should resolve rejection without user input`, () => + Effect.gen(function* () { + const store = yield* SessionHooks.Service + const permissions = yield* Permission.Service + const id = SessionID.descending() + const command = + variant === "deny" + ? jsonCommand({ hookSpecificOutput: { hookEventName: "PermissionRequest", permissionDecision: "deny" } }) + : "printf '%s' 'AUDIT_PERMISSION_BLOCK' >&2; exit 2" + yield* store.add(id, { event: "PermissionRequest", hooks: [{ type: "command", command }] }) + const exit = yield* permissions + .ask({ + sessionID: id, + permission: "bash", + patterns: ["echo test"], + always: [], + metadata: {}, + ruleset: [], + }) + .pipe(Effect.timeout(150), Effect.exit) + const error = exit._tag === "Failure" ? Cause.pretty(exit.cause) : "success" + expect(error).not.toContain("Timeout") + expect(exit._tag).toBe("Failure") + expect(yield* permissions.list()).toHaveLength(0) + if (variant === "exit2") expect(error).toContain("AUDIT_PERMISSION_BLOCK") + }), + ) + } +}) diff --git a/packages/opencode/test/hook/warn-unsupported.test.ts b/packages/opencode/test/hook/warn-unsupported.test.ts index 23ea587871..360ef7852d 100644 --- a/packages/opencode/test/hook/warn-unsupported.test.ts +++ b/packages/opencode/test/hook/warn-unsupported.test.ts @@ -1,11 +1,6 @@ import { describe, expect, test } from "bun:test" import { detectUnsupportedFields, type Settings } from "@/hook/settings" -// hooks-api-fidelity: async / asyncRewake / `if` are all fully implemented -// (hook-async-execution + condition-filter) and MUST NOT be flagged as -// unsupported. Only `shell` remains a runtime placeholder and MUST still be -// flagged so users know it is inert. - const hooks = (hook: Record<string, unknown>): Settings["hooks"] => ({ SessionStart: [{ matcher: "", hooks: [{ type: "command", command: "true", ...hook }] }], }) @@ -21,17 +16,16 @@ describe("detectUnsupportedFields", () => { expect(unsupported).toEqual([]) }) - test("shell is still flagged (placeholder)", () => { + test("shell is supported by command hooks", () => { const unsupported = detectUnsupportedFields(hooks({ shell: "powershell" })) - expect(unsupported).toHaveLength(1) - expect(unsupported[0]).toMatchObject({ field: "shell", value: "powershell", eventName: "SessionStart" }) + expect(unsupported).toEqual([]) }) - test("only shell is flagged when if+shell+async all present", () => { + test("command options compose without unsupported-field warnings", () => { const unsupported = detectUnsupportedFields( hooks({ if: "Edit(*.ts)", shell: "bash", async: true, asyncRewake: true }), ) - expect(unsupported.map((u) => u.field).sort()).toEqual(["shell"]) + expect(unsupported).toEqual([]) }) test("undefined / empty hooks yield no flags", () => { @@ -39,15 +33,16 @@ describe("detectUnsupportedFields", () => { expect(detectUnsupportedFields({})).toEqual([]) }) - // GOAL-FP/issue #286: HookCommand fields accepted by the schema but dropped - // by every executor must be surfaced, not silently swallowed. `timeout` for - // type "prompt" is implemented (excluded here); allowedEnvVars/statusMessage - // have zero consumers anywhere, and per-command `once` is never read (only - // the entry-level _sessionEntry?.once is consumed). - test("allowedEnvVars / statusMessage / per-command once are flagged (dropped by executors)", () => { - const unsupported = detectUnsupportedFields( - hooks({ allowedEnvVars: ["FOO"], statusMessage: "hi", once: true }), - ) - expect(unsupported.map((u) => u.field).sort()).toEqual(["allowedEnvVars", "once", "statusMessage"]) + test("allowedEnvVars is restricted to HTTP; statusMessage and once are supported", () => { + const unsupported = detectUnsupportedFields(hooks({ allowedEnvVars: ["FOO"], statusMessage: "hi", once: true })) + expect(unsupported.map((u) => u.field).sort()).toEqual(["allowedEnvVars"]) + }) + + test("HTTP accepts environment interpolation and diagnoses an irrelevant shell", () => { + expect( + detectUnsupportedFields( + hooks({ type: "http", url: "http://localhost", allowedEnvVars: ["TEST"], shell: "bash" }), + ), + ).toEqual([{ field: "shell", value: "bash", eventName: "SessionStart" }]) }) }) diff --git a/packages/opencode/test/server/session-hooks-api.test.ts b/packages/opencode/test/server/session-hooks-api.test.ts index 9aaf012f14..c54918c323 100644 --- a/packages/opencode/test/server/session-hooks-api.test.ts +++ b/packages/opencode/test/server/session-hooks-api.test.ts @@ -18,6 +18,33 @@ function addHook(directory: string, sessionID: string, hook: Record<string, unkn } describe("session hook add validation", () => { + it.instance( + "rejects missing handler descriptors and round-trips supported command fields", + () => + Effect.gen(function* () { + const instance = yield* TestInstance + const session = yield* Session.use.create({}) + for (const type of ["mcp", "http", "prompt", "agent"]) { + const response = yield* addHook(instance.directory, session.id, { type }) + expect(response.status).toBe(400) + } + const input = { + type: "command", + command: "true", + shell: "bash", + once: true, + statusMessage: "checking", + options: { mode: "check" }, + } + const added = yield* addHook(instance.directory, session.id, input) + expect(added.status).toBe(200) + const listed = yield* requestInDirectory(`/session/${session.id}/hook`, instance.directory) + expect(listed.status).toBe(200) + expect(yield* listed.json).toMatchObject([{ hooks: [input] }]) + }), + { git: true }, + ) + it.instance( "rejects command-type hooks with a missing or blank command", () => diff --git a/packages/sdk/js/src/v2/gen/sdk.gen.ts b/packages/sdk/js/src/v2/gen/sdk.gen.ts index f376c2222b..f07f686863 100644 --- a/packages/sdk/js/src/v2/gen/sdk.gen.ts +++ b/packages/sdk/js/src/v2/gen/sdk.gen.ts @@ -3461,6 +3461,13 @@ export class Hook extends HeyApiClient { if?: string async?: boolean asyncRewake?: boolean + shell?: "bash" | "powershell" + allowedEnvVars?: Array<string> + statusMessage?: string + once?: boolean + options?: { + [key: string]: unknown + } }> once?: boolean }, diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index 10d0a3a905..c18e600a24 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -10125,6 +10125,13 @@ export type SessionHookListResponses = { if?: string async?: boolean asyncRewake?: boolean + shell?: "bash" | "powershell" + allowedEnvVars?: Array<string> + statusMessage?: string + once?: boolean + options?: { + [key: string]: unknown + } }> once?: boolean }> @@ -10174,6 +10181,13 @@ export type SessionHookAddData = { if?: string async?: boolean asyncRewake?: boolean + shell?: "bash" | "powershell" + allowedEnvVars?: Array<string> + statusMessage?: string + once?: boolean + options?: { + [key: string]: unknown + } }> once?: boolean }