From 62fa870e19c629c7513fde7de1c16556d06efeed Mon Sep 17 00:00:00 2001 From: Lex Date: Mon, 7 Sep 2026 11:13:11 +0800 Subject: [PATCH 1/6] chore: record delivery binding for hooks-runtime-reliability --- .specgit.yaml | 41 ++++++++++++++++++++++++++++++++++------- spec_git/policy.yaml | 3 +++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/.specgit.yaml b/.specgit.yaml index f4e219ff2..1471fb80e 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,11 +1,38 @@ version: 1 -delivery: stable-release +delivery: hooks-runtime-reliability context: - kind: branch - branch: chore/543-stable-release + kind: worktree + label: opencode-hooks-fix + branch: fix/557-hooks-runtime-reliability issues: - - 543 + - 557 + - 558 + - 559 + - 560 + - 561 + - 562 + - 563 + - 564 + - 565 + - 566 issueKinds: - - issue: 543 - kind: kind::chore -pr: 544 + - 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 diff --git a/spec_git/policy.yaml b/spec_git/policy.yaml index fe3768c27..3d6b9f65a 100644 --- a/spec_git/policy.yaml +++ b/spec_git/policy.yaml @@ -2,3 +2,6 @@ version: 1 required_checks: - Typecheck - Unit Tests (linux) +automation: + merge: false + close_issues: false From deb8b7e6290120fe1361f42f2789eefcb3b7f5d9 Mon Sep 17 00:00:00 2001 From: Lex Date: Mon, 7 Sep 2026 11:13:58 +0800 Subject: [PATCH 2/6] chore: record delivery binding for hooks-runtime-reliability --- .specgit.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.specgit.yaml b/.specgit.yaml index 1471fb80e..ce102a001 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -36,3 +36,4 @@ issueKinds: kind: kind::fix - issue: 566 kind: kind::fix +pr: 567 From eba6221ca5e93d1ed0e5da47acfc9e7941d8e4c8 Mon Sep 17 00:00:00 2001 From: Lex Date: Mon, 7 Sep 2026 12:07:37 +0800 Subject: [PATCH 3/6] chore: refresh SpecGit 1.14 delivery harness --- .github/workflows/specgit-accept.yml | 183 +++++++++++++++++++----- .gitignore | 13 ++ .opencode/hooks/specgit-merge-guard.sh | 190 +++++++++++++++++++++++-- AGENTS.md | 117 ++++++++++----- 4 files changed, 419 insertions(+), 84 deletions(-) diff --git a/.github/workflows/specgit-accept.yml b/.github/workflows/specgit-accept.yml index 93d6a6166..2d8ce8d51 100644 --- a/.github/workflows/specgit-accept.yml +++ b/.github/workflows/specgit-accept.yml @@ -2,22 +2,19 @@ name: SpecGit Acceptance on: pull_request: - branches: [main] + branches: ["main"] # A draft PR fails the verdict (pr_draft), so the draft→ready # transition must re-verdict. Listing types replaces the defaults, - # so the default activity types are listed alongside. - types: [opened, synchronize, reopened, ready_for_review] - # No workflow_dispatch (local specialization): dispatch is the privileged - # context that fires CodeQL's cache-poisoning taint rule on the head_ref - # checkout (false positive: no cache use, read-only token, - # persist-credentials: false), and on dispatch events head_ref is empty so - # the verdict would evaluate the default branch — the wrong tree. Delivery - # here always goes through a PR. + # so the default activity types are listed alongside. Title and body + # edits change live acceptance evidence even when the head is unchanged. + types: [opened, synchronize, reopened, ready_for_review, edited] + # Local specialization: acceptance runs only against a bound pull request. permissions: contents: read issues: read pull-requests: read + actions: read # One verdict per head at a time (#319): a newer trigger event (a push # after the draft opened, then ready_for_review) supersedes the older @@ -35,33 +32,46 @@ jobs: # project's own toolchain (package manager, lockfile, build, layout) # is never assumed and never invoked. runs-on: ubuntu-latest - # Local specialization: must exceed the slowest required sibling - # (Unit Tests (linux) runs ~28min on PRs) — the verdict waits for every - # policy check to reach a terminal state before evaluating. + # Unit Tests (linux) can take about 28 minutes on pull requests. timeout-minutes: 45 steps: - name: Checkout code uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - # Check out the PR head branch by name so HEAD is on the branch - # (not the detached merge ref): the execution context gate reads - # live git. Falls back to the default ref on non-PR events. - ref: ${{ github.head_ref || github.ref }} + # Pin execution to this event; a newer branch push must not change + # the code tested by an older run. Manual dispatch uses its own SHA. + ref: ${{ github.event.pull_request.head.sha || github.sha }} fetch-depth: 0 persist-credentials: false + - name: Restore the event branch + if: github.event_name == 'pull_request' || github.ref_type == 'branch' + env: + SPECGIT_BRANCH: ${{ github.head_ref || github.ref_name }} + run: | + git check-ref-format --branch "$SPECGIT_BRANCH" >/dev/null + git switch --create "$SPECGIT_BRANCH" + - name: Setup Node.js uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: node-version: '22' + package-manager-cache: false - name: Install pinned SpecGit CLI - # Local specialization — install GLOBALLY, not `npm install --no-save - # specgit@X`: a workspace-local install reads this bun workspace's - # package.json and dies on the `catalog:` protocol (EUNSUPPORTEDPROTOCOL, - # #434/#459). Exact pin on purpose: the gate must evaluate with the - # same CLI generation that wrote the binding (1.10.1 re-init). - run: npm install -g --no-audit --no-fund specgit@1.10.1 + # Exact version on purpose (no ^): the gate must evaluate with the + # 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.14.0 + + - name: Prepare approved policy for acceptance + env: + GH_TOKEN: ${{ github.token }} + SPECGIT_WAIT_POLICY: ${{ runner.temp }}/specgit-policy.yaml + run: | + gh auth setup-git + node "$RUNNER_TEMP/specgit-cli/node_modules/specgit/dist/automation/workflow-policy.js" - name: Wait for sibling checks # The verdict must see the OTHER required checks in a terminal @@ -78,21 +88,23 @@ jobs: WAIT_REPO: ${{ github.repository }} WAIT_SHA: ${{ github.event.pull_request.head.sha || github.sha }} WAIT_PR: ${{ github.event.pull_request.number || '' }} + WAIT_POLICY: ${{ runner.temp }}/specgit-policy.yaml + SPECGIT_CLI_DIR: ${{ runner.temp }}/specgit-cli run: | node --input-type=module <<'EOF' import { existsSync, readFileSync } from 'node:fs'; import { execFileSync } from 'node:child_process'; - if (!existsSync('spec_git/policy.yaml')) { + import { createRequire } from 'node:module'; + const { parse } = process.env.SPECGIT_CLI_DIR + ? createRequire(process.env.SPECGIT_CLI_DIR + '/node_modules/specgit/package.json')('yaml') + : await import('yaml'); + const policyPath = process.env.WAIT_POLICY || 'spec_git/policy.yaml'; + if (!existsSync(policyPath)) { console.error('spec_git/policy.yaml is absent at this head — an adoption PR carries no binding commit yet (expected once; merge it before enabling branch protection), and a delivery PR must carry it via specgit issue.'); process.exit(1); } - // Local specialization — minimal hand parse of policy.yaml's - // required_checks block: this bun-based repo does not expose a - // root-reachable `yaml` package (workspace catalog isolation), so - // `import { parse } from 'yaml'` would fail to resolve here. - const policy = readFileSync('spec_git/policy.yaml', 'utf8'); - const section = policy.slice(policy.indexOf('required_checks:')); - const required = [...section.matchAll(/^\s*-\s*(.+)$/gm)].map((m) => m[1].trim()); + const policy = parse(readFileSync(policyPath, 'utf8')); + const required = policy.required_checks ?? []; // gh.cmd needs a shell on Windows; POSIX execs the binary // directly (shell stays off where it is not needed). The @@ -169,6 +181,109 @@ jobs: } } }; + /** @param {unknown} value @returns {value is number} */ + function positiveIdentity(value) { + return typeof value === 'number' && Number.isSafeInteger(value) && value > 0; + } + + /** @param {unknown} app */ + export function isGithubActionsApp(app) { + return typeof app === 'object' && app !== null && + (('slug' in app && app.slug === 'github-actions') || ('id' in app && app.id === 15368)); + } + + /** + * One ownership decision for the provider and embedded wait program. + * Callers prove the head and list completeness before passing normalized rows. + * Pending owners stay pending; callers decide which jobs must wait for them. + * @template {import('./actions-ownership.mjs').ActionsWorkflow} T + * @param {readonly T[]} workflows + * @returns {import('./actions-ownership.mjs').ActionsOwnership} + */ + export function createActionsOwnership(workflows) { + /** @type {Map} */ + const latest = new Map(); + /** @type {Map} */ + const owners = new Map(); + const ids = new Set(); + for (const workflow of workflows) { + const check = workflow.check; + if (typeof workflow.key !== 'string' || !workflow.key || + !positiveIdentity(check.id) || !positiveIdentity(workflow.checkSuiteId) || + !positiveIdentity(workflow.runAttempt) || typeof check.startedAt !== 'string' || + !Number.isFinite(Date.parse(check.startedAt)) || owners.has(workflow.checkSuiteId) || + ids.has(check.id) || !['queued', 'in_progress', 'completed', 'waiting', 'pending', 'requested'].includes(check.status)) { + throw new Error('GitHub returned incomplete or ambiguous Actions workflow ownership.'); + } + owners.set(workflow.checkSuiteId, workflow); + ids.add(check.id); + const previous = latest.get(workflow.key); + const started = Date.parse(check.startedAt); + if (!previous || started > Date.parse(previous.check.startedAt ?? '') || + (started === Date.parse(previous.check.startedAt ?? '') && check.id > previous.check.id)) { + latest.set(workflow.key, workflow); + } + } + return { + latest: [...latest.values()], + currentFor(checkSuiteId) { + if (!positiveIdentity(checkSuiteId)) { + throw new Error('GitHub returned an Actions check without a check-suite identity.'); + } + const owner = owners.get(checkSuiteId); + if (!owner) throw new Error('The Actions check has no proven owning workflow run.'); + return latest.get(owner.key)?.check.id === owner.check.id ? owner : null; + }, + }; + } + const isActions = (check) => isGithubActionsApp(check.app); + const listWorkflowsWithRetry = async (page) => { + for (let attempt = 1; ; attempt++) { + try { + return JSON.parse(execFileSync('gh', [ + 'api', 'repos/' + process.env.WAIT_REPO + '/actions/runs', '--method', 'GET', + '--field', 'head_sha=' + process.env.WAIT_SHA, '--field', 'per_page=' + PER_PAGE, '--field', 'page=' + page, + ], { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'pipe'], shell: process.platform === 'win32' })); + } catch (error) { + const text = String(error) + ' ' + String(error && error.stderr ? error.stderr : ''); + if (attempt >= 5 || !/HTTP 5\d\d|HTTP 429|ETIMEDOUT|ECONNRESET|ENOTFOUND|timed out/i.test(text)) throw error; + await new Promise((resolve) => setTimeout(resolve, Math.min(30000, 2000 * 2 ** (attempt - 1)))); + } + } + }; + const currentExecutionChecks = async (checks) => { + if (!checks.some(isActions)) return checks; + const workflows = []; + const positive = (value) => Number.isSafeInteger(value) && value > 0; + let total; + for (let page = 1; page <= 10; page++) { + const payload = await listWorkflowsWithRetry(page); + if (!Array.isArray(payload?.workflow_runs) || payload.workflow_runs.length > PER_PAGE + || !Number.isSafeInteger(payload.total_count) || payload.total_count < 0) throw new Error('GitHub Actions workflow evidence is malformed.'); + if (payload.total_count > 1000) throw new Error('GitHub Actions workflow evidence exceeds the 1000-run API limit.'); + if (total !== undefined && total !== payload.total_count) throw new Error('GitHub Actions workflow evidence changed during pagination.'); + total = payload.total_count; + for (const run of payload.workflow_runs) { + if (!positive(run?.workflow_id) || run.head_sha !== process.env.WAIT_SHA + || typeof run.event !== 'string' || run.event.length === 0) throw new Error('GitHub Actions workflow ownership is missing, invalid, or ambiguous.'); + workflows.push({ key: JSON.stringify([run.workflow_id, run.event]), checkSuiteId: run.check_suite_id, + runAttempt: run.run_attempt, check: { id: run.id, startedAt: run.run_started_at, status: run.status } }); + } + if (payload.workflow_runs.length < PER_PAGE) break; + if (page === 10) throw new Error('GitHub Actions workflow evidence reached the pagination limit.'); + } + if (workflows.length !== total) throw new Error('GitHub Actions workflow evidence is incomplete.'); + const ownership = createActionsOwnership(workflows); + return checks.flatMap((check) => { + if (!isActions(check)) return [check]; + let owner; + try { owner = ownership.currentFor(check.check_suite?.id); } + catch { throw new Error('A GitHub Actions check has no verified workflow owner.'); } + if (owner === null) return []; + // A rerun can retain completed jobs from its prior attempt until new jobs register. + return [owner.check.status === 'completed' ? check : { ...check, status: 'in_progress', conclusion: null }]; + }); + }; const terminal = new Set(['completed']); const PER_PAGE = 100; // #300: page the listing to exhaustion — a head with more than @@ -227,10 +342,6 @@ jobs: // being killed by the job timeout mid-line. - // Local specialization: 40min because the slowest required - - // sibling (Unit Tests (linux)) runs ~28min on PRs. - const deadline = Date.now() + 40 * 60 * 1000; while (Date.now() < deadline) { // #315: re-read the anchor every cycle — the transition @@ -247,7 +358,7 @@ jobs: + (error && error.message ? error.message : String(error))); process.exit(1); } - const runs = await fetchAllCheckRuns(); + const runs = await currentExecutionChecks(await fetchAllCheckRuns()); // #119: re-runs keep every same-name run; terminality is // decided on the truth run — latest started_at, ties broken // by the higher check-run id (docs/reference.md) — never on @@ -304,6 +415,6 @@ jobs: - name: specgit finish - run: specgit finish --json + run: '"$RUNNER_TEMP/specgit-cli/node_modules/.bin/specgit" finish --json' env: GH_TOKEN: ${{ github.token }} diff --git a/.gitignore b/.gitignore index f2caa4f7c..56e3391c7 100644 --- a/.gitignore +++ b/.gitignore @@ -62,4 +62,17 @@ tsconfig.tsbuildinfo # >>> specgit: local delivery assets (managed by specgit init) >>> /.specgit.yaml /spec_git/ +/.opencode/hooks/specgit-merge-guard.sh +/.opencode/command/specgit-issue.md +/.agents/skills/specgit-issue/SKILL.md +/.opencode/command/specgit-finish.md +/.agents/skills/specgit-finish/SKILL.md +/.opencode/command/specgit-doctor.md +/.agents/skills/specgit-doctor/SKILL.md +/.opencode/command/specgit-pr.md +/.agents/skills/specgit-pr/SKILL.md +/.opencode/command/specgit-status.md +/.agents/skills/specgit-status/SKILL.md +/.local/state/ +/.local/cache/ # <<< specgit: local delivery assets (managed by specgit init) <<< diff --git a/.opencode/hooks/specgit-merge-guard.sh b/.opencode/hooks/specgit-merge-guard.sh index 01981b809..49e34c573 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/AGENTS.md b/AGENTS.md index 34724ac48..9e32af2d0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -292,42 +292,55 @@ 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. -- Issue bodies are filled at bootstrap, from the conversation: right after - `specgit issue` succeeds, edit each issue it created (`gh issue edit <n>`) - with the discussed Why / Scope / Approach / Acceptance, then implement. - The PR scaffold's placeholders are advisory — fill those sections in as - you deliver; the closing references are the only body gate. The PR body - is written once at creation; no SpecGit command edits an existing PR - body, and the repository's own pull-request template is never read. -- A draft pull request always fails the verdict (`pr_draft`): before + 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 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 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. -- Finish with `specgit finish`: the verdict, derived from real git, PR, - and CI evidence. Exit code 0 is the only "done". +- `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 + `target_branch`, fresh acceptance, and all current-head CI, then confirms + the merge and every bound issue closure before reporting completed. + A failed closure remains recoverable and is never reported as completed. ### Issue tags +- 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 + `specgit init --force --configure-rules`. - Every bootstrap applies the title's `kind::<type>` member automatically; pass `--tags <a,b>` to choose the full set explicitly. - Selection is pool-first: existing on-spec labels win verbatim; anything missing is seeded from the built-in `kind::` catalog or the policy's `tags:` declarations. Unknown vocabulary exits 2 naming the universe. -- Choose with restraint: at most one label per axis, none when unsure — +- Choose at most one label per axis; omit uncertain optional labels and + keep every label required by the selected policy. Existing pool labels + cannot override that policy — off-spec pool labels are reported (`tag_pool_dirty` warnings are for humans) and never renamed by SpecGit. ### Repair and diagnostics -- `specgit pr` repairs the pull-request binding: with no arguments it - auto-discovers the pull request for this head branch, errors with a fix +- `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 @@ -337,14 +350,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`). 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 @@ -366,28 +388,51 @@ verified on its own evidence, split it before binding. ### Agent contract essentials -- **SpecGit is the default way of working here.** Any non-trivial - task — a feature, a fix, a refactor, a docs change — is a delivery: +- **SpecGit is the default delivery workflow here.** An intended tracked + change — a feature, a fix, a refactor, a docs change, or shared rules — is a delivery: work items live in this tracker as issues, never in private task lists or conversational checklists. The trigger is the decision to start: the moment the conversation settles and you begin turning the plan into changes, the FIRST action is - `specgit issue <type>: <title>...` — before any file edit. + `specgit issue <type>: <title>...` — before tracked implementation edits. + Preparing temporary body files for bootstrap is part of this first step. Working without a binding is a contract violation, not a style - choice. Immediately after bootstrap, fill each issue body - (Why / Scope / Approach / Acceptance) from the discussion with - `gh issue edit`, then implement. Mid-conversation inventories + choice. After bootstrap, verify each issue contains the discussed + Why / Scope / Approach / Acceptance and fill only missing content with + `gh issue edit` or `glab issue update`, + then implement. Mid-conversation inventories ("let me list everything to do") become issues, not chat artifacts. Trivial replies and read-only questions need none of this. -- The one rule: a delivery is done if and only if `specgit finish` - exits `0`. Never declare completion from task lists, file states, or - test runs you performed yourself. +- Local maintenance: installing or upgrading the CLI and running `init` / + `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. 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; + local maintenance and merging do not imply publication. +- `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/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. From 07e8787d7c8bcdcd04961c73c5264e79997d9101 Mon Sep 17 00:00:00 2001 From: Lex <sunsan05@Gmail.com> Date: Mon, 7 Sep 2026 12:08:41 +0800 Subject: [PATCH 4/6] fix(hooks): repair execution, decisions and lifecycle --- .../core/src/plugin/skill/configure-hooks.md | 63 ++- packages/opencode/src/hook/agent-tools.ts | 160 ++---- .../src/hook/extensions/hot-reload.ts | 16 +- packages/opencode/src/hook/file-changes.ts | 37 ++ .../opencode/src/hook/readonly-command.ts | 161 ++++++ packages/opencode/src/hook/rewake.ts | 14 + packages/opencode/src/hook/schema.ts | 54 ++ packages/opencode/src/hook/session-hooks.ts | 57 +- packages/opencode/src/hook/settings.ts | 492 ++++++++++-------- packages/opencode/src/hook/trigger-result.ts | 21 + packages/opencode/src/permission/index.ts | 65 +-- .../routes/instance/httpapi/groups/session.ts | 5 + .../instance/httpapi/handlers/session.ts | 14 +- packages/opencode/src/session/prompt.ts | 296 +++++++---- packages/opencode/src/session/tools.ts | 374 +++++++------ packages/opencode/src/tool/task.ts | 2 +- .../test/hook/handler-cancellation.test.ts | 159 ++++++ .../opencode/test/hook/http-handler.test.ts | 45 ++ .../test/hook/prompt-admission.test.ts | 386 ++++++++++++++ .../test/hook/readonly-command.test.ts | 57 ++ .../test/hook/runtime-boundaries.test.ts | 292 +++++++++++ .../test/hook/tool-boundaries.test.ts | 236 +++++++++ .../test/hook/warn-unsupported.test.ts | 35 +- .../test/server/session-hooks-api.test.ts | 27 + packages/sdk/js/src/v2/gen/sdk.gen.ts | 7 + packages/sdk/js/src/v2/gen/types.gen.ts | 14 + 26 files changed, 2352 insertions(+), 737 deletions(-) create mode 100644 packages/opencode/src/hook/file-changes.ts create mode 100644 packages/opencode/src/hook/readonly-command.ts create mode 100644 packages/opencode/src/hook/schema.ts create mode 100644 packages/opencode/test/hook/handler-cancellation.test.ts create mode 100644 packages/opencode/test/hook/prompt-admission.test.ts create mode 100644 packages/opencode/test/hook/readonly-command.test.ts create mode 100644 packages/opencode/test/hook/runtime-boundaries.test.ts create mode 100644 packages/opencode/test/hook/tool-boundaries.test.ts diff --git a/packages/core/src/plugin/skill/configure-hooks.md b/packages/core/src/plugin/skill/configure-hooks.md index 7718f6489..cf806c94f 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 3a7f98a0f..c4f2d1ca5 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 48b0eebe9..c9ded4fbc 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 000000000..8d675e052 --- /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 000000000..11cd7ca1a --- /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 fbee871d5..df4c2e7e2 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 000000000..34470cebb --- /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 0e5189a1d..07789f96e 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 c9851b257..5edd32900 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 dae152bda..b5eef5c7a 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 fa066d5a3..48afecf29 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 2e15bf35a..904da3293 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 3c7cb06b0..3e22950f5 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 9074c71f2..540b9f301 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 c0aa7494d..ed2f1d640 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 * 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,180 @@ 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.catch((error: unknown) => + Effect.gen(function* () { + // 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 }) + error = withHookFailure(error, failResult) + } + return yield* Effect.fail(error) + }), + ), ), ), - ), - ) + ) }, }) } @@ -520,8 +552,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 +576,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 +601,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 +611,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,19 +701,18 @@ 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) @@ -687,26 +721,58 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { }).pipe( Effect.catch((error: unknown) => Effect.gen(function* () { - // SettingsHook PostToolUseFailure 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 }) + error = withHookFailure(error, failResult) } return yield* Effect.fail(error) }), ), ), ) + } 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 d08a5edef..2d76b5518 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 000000000..b3a4e707a --- /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 ade67bdcb..7d0882d2b 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 000000000..a0ff22fce --- /dev/null +++ b/packages/opencode/test/hook/prompt-admission.test.ts @@ -0,0 +1,386 @@ +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 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 000000000..0f290af3d --- /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 000000000..9c0e109d3 --- /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 000000000..f61718164 --- /dev/null +++ b/packages/opencode/test/hook/tool-boundaries.test.ts @@ -0,0 +1,236 @@ +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" + +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") + }), + ) +}) + +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 23ea58787..360ef7852 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 9aaf012f1..c54918c32 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 f376c2222..f07f68686 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 10d0a3a90..c18e600a2 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 } From 98a558829873174af53c8c0dd324872daae9156b Mon Sep 17 00:00:00 2001 From: Lex <sunsan05@Gmail.com> Date: Mon, 7 Sep 2026 12:18:30 +0800 Subject: [PATCH 5/6] chore: record delivery binding for hooks-runtime-reliability --- .specgit.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.specgit.yaml b/.specgit.yaml index ce102a001..2995f8957 100644 --- a/.specgit.yaml +++ b/.specgit.yaml @@ -1,8 +1,7 @@ version: 1 delivery: hooks-runtime-reliability context: - kind: worktree - label: opencode-hooks-fix + kind: branch branch: fix/557-hooks-runtime-reliability issues: - 557 From b56d931b28dfa0d2a97ff3f151d1ece2bac61d59 Mon Sep 17 00:00:00 2001 From: Lex <sunsan05@Gmail.com> Date: Mon, 7 Sep 2026 12:29:08 +0800 Subject: [PATCH 6/6] fix(hooks): deliver feedback for tool defects and rejected MCP calls --- packages/opencode/src/session/tools.ts | 21 ++++++--- .../test/hook/prompt-admission.test.ts | 39 +++++++++++++++ .../test/hook/tool-boundaries.test.ts | 47 +++++++++++++++++++ 3 files changed, 100 insertions(+), 7 deletions(-) diff --git a/packages/opencode/src/session/tools.ts b/packages/opencode/src/session/tools.ts index ed2f1d640..3e31a5474 100644 --- a/packages/opencode/src/session/tools.ts +++ b/packages/opencode/src/session/tools.ts @@ -18,7 +18,7 @@ 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 { Session } from "./session" import { SessionProcessor } from "./processor" @@ -260,8 +260,10 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { } return output }).pipe( - Effect.catch((error: unknown) => + 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 @@ -281,9 +283,11 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { ), ) yield* SettingsHook.landSystemMessages(failResult, { sessionID: input.session.id }) - error = withHookFailure(error, failResult) + 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) }), ), ), @@ -719,8 +723,10 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { } return output }).pipe( - Effect.catch((error: unknown) => + Effect.catchCause((cause) => Effect.gen(function* () { + if (Cause.hasInterrupts(cause)) return yield* Effect.failCause(cause) + const error = Cause.squash(cause) if (settingsHook) { const failResult = yield* settingsHook .trigger( @@ -737,9 +743,10 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { Effect.catch(() => Effect.succeed<TriggerResult>({ additionalContexts: [], systemMessages: [] })), ) yield* SettingsHook.landSystemMessages(failResult, { sessionID: input.session.id }) - error = withHookFailure(error, failResult) + 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) }), ), ), diff --git a/packages/opencode/test/hook/prompt-admission.test.ts b/packages/opencode/test/hook/prompt-admission.test.ts index a0ff22fce..7dd470806 100644 --- a/packages/opencode/test/hook/prompt-admission.test.ts +++ b/packages/opencode/test/hook/prompt-admission.test.ts @@ -306,6 +306,45 @@ it.live("session cancellation aborts a post-tool hook and preserves the complete ), ) +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 }) { diff --git a/packages/opencode/test/hook/tool-boundaries.test.ts b/packages/opencode/test/hook/tool-boundaries.test.ts index f61718164..385655b57 100644 --- a/packages/opencode/test/hook/tool-boundaries.test.ts +++ b/packages/opencode/test/hook/tool-boundaries.test.ts @@ -20,6 +20,7 @@ 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)) @@ -115,6 +116,52 @@ describe("real SessionTools call path", () => { 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))