Skip to content
This repository was archived by the owner on Sep 29, 2026. It is now read-only.

fix: keep the classifier out of the reasoning budget and escalation with a human - #20

Closed
bybybybyb wants to merge 1 commit into
NanmiCoder:mainfrom
bybybybyb:fix/classifier-reliability-escalation
Closed

bybybybyb wants to merge 1 commit into
NanmiCoder:mainfrom
bybybybyb:fix/classifier-reliability-escalation

Conversation

@bybybybyb

Copy link
Copy Markdown

Stop reasoning tokens from starving the classifier, and make escalation reachable

Two reported defects, both reproduced before any production change:

  1. Error: [auto-mode classifier unavailable; action denied] classifier response reached its output limit — frequent, and it fail-closes ordinary work to deny.
  2. Agents "try to hack instead of asking for escalation": after a denial the model looks for an equivalent route rather than requesting authority.

Root cause of (1)

The native classifier reuses the Session route but constructed GenerateOptions without reasoningEffort (src/dsh-classifier.ts). In this Harness that is not "no reasoning":

  • LlmService.adapterStream calls resolveCallWithInfo, which materializes reasoning.defaultEffort whenever the caller omitted an effort (@deepseek-ai/dsh-llm/lib/index.js:2115-2131).
  • The DeepSeek adapter advertises defaultEffort = HIGH_REASONING_EFFORT when neither thinking nor reasoningEffort is configured for the deployment (@deepseek-ai/dsh-llm-deepseek/lib/index.js:1591-1597) — which is exactly this profile's llm-deepseek: {}.
  • serializeRequest therefore put thinking: {type:'enabled'}, reasoning_effort:'high' on the wire next to max_tokens: 1024 (same file, :236-243).
  • Reasoning tokens shared the output cap, so the wire returned finish_reason: "length", which the adapter maps to {kind:'max-tokens'} (:1135); collectResponse threw classifier response reached its output limit, and tools/pre-execute failed closed to deny.

The 3-consecutive-failure manual fallback rarely fired because any successful classification reset the counter.

Reproduced live in an Auto session: a read-only compound command
cd … && wc -l … && grep -n "credential\|DEEPSEEK\|apiKey\|env" … | head -30
was denied with exactly that message, while the same reads issued as simple commands were allowed. A deterministic probe of assessShell confirmed the compound form is classifier-eligible (ask, reason shell command reads potentially sensitive credential or environment data).

Root cause of (2)

Three compounding causes:

  • The Agent's natural escalation channel grants nothing. ask_user_question answers are returned as an ordinary tool result (@deepseek-ai/dsh-tool-ask-user/lib/index.js:7), and trustedUserMessages only reads user/message events with source.kind === 'user'. So the agent asked, the user agreed, the retry was denied again — and the agent learned that asking is useless while rerouting works.
  • Reviewer unavailability was a silent deny, including for an explicit one-shot escalation, with no recovery guidance (only the third consecutive failure became an ask).
  • The Agent guidance never said what a refusal means, so nothing distinguished "replan" from "get authority" from "stop and hand it to the user".

The change

Classifier reliability (src/dsh-classifier.ts)

  • Pin a reasoning effort (classifierReasoningEffort, default off) so thinking cannot consume the answer budget.
  • Validate the pin against the exact route's advertised efforts via the public ctx.llm.resolveModelInfo. A route with no reasoning metadata rejects any explicit effort with UNSUPPORTED_REASONING_EFFORT, so it receives no effort at all; a route that cannot disable thinking receives the largest accepted cap (4096) instead.
  • Retry once without the pin if an adapter still refuses it, so an unverifiable probe cannot make every call on a route fail closed.
  • Raise the ordinary cap from 1024 to 2048, and give the classifier its own constants rather than repeating literal numbers in src/index.ts.
  • A truncated response is refused outright. An earlier revision of this branch tried to salvage a decision from a complete object inside a max-tokens response; adversarial review showed that the "was this the model's own conclusion or text it merely quoted from untrusted input?" question has no sound provenance signal, and that any string-containment heuristic is evadable (different key order, whitespace, or \uXXXX escaping). Emitting no partial trust keeps the classifier's failure mode a denial, which is what it already does with every other malformed answer.

