Skip to content

fix: restore throw-based abort for /acp subcommands (#398) - #399

Merged
ranxianglei merged 4 commits into
masterfrom
2026-09-15_restore-command-abort-throw
Sep 15, 2026
Merged

ranxianglei merged 4 commits into
masterfrom
2026-09-15_restore-command-abort-throw

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Problem

Since v1.17.0 (regression commit 5fd8f5ee9, PR #297), running /acp status, /acp stats, or /acp context leaked the subcommand word into the model as a plain user message, triggering a spurious model call (~40K input tokens in the reporter's A/B test). Bare /acp, /acp help, and /acp export were unaffected.

Root cause chain (verified against opencode 1.18.x source)

  1. opencode's Plugin.trigger aborts a command.execute.before hook only when the hook throws (packages/opencode/src/plugin/index.ts:284-296); the hook output type has only { parts } — there is no cancel signal (@opencode-ai/plugin CommandOutput).
  2. ACP registers the acp command with template: "" (no $ARGUMENTS) (index.ts:194-197); for placeholder-less templates opencode appends the raw arguments to the template (packages/opencode/src/session/prompt.ts:1390-1393) and sends the result to the model.
  3. PR fix: remove __DCP_CONTEXT_HANDLED__ throw that leaked to error logs (#296) #297 changed the stats/status/bare branch from throw new Error("__DCP_CONTEXT_HANDLED__") to a bare return, and removed the trailing throw after the context fallback — so the command was no longer aborted and the argument word reached the model. Its replacement test asserted the wrong property (handler resolves) instead of the observable contract (command aborted).

Fix

  • lib/hooks.ts createCommandExecuteHandler: restored throw new Error("__DCP_CONTEXT_HANDLED__") after each handled branch (stats/status/bare, context fallback) — back to ≤v1.16.0 semantics. Added a [FIX #398] comment documenting that throwing is the only available abort mechanism and must not be converted back to a return.
  • Accepted cost: opencode ≥ 1.18.18 logs one level=ERROR line per /acp invocation (the log noise [Bug]: Error: __DCP_CONTEXT_HANDLED__ #296 complained about). A spurious ~40K-token model call is far worse than one log line. Follow-up: negotiate a silent-abort API upstream (hook output control field / explicit cancel signal); tracked in devlog §7.

Tests

  • tests/hooks-permission.test.ts: replaced the flawed [Bug]: Error: __DCP_CONTEXT_HANDLED__ #296 test with a regression guard: every handled branch (stats, status, "", context, help, export --stdout) must reject with the sentinel and record exactly one session.prompt with noReply: true + ignored: true (notification path intact, no plain-message leak). Non-acp pass-through preserved. Legacy deny-permission test tightened to the same abort assertion.
  • Mutation check: temporarily reverting lib/hooks.ts makes the guard fail (not ok 4 - command execute aborts every handled /acp branch by throwing (regression guard #398)); restored → green. This confirms the test catches the original bug.
  • npm run typecheck PASS; full suite 1264/1264 PASS.

Review

Dual-agent review per AGENTS.md §5.3 (code) + §5.6 (tests): both reviewers returned APPROVE-WITH-NITS, no blockers/majors; all actionable nits addressed in follow-up commit 2202bcc.

Devlog: devlog/2026-09-15_restore-command-abort-throw/

Fixes #398

…398)

PR #297 (fixing #296's ERROR-log noise) downgraded two throw statements in
createCommandExecuteHandler to normal returns. opencode's Plugin.trigger only
aborts a command on hook errors, and the acp command is registered with
template: "" (no $ARGUMENTS), so a normal return lets opencode append the raw
arguments to the empty template and send them to the model as a user message.

Since v1.17.0 this leaked "status"/"stats"/"context" as plain user messages
and triggered spurious ~40K-token model calls (#398).

Restore the sentinel throw in both spots and add a regression-guard test suite
that asserts every handled branch rejects with __DCP_CONTEXT_HANDLED__ and that
the notification prompt carries noReply:true + ignored:true. The guard was
verified to fail against the buggy code (mutation check).

Known accepted cost: one level=ERROR log line per /acp invocation on
opencode >= 1.18.18 (#296). Follow-up: negotiate a silent-abort API upstream.
@github-actions

Copy link
Copy Markdown
Contributor

📦 Built Plugin Artifact

Branch: 2026-09-15_restore-command-abort-throw (0b62542)

Option A — Install from npm PR tag (recommended)

opencode plugin opencode-acp@pr-399 --global

Each push to this PR publishes a new version under the pr-399 npm tag.

Option B — Install from GitHub

opencode plugin "github:ranxianglei/opencode-acp#2026-09-15_restore-command-abort-throw" --global

Option C — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf opencode-acp-pr399.tgz
cp -r package/dist ~/.cache/opencode/packages/opencode-acp@latest/node_modules/opencode-acp/dist
  1. Restart opencode to pick up changes.

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Verification report for PR #399 (fix for #398) — this is completed agent work, so per protocol I pulled the branch and verified instead of redoing it.

What I verified (all independently, not just trusting the description):

  1. Code diff is minimal and correct — 4 files, +235/−20: lib/hooks.ts (+11/−3), tests/hooks-permission.test.ts, REQ.md + WORKLOG.md in devlog/2026-09-15_restore-command-abort-throw/. No version bump, no unrelated churn. All four handled branches in createCommandExecuteHandler (lib/hooks.ts:520-538) now end with throw new Error("__DCP_CONTEXT_HANDLED__"): stats/status/bare, export, help, context-fallback. Non-acp commands still pass through without throwing; the [FIX #398] comment documents why these throws must never become returns.
  2. Root cause independently confirmed against opencode source — current opencode main packages/opencode/src/plugin/index.ts: trigger runs each hook via yield* Effect.promise(async () => fn(input, output)). A throwing hook fails the whole trigger effect (aborting command execution); a normal return just continues, and the hook output type carries no cancel field. So "throw-or-leak" is exactly right — there is no other available abort mechanism today.
  3. Local build+test on head 0b62542: npm run typecheck PASS; full suite 1264/1264 PASS, 0 fail — matches the PR claim exactly.
  4. Reproduced the mutation check myself: temporarily reverting the two restored throws makes the new guard fail (not ok 4 - command execute aborts every handled /acp branch by throwing (regression guard #398), plus the tightened deny-permission test); restoring the fix → all 10 tests in the file green. The test genuinely catches the original v1.17.0 regression — it asserts the observable contract (reject with sentinel and one session.prompt with noReply: true + parts[0].ignored: true), not the flawed "handler resolves" property from [Bug]: Error: __DCP_CONTEXT_HANDLED__ #296.
  5. CI on head commit: pr-validation ✅, build ✅, test (22) ✅, test (24) ✅, build-artifact ✅; e2e still in progress at time of writing — I'll follow up here when it lands.
  6. Merge readiness: branch is behind master (master moved past the paper-preprint merge docs: add paper preprint v0.2 (EN+ZH) + README links #394), but a git merge-tree dry-run against current master is conflict-free.
  7. Devlog: REQ.md (repro, constraints) + WORKLOG.md (results, rollback = revert 2c59c07, follow-ups incl. upstream silent-abort API negotiation) — compliant with §5.1.2.

Assessment: root cause analysis is sound (I confirmed the mechanism against opencode source myself), the fix restores ≤v1.16.0 semantics with the accepted cost of one ERROR log line per /acp call on opencode ≥ 1.18.18, and the regression guard is real (mutation-checked twice now). Dual-agent review already returned APPROVE-WITH-NITS with nits addressed in 2202bcc. No blockers found.

Ready for human merge once e2e turns green. Per AGENTS.md §5.1.1.2 I will not merge — please merge yourself: #399

中文摘要:独立验证了该修复——恢复 /acp 各子命令分支的 throw 中断(opencode 只有 hook 抛错才能中止命令,已对照 opencode 源码确认机制),本地 typecheck + 1264/1264 测试全绿,并亲手复现了变异检查(回退修复后新回归守卫确实失败),CI 除 e2e 仍在跑外全部通过、与 master 无合并冲突,可以合并。

@ranxianglei
ranxianglei merged commit aeaf9b4 into master Sep 15, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: /acp status|stats|context 子命令参数被作为普通消息发给模型(v1.17.0 回归)

1 participant