mirror of
https://github.com/mksglu/context-mode.git
synced 2026-10-02 04:14:38 +08:00
fix(routing): drop negation framing from CASE A deny reasons (#683)
All four redirect-style deny reasons in hooks/core/routing.mjs (curl/wget, inline HTTP, build tools, WebFetch) rewritten to fully positive imperative voice per ADR-0002 + ADR-0003. - Removed "(context-window optimization, NOT a network restriction)" - Removed "Do NOT retry with curl/wget|Bash|WebFetch" - Replaced "retry if it fails" hedge with imperative "Retry the same call on a transient DNS error (EAI_AGAIN, ETIMEDOUT, ENETUNREACH)" Cross-LLM rationale: negation framing primes LLM attention on the forbidden item (ironic process theory). Positive routing intent + explicit capability affirmation + imperative next-action work uniformly across Claude/GPT/Gemini/Llama. ADR-0003 amended with §Amendment noting the empirical rationale. Contract tests in tests/core/server.test.ts gain two guards (PR #683 follow-up) that fail loud if "NOT a network" or "Do NOT retry" reappear.
This commit is contained in:
Mert Koseoglu
@@ -1,6 +1,7 @@
|
||||
# ADR-0003 — Routing deny reasons: redirect ≠ restriction
|
||||
|
||||
- **Status**: Accepted
|
||||
- **Status**: Accepted — amended 2026-05-24 (PR #683 follow-up) to drop the
|
||||
`"NOT a network restriction"` negation prescription; see §Amendment.
|
||||
- **Date**: 2026-05-24
|
||||
- **PR**: #683 (substitutes #654)
|
||||
- **Motivating bug**: kerneltoast / @noctivoro / Mert reproduced on Opus 4.6
|
||||
@@ -39,11 +40,19 @@ Routing deny reasons MUST distinguish two cases:
|
||||
The action is supported, via a different tool, for context-window or
|
||||
efficiency reasons.
|
||||
|
||||
- **Opening verb**: "redirected"
|
||||
- **MUST state**: "this is NOT a network / security restriction"
|
||||
- **MUST specify**: the alternative tool to use
|
||||
- **MUST end with**: an imperative next-action — e.g. "retry if it fails
|
||||
with a transient error (EAI_AGAIN, ETIMEDOUT, ENETUNREACH)"
|
||||
- **Opening verb**: "redirected to <ctx_tool> for context-window
|
||||
efficiency" — affirmative routing intent, no negation.
|
||||
- **MUST affirm capability**: "<ctx_tool> has full network access"
|
||||
(positive frame of the same signal the old `NOT a network restriction`
|
||||
parenthetical tried to deliver).
|
||||
- **MUST specify**: the alternative tool to use, by name, as an
|
||||
imperative call (e.g. `Call ctx_fetch_and_index(url, source) now`).
|
||||
- **MUST end with**: a positive imperative retry hint —
|
||||
`"Retry the same call on a transient DNS error (EAI_AGAIN,
|
||||
ETIMEDOUT, ENETUNREACH)"`.
|
||||
- **MUST NOT contain**: bare-NOT negations (`NOT a network restriction`,
|
||||
`Do NOT retry with curl`, etc.). See §Amendment below for the
|
||||
empirical rationale.
|
||||
|
||||
The word `BLOCKED` MUST NOT appear bare in CASE A. It is reserved for
|
||||
true policy denial (CASE B) where the agent's correct response IS to
|
||||
@@ -58,6 +67,38 @@ sandbox capability.
|
||||
- **MUST cite**: the pattern or rule violated
|
||||
- **MAY suggest**: a safe alternative
|
||||
|
||||
## Amendment (PR #683 follow-up, 2026-05-24)
|
||||
|
||||
The original CASE A rubric required the parenthetical `"this is NOT a
|
||||
network / security restriction"`. Mert flagged on review that this
|
||||
prescription itself violates ADR-0002 rubric #2 (affirmative beats
|
||||
negation): the bare-NOT construct primes the very frame it tries to
|
||||
deny (ironic process theory). `TOOL-DESCRIPTIONS-AUDIT.md §2 Probe 3`
|
||||
already documented this — the NOT-parenthetical regressed Haiku
|
||||
capitulation rate from 0/6 → 2/6 vs the original "blocked" wording.
|
||||
|
||||
PR #654 fixed the headline word (`blocked` → `redirected`) but kept the
|
||||
sibling negation in the very next clause. PR #683 follow-up
|
||||
(this amendment) eradicates the negation construct entirely:
|
||||
|
||||
- Replace `"(context-window optimization, NOT a network restriction)"`
|
||||
with affirmative `"to <ctx_tool> for context-window efficiency"` +
|
||||
separate affirmative sentence `"<ctx_tool> has full network
|
||||
access."`.
|
||||
- Replace `"Do NOT retry with curl/wget"` with positive
|
||||
`"Retry the same call on a transient DNS error (...)"` — the
|
||||
affirmative retry hint IS the next-action signal; the prohibition was
|
||||
redundant once the routing is correct.
|
||||
|
||||
The four CASE A sites in `hooks/core/routing.mjs` (L707 curl/wget,
|
||||
L738 inline HTTP, L751 build tool, L804 WebFetch) all conform to the
|
||||
amended rubric after this PR. A contract test in
|
||||
`tests/core/server.test.ts` (`ADR-0003 CASE A: routing.mjs redirect
|
||||
deny reasons`) locks the rule: every CASE A string MUST open with
|
||||
"redirected", MUST NOT contain bare uppercase `BLOCKED`, MUST name at
|
||||
least one `ctx_*` alternative, MUST NOT contain `"NOT a network"`,
|
||||
MUST NOT contain `"Do NOT retry"`.
|
||||
|
||||
## Consequences
|
||||
|
||||
- PR #654's wording fix (`"blocked"` → `"redirected"`) becomes formal
|
||||
|
||||
@@ -704,7 +704,7 @@ export function routePreToolUse(toolName, toolInput, projectDir, platform, sessi
|
||||
return mcpRedirect({
|
||||
action: "modify",
|
||||
updatedInput: {
|
||||
command: `echo "context-mode: curl/wget redirected (context-window optimization, NOT a network restriction). ${t("ctx_execute")} and ${t("ctx_fetch_and_index")} have full network access — retry if it fails with a transient DNS error. Think in Code — call ${t("ctx_execute")}(language, code) now to fetch, process, and print only the answer. Or ${t("ctx_fetch_and_index")}(url, source) to fetch and index. Write pure JS with try/catch, no npm deps. Do NOT retry with curl/wget."`,
|
||||
command: `echo "context-mode: curl/wget redirected to ${t("ctx_execute")} for context-window efficiency. ${t("ctx_execute")} and ${t("ctx_fetch_and_index")} have full network access. Call ${t("ctx_execute")}(language, code) now to fetch, process, and print only the answer. Or call ${t("ctx_fetch_and_index")}(url, source) to fetch and index. Write pure JS with try/catch, no npm deps. Retry the same call on a transient DNS error (EAI_AGAIN, ETIMEDOUT, ENETUNREACH)."`,
|
||||
},
|
||||
// D2 PRD Phase 3.1: marker payload for PostToolUse byte accounting.
|
||||
redirectMeta: {
|
||||
@@ -735,7 +735,7 @@ export function routePreToolUse(toolName, toolInput, projectDir, platform, sessi
|
||||
return mcpRedirect({
|
||||
action: "modify",
|
||||
updatedInput: {
|
||||
command: `echo "context-mode: Inline HTTP redirected (context-window optimization, NOT a network restriction). ${t("ctx_execute")} has full network access — retry if it fails with a transient DNS error. Think in Code — call ${t("ctx_execute")}(language, code) now to fetch, process, and console.log() only the result. Write robust pure JS with try/catch, no npm deps. Do NOT retry with Bash."`,
|
||||
command: `echo "context-mode: Inline HTTP redirected to ${t("ctx_execute")} for context-window efficiency. ${t("ctx_execute")} has full network access. Call ${t("ctx_execute")}(language, code) now to fetch, process, and console.log() only the result. Write robust pure JS with try/catch, no npm deps. Retry the same call on a transient DNS error (EAI_AGAIN, ETIMEDOUT, ENETUNREACH)."`,
|
||||
},
|
||||
});
|
||||
}
|
||||
@@ -748,7 +748,7 @@ export function routePreToolUse(toolName, toolInput, projectDir, platform, sessi
|
||||
return mcpRedirect({
|
||||
action: "modify",
|
||||
updatedInput: {
|
||||
command: `echo "context-mode: Build tool redirected. Think in Code — use ${t("ctx_execute")}(language: \\"shell\\", code: \\"${safeCmd} 2>&1 | tail -30\\") to run and print only errors/summary. Do NOT retry with Bash."`,
|
||||
command: `echo "context-mode: Build tool redirected to ${t("ctx_execute")} for context-window efficiency. Call ${t("ctx_execute")}(language: \\"shell\\", code: \\"${safeCmd} 2>&1 | tail -30\\") to run and print only errors/summary. Use the sandbox so the verbose build log stays out of context."`,
|
||||
},
|
||||
});
|
||||
}
|
||||
@@ -801,7 +801,7 @@ export function routePreToolUse(toolName, toolInput, projectDir, platform, sessi
|
||||
const url = toolInput.url ?? "";
|
||||
return mcpRedirect({
|
||||
action: "deny",
|
||||
reason: `context-mode: WebFetch redirected to ${t("ctx_fetch_and_index")} (context-window optimization, NOT a network restriction). ${t("ctx_fetch_and_index")} has full network access — retry if it fails with a transient DNS error. Call ${t("ctx_fetch_and_index")}(url: "${url}", source: "...") now, then ${t("ctx_search")}(queries: [...]) to query. Or ${t("ctx_execute")}(language, code) to fetch, process, and console.log() only what you need. Write pure JS, no npm deps. Do NOT retry with WebFetch, curl, or wget.`,
|
||||
reason: `context-mode: WebFetch redirected to ${t("ctx_fetch_and_index")} for context-window efficiency. ${t("ctx_fetch_and_index")} and ${t("ctx_execute")} have full network access. Call ${t("ctx_fetch_and_index")}(url: "${url}", source: "...") now, then ${t("ctx_search")}(queries: [...]) to query the indexed content. Or call ${t("ctx_execute")}(language, code) to fetch, process, and console.log() only what you need (pure JS, no npm deps). Retry the same call on a transient DNS error (EAI_AGAIN, ETIMEDOUT, ENETUNREACH).`,
|
||||
// D2 PRD Phase 4.1: marker payload for PostToolUse byte accounting.
|
||||
redirectMeta: {
|
||||
tool: "WebFetch",
|
||||
|
||||
@@ -5753,7 +5753,145 @@ describe("hook routing prompt-surface contract (#683 ADR-0002 + ADR-0003)", () =
|
||||
// is mentioned so the agent has a concrete next call.
|
||||
expect(cs.payload).toMatch(/ctx_(execute|fetch_and_index|search|batch_execute)/);
|
||||
});
|
||||
|
||||
// ── PR #683 follow-up (Mert flag): negation-pattern eradication ──
|
||||
//
|
||||
// The original PR #654 fix replaced the single word "blocked" with
|
||||
// "redirected", which removed the Constitutional-AI safety trigger but
|
||||
// kept a sibling rubric #2 violation in the very next clause:
|
||||
//
|
||||
// "(context-window optimization, NOT a network restriction)"
|
||||
//
|
||||
// The audit (TOOL-DESCRIPTIONS-AUDIT.md §2 Probe 3) measured this
|
||||
// parenthetical regressing Haiku capitulation from 0/6 → 2/6 — the
|
||||
// bare-NOT construct primes the very frame it tries to deny (ironic
|
||||
// process theory). Per ADR-0002 rubric #2 (affirmative beats negative),
|
||||
// CASE A strings MUST avoid bare-NOT negations entirely. Reframe with
|
||||
// affirmative "X has full network access" + imperative retry hint.
|
||||
test("MUST NOT contain the 'NOT a network' negation (PR #683 follow-up)", () => {
|
||||
// Matches "NOT a network restriction", "NOT a network/security
|
||||
// restriction", "is NOT a network ...", etc. Affirmative-only voice.
|
||||
expect(cs.payload).not.toMatch(/\bNOT\s+a\s+network\b/i);
|
||||
});
|
||||
|
||||
test("MUST NOT contain 'Do NOT retry' negation (PR #683 follow-up)", () => {
|
||||
// Same rubric: "Do NOT retry with curl/wget" anchors attention on
|
||||
// the disallowed action. Express as the positive next step — the
|
||||
// ctx_execute / ctx_fetch_and_index call IS the next step.
|
||||
expect(cs.payload).not.toMatch(/\bDo\s+NOT\s+retry\b/i);
|
||||
});
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ──────────────────────────────────────────────────────────────────────────
|
||||
// PR #683 follow-up (Mert flag, 2026-05-24): tool description source-form
|
||||
// uniformity — embedded `\n\n` escapes in template literals
|
||||
//
|
||||
// ctx_execute was the lone ctx_* tool whose description used a template
|
||||
// literal with embedded `\n\n` escape sequences inline (compact but
|
||||
// escape-heavy and harder to scan). Every other multi-section ctx_*
|
||||
// description used `"..." + "...\n\n"` concat-form. Both render identically
|
||||
// to the host LLM — the bytes are the same `\n\n` separators — but the
|
||||
// source form was inconsistent.
|
||||
//
|
||||
// Canonical decision (ADR-0002 follow-up): template literal with REAL
|
||||
// newlines. Source mirrors the rendered prompt. Zero escape sequences.
|
||||
// Markdown-friendly when read in an editor. The contract test below locks
|
||||
// the decision so a future contributor can't slip back to either of the
|
||||
// rejected forms (embedded `\n\n` escapes OR multi-line string concat).
|
||||
// ──────────────────────────────────────────────────────────────────────────
|
||||
describe("tool description source form contract (#683 PR follow-up)", () => {
|
||||
const serverTsPath = resolve(__dirname, "../../src/server.ts");
|
||||
const serverTs = readFileSync(serverTsPath, "utf-8");
|
||||
|
||||
// Locate every `description: ...,` block under a server.registerTool() call
|
||||
// and capture its raw source text. Reuse the same anchor pattern as the
|
||||
// ADR-0002 contract test for consistency.
|
||||
function extractDescriptionBlocks(): Array<{ name: string; raw: string; lineNo: number }> {
|
||||
const out: Array<{ name: string; raw: string; lineNo: number }> = [];
|
||||
const lines = serverTs.split("\n");
|
||||
const RE_REGISTER = /server\.registerTool\(\s*$/;
|
||||
const RE_NAME = /^\s*"(ctx_[a-z_]+)"\s*,\s*$/;
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
if (!RE_REGISTER.test(lines[i])) continue;
|
||||
const nameMatch = lines[i + 1]?.match(RE_NAME);
|
||||
if (!nameMatch) continue;
|
||||
const name = nameMatch[1];
|
||||
let descStart = -1;
|
||||
let descEnd = -1;
|
||||
for (let j = i + 2; j < Math.min(i + 80, lines.length); j++) {
|
||||
if (descStart < 0 && /^\s*description:/.test(lines[j])) {
|
||||
descStart = j;
|
||||
} else if (descStart >= 0 && /^\s*(inputSchema|outputSchema|annotations):/.test(lines[j])) {
|
||||
descEnd = j;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (descStart < 0 || descEnd < 0) continue;
|
||||
out.push({ name, raw: lines.slice(descStart, descEnd).join("\n"), lineNo: descStart + 1 });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
const blocks = extractDescriptionBlocks();
|
||||
|
||||
// Tools whose descriptions are intentionally one-line concats with no
|
||||
// section structure (diagnostic / GUI affordances per ADR-0002
|
||||
// §Exemptions). They carry no `\n` separators in the source at all, so
|
||||
// the embedded-escape rule is trivially satisfied and the concat-form
|
||||
// exemption applies. Future contributors can promote these to template
|
||||
// literals at will; they just aren't forced to.
|
||||
const SHORT_DESCRIPTION_EXEMPT = new Set([
|
||||
"ctx_stats",
|
||||
"ctx_doctor",
|
||||
"ctx_upgrade",
|
||||
"ctx_insight",
|
||||
]);
|
||||
|
||||
test("at least 11 ctx_* tools surfaced for source-form contract", () => {
|
||||
expect(blocks.length).toBeGreaterThanOrEqual(11);
|
||||
});
|
||||
|
||||
for (const block of blocks) {
|
||||
if (SHORT_DESCRIPTION_EXEMPT.has(block.name)) continue;
|
||||
|
||||
describe(block.name, () => {
|
||||
test("source-form MUST NOT contain embedded '\\n\\n' escape (use real newlines in template literal)", () => {
|
||||
// The forbidden pattern is the literal four-character source-text
|
||||
// sequence: backslash, n, backslash, n. In source these appear inside
|
||||
// template literals (`...\n\n...`) or quoted strings ("...\n\n").
|
||||
// The canonical form is a template literal with REAL newlines so the
|
||||
// source mirrors the rendered prompt byte-for-byte without escapes.
|
||||
const hasEscapedDoubleNewline = /\\n\\n/.test(block.raw);
|
||||
expect(
|
||||
hasEscapedDoubleNewline,
|
||||
`${block.name} description (src/server.ts:${block.lineNo}) contains embedded '\\n\\n' escapes. ` +
|
||||
`Use a template literal with REAL newlines instead — source should mirror the rendered prompt. ` +
|
||||
`Diff: replace \`...\\n\\n...\` source spans with multi-line template literals.`,
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test("source-form MUST NOT use multi-line string concat with '+' (use template literal)", () => {
|
||||
// Concat-form was the legacy alternative — `"...\n\n" + "WHEN:\n"`.
|
||||
// Once we ban `\n\n`, the only sensible multi-section shape is a
|
||||
// template literal. This second assertion makes that explicit so a
|
||||
// contributor doesn't switch one negative form for another (e.g.
|
||||
// splitting on a newline inside a `+ "\n"` chain).
|
||||
//
|
||||
// Heuristic: a `"\n" +` or `"+ \n"` chain inside the description
|
||||
// block. Single-line concats (e.g. `"a " + "b"`) are too loose to
|
||||
// ban without false positives, so we anchor on the embedded newline
|
||||
// form which is what multi-section descriptions actually used.
|
||||
const hasConcatNewline = /"[^"]*"\s*\+\s*\n\s*"/.test(block.raw) ||
|
||||
/\\n"\s*\+/.test(block.raw);
|
||||
expect(
|
||||
hasConcatNewline,
|
||||
`${block.name} description (src/server.ts:${block.lineNo}) uses multi-line string concat with '+'. ` +
|
||||
`Use a template literal with REAL newlines instead — single canonical source form for all multi-section descriptions.`,
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user