Escalation reachability (src/index.ts)

  • An explicit one-shot danger-full-access escalation is never denied merely because the reviewer is unavailable, and a widening the reviewer declines to clear is never handed to the tool body either. The plugin raises that single approval request itself and arms the same exact grant, so the official seam — where a tool implements one — resolves against that one human decision instead of prompting twice, while a tool that simply ignores the sandbox fields cannot run the call with nobody asked.
    • The first revision of this branch delegated to the tool body, and review correctly rejected it: only bash, pwsh, write and edit implement the official escalation seam (@deepseek-ai/dsh-tool-fs/lib/index.js calls resolvePolicy/approveEscalation only at :648 for write and :797 for edit; read at :332 never does), while undeclared argument keys pass parameter validation (@deepseek-ai/dsh-tools/lib/index.js:466-467 only rejects extras when additionalProperties:false is explicit, and parameterSchemaSpecToJsonSchema at :800-807 never sets it). An inert sandbox_permissions argument would therefore have carried a sensitive out-of-workspace read past a failed reviewer with nobody asked.
    • The same hole existed on the sibling classifier-ask branch and was pre-existing at HEAD: there too the plugin trusted the tool body, so attaching the two inert fields turned a human prompt into silent execution. Both branches now raise the approval themselves.
    • With no composed approval service, or under an approval: never policy, the harness turns that ask into a rejection, so the path still fails closed.
    • An ask a human decided is not recorded as a refusal, so an approved call that later fails for its own reason is never handed "do not repeat this escalation" guidance.
  • Refusals are classified for the Agent, and a post-call recovery notice is attached for the three classes it must not work around: authority (needs authority it does not hold), escalation (a refused escalation must not be repeated), and hard (monotonic). Deterministic and invalid-request refusals deliberately get no notice — rewriting the call is their intended recovery.
  • The notice is triggered from a call-scoped registry written by the decision that produced the refusal (AutoRefusalNotices), not by parsing the tool's error text: a tool result is untrusted data, so a tool returning its own [auto-mode ...]-looking error must not make the trusted plugin inject guidance.
  • The Agent guidance now names each refusal class, states that a hard denial is monotonic and must be handed to the user, forbids reaching a denied effect by an equivalent route, warns that a wider sandbox is not a substitute for authority it does not supply, and states that an ask_user_question answer is information and never authorization.

Deliberately not changed

  • No detector logic in src/shell.ts / src/paths.ts. The classifier's own false positive observed during reproduction (a read-only grep whose search pattern mentions env is escalated to the reviewer) is real but out of scope here.
  • ask_user_question answers are still not authority. Promoting them would let a prompt-injected agent launder a request through a user click, which is the "magic words" failure this design avoids; the fix is to make the sanctioned path reachable and to say so.
  • The deterministic shell and path policy, and the exact-grant semantics, are untouched.

Verification

  • pnpm verify (typecheck, build, full suite, package contract): 206 passing / 27 skipped against the upstream 0.1.10 baseline of 178 passing / 27 skipped. New coverage: classifier effort/cap/retry/refusal cases, the composed refusal-and-escalation cases, and a set of pairs that are discriminating in both directions — a tool with no escalation seam cannot execute unapproved while the reviewer is down but does execute once the human approves; an escalation raises exactly one approval (a missing grant would make it two); a classifier ask on an escalation prompts once and runs it on approval; a classifier deny on an escalation denies with no request; an approved call that later fails on its own carries no refusal notice; and a subagent's denied widening carries its own notice.
  • Repository CI gates also pass locally: node scripts/verify-maintenance.mjs (6 exact hosts, 120 pinned skill files) and node --test scripts/harness-doctor.test.mjs (7/7).
  • A directed real-wire probe confirmed the mechanism against the configured provider (deepseek-official / deepseek-flash) using the plugin's own classifier prompt:
    • the pre-fix request shape really does put thinking: {type:'enabled'}, reasoning_effort:'high' on the wire next to max_tokens: 1024, so omitting the effort materializes the adapter default as claimed;
    • reasoning shares the answer cap and dominates it — across 7 samples on payloads up to 5245 characters (the sanitizer's ceiling) the pre-fix shape produced 138, 233, 252, 271, 315, 404, 438 and 586 output tokens with 940–2556 characters of reasoning;
    • the fixed shape (thinking disabled) produced 31–48 output tokens with zero reasoning characters, a ~10–20× margin under the cap;
    • a single classifier call returning finish_reason: "length" at cap 1024 was not reproduced: the peak sample was 586 output tokens, so the field symptom needs a longer reasoning run than this route produced here — plausibly a session whose own route is a heavier reasoning model, since the classifier classifies on the session's route rather than the agent default. The field evidence remains the measured occurrences (155 raw matches for the error string across stored transcripts, including real tool errors) plus a live hit during this work.
  • Three independent adversarial reviewers reviewed the first revision (parser/security, denial-and-escalation behaviour, test quality), and two more re-reviewed the frozen revision. Their findings are fixed here:
    • the widening hand-off described above (Critical), including its classifier-ask sibling, which was pre-existing at HEAD and equally silent;
    • the truncated-response salvage and its evadable echo guard (Critical/Important) — the salvage is gone and a max-tokens finish is refused before parsing;
    • guidance that told the agent to repeat an escalation that had just been refused (Important) — a distinct escalation class and notice now say the opposite;
    • a refusal notice attached to an approved call that merely failed later (Important) — refusals are recorded only where the plugin actually refuses;
    • a spoofable notice trigger (Minor) — all four notices now come from a call-scoped registry written by the refusing decision, with no error-text matching left;
    • [auto-mode invalid sandbox request] guidance that described the wrong corrective action (Minor) — corrected;
    • a non-object capability-probe result escaping the timeout/cancellation translation (Minor) — the probe now runs inside that try;
    • an escalation producing two approval prompts (Minor) — the exact grant is armed so one human decision covers it, including on the pre-classification sanitize path;
    • a subagent's denied widening carrying no per-call guidance (Minor) — it now has its own delegated class and notice;
    • a caller-cancelled classifier call being labelled as needing authority (Minor) — cancellations record no refusal;
    • a finish kind that is not stop being parsed anyway (Minor) — the success kind is now allow-listed, which matters because FinishReasonMap is merge-extensible.
  • A third review round verified on this revision that the escalation Critical is closed for every tool, that the armed grant cannot suppress the human prompt (the plugin's ask reason always begins with [auto-mode , the grant reason with escalate , and AutoApprovalGrants.decide compares exactly), and found no remaining Critical or Important issue.

Residual test-fidelity note, not a defect: the composed approval double answers through the real approval/request seam but bypasses ApprovalService.decide, so the approval: never policy path is asserted by a scripted outcome rather than by the real policy short-circuit.

Not verified here

  • The platform-gated real-Seatbelt suites (tests/sandbox-business.spec.ts, tests/windows-sandbox-business.spec.ts) skip when sandbox-exec cannot apply a nested sandbox, which is the case inside an Auto-mode session. They run in the CI matrix.
  • scripts/acceptance/run-real-api.mjs was not executed: it needs a supported installed runtime and a nested OS sandbox, neither available in the authoring session (the only local runtimes are 0.1.5-rc.3 and 0.1.6-alpha.2, which compatibility.json does not accept, and sandbox-exec refuses to apply inside another sandbox). The directed wire probe described under Verification was used instead.

…ith a human

The native classifier reused the Session route without a reasoning effort, so the
Harness materialized the DeepSeek adapter's advertised `high` defaultEffort,
thinking was enabled, and reasoning tokens shared the whole 1024-token answer
budget. The wire returned `finish_reason: "length"`, the plugin mapped that to
"classifier response reached its output limit", and every classifier-eligible
call failed closed to deny.

- Pin a reasoning effort (`off` by default) after checking the exact route's
  advertised efforts, because a route with no reasoning metadata rejects any
  explicit effort. Retry once without the pin if an adapter refuses it, and give
  a route that cannot disable thinking the largest accepted cap.
- Raise the ordinary cap from 1024 to 2048, and refuse a truncated response
  before parsing: recovering a decision from a partial answer cannot separate the
  model's own conclusion from text it merely quoted out of untrusted input.
- Never let an escalation the classifier did not affirmatively allow be delegated
  to the tool body. Only some tools implement the official escalation seam and
  inert sandbox fields pass parameter validation, so the plugin raises that one
  approval itself and arms the exact grant, which keeps the human decision single
  without letting a seam-less tool run the call.
- Classify every refusal for the Agent and drive the recovery notice from a
  call-scoped registry written by the refusing decision, so no tool can forge
  guidance with its own error text.

Verification: pnpm verify passes 206 tests with 27 platform-gated skips against
the upstream 0.1.10 baseline of 178 + 27; the maintenance contract (6 exact
hosts, 120 pinned skill files) and the harness doctor tests (7) also pass.
@bybybybyb

Copy link
Copy Markdown
Author

拆分成了两个聚焦的 PR,请以它们为准:

  • 分类器推理预算(输出上限报错):bybybybyb:fix/classifier-reasoning-budget
  • 升级门禁与拒绝恢复指引:bybybybyb:fix/escalation-human-gate

两者 diff 互不重叠,任意顺序合并都不会冲突;合并后测试套件与未拆分时完全一致(206 passed / 27 skipped)。

Split into two focused PRs, please review those instead:

  • classifier reasoning budget (the output-limit error): bybybybyb:fix/classifier-reasoning-budget
  • escalation gating and refusal recovery: bybybybyb:fix/escalation-human-gate

They touch disjoint hunks and merge cleanly in either order; together they reproduce the undivided suite exactly (206 passed / 27 skipped).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant