From 3ac34cfc3c395dbcf582667519581c6747680d36 Mon Sep 17 00:00:00 2001
From: Eneko Ruiz <196690101+eneekoruiz@users.noreply.github.com>
Date: Sun, 11 Oct 2026 11:37:01 +0200
Subject: [PATCH 1/7] fix: validate skill execution failure reasons before
 storage

---
 scripts/lib/skill-evolution/tracker.js |  7 ++++++-
 tests/lib/skill-evolution.test.js      | 16 ++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/skill-evolution/tracker.js b/scripts/lib/skill-evolution/tracker.js
index 0ea0ba1cc..48d633f92 100644
--- a/scripts/lib/skill-evolution/tracker.js
+++ b/scripts/lib/skill-evolution/tracker.js
@@ -53,6 +53,7 @@ function normalizeExecutionRecord(input, options = {}) {
   const outcome = input.outcome;
   const recordedAt = input.recorded_at || options.now || new Date().toISOString();
   const userFeedback = input.user_feedback || input.userFeedback || null;
+  const failureReason = input.failure_reason ?? input.failureReason ?? null;
 
   if (typeof skillId !== 'string' || skillId.trim().length === 0) {
     throw new Error('skill_id is required');
@@ -74,6 +75,10 @@ function normalizeExecutionRecord(input, options = {}) {
     throw new Error('user_feedback must be accepted, corrected, rejected, or null');
   }
 
+  if (failureReason !== null && typeof failureReason !== 'string') {
+    throw new Error('failure_reason must be a string or null');
+  }
+
   if (Number.isNaN(Date.parse(recordedAt))) {
     throw new Error('recorded_at must be an ISO timestamp');
   }
@@ -83,7 +88,7 @@ function normalizeExecutionRecord(input, options = {}) {
     skill_version: skillVersion,
     task_description: taskDescription,
     outcome,
-    failure_reason: input.failure_reason || input.failureReason || null,
+    failure_reason: failureReason || null,
     tokens_used: toNullableNumber(input.tokens_used ?? input.tokensUsed, 'tokens_used'),
     duration_ms: toNullableNumber(input.duration_ms ?? input.durationMs, 'duration_ms'),
     user_feedback: userFeedback,
diff --git a/tests/lib/skill-evolution.test.js b/tests/lib/skill-evolution.test.js
index f2dd4e6ec..f8c826b13 100644
--- a/tests/lib/skill-evolution.test.js
+++ b/tests/lib/skill-evolution.test.js
@@ -381,11 +381,27 @@ function runTests() {
         [{ ...valid, recorded_at: 'not-a-date' }, /recorded_at must be/],
         [{ ...valid, tokens_used: 'lots' }, /tokens_used must be a number/],
         [{ ...valid, duration_ms: Number.POSITIVE_INFINITY }, /duration_ms must be a number/],
+        ...[42, 0, false, {}, []].flatMap(value => [
+          [{ ...valid, failure_reason: value }, /failure_reason must be a string or null/],
+          [{ ...valid, failureReason: value }, /failure_reason must be a string or null/],
+        ]),
       ];
 
       for (const [payload, expectedError] of cases) {
         assert.throws(() => tracker.normalizeExecutionRecord(payload), expectedError);
       }
+      for (const value of [null, '', 'Useful detail']) {
+        assert.strictEqual(
+          tracker.normalizeExecutionRecord({ ...valid, failureReason: value }).failure_reason,
+          value || null
+        );
+      }
+      const invalidRunsFile = path.join(homeDir, '.claude', 'state', 'invalid-reason-runs.jsonl');
+      assert.throws(
+        () => tracker.recordSkillExecution({ ...valid, failure_reason: 42 }, { runsFilePath: invalidRunsFile }),
+        /failure_reason must be a string or null/
+      );
+      assert.ok(!fs.existsSync(invalidRunsFile), 'invalid reasons must be rejected before writing a run');
     })) passed++; else failed++;
 
     if (test('uses state-store adapters for recording and reading when available', () => {

From 95585e4b291f410e0aab60d6d9de4f3f29c51746 Mon Sep 17 00:00:00 2001
From: Eneko Ruiz <196690101+eneekoruiz@users.noreply.github.com>
Date: Sun, 11 Oct 2026 11:37:33 +0200
Subject: [PATCH 2/7] fix: read piped tasks when evaluating output files

RED: the documented --output pipe mode discarded its task. GREEN: 8 tests pass across existing input modes and the real CLI conciseness score. Changed input functions have 100% line coverage.
---
 .../agent-self-evaluation/scripts/evaluate.py |  5 +-
 tests/test_agent_self_evaluation.py           | 87 +++++++++++++++++++
 2 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_agent_self_evaluation.py

diff --git a/skills/agent-self-evaluation/scripts/evaluate.py b/skills/agent-self-evaluation/scripts/evaluate.py
index 2d129c407..619a8b284 100755
--- a/skills/agent-self-evaluation/scripts/evaluate.py
+++ b/skills/agent-self-evaluation/scripts/evaluate.py
@@ -382,7 +382,10 @@ def _read_input(args: argparse.Namespace) -> tuple[Optional[str], str]:
         print("Paste agent output (Ctrl+D to finish):")
         return task, sys.stdin.read()
     if args.output:
-        return _read_file_or_text(args.task), _read_file_or_text(args.output, required=True) or ""
+        task = _read_file_or_text(args.task)
+        if task is None and not sys.stdin.isatty():
+            task = sys.stdin.read()
+        return task, _read_file_or_text(args.output, required=True) or ""
     return _read_file_or_text(args.task), sys.stdin.read()
 
 
diff --git a/tests/test_agent_self_evaluation.py b/tests/test_agent_self_evaluation.py
new file mode 100644
index 000000000..b3f7823e8
--- /dev/null
+++ b/tests/test_agent_self_evaluation.py
@@ -0,0 +1,87 @@
+"""Input-mode regressions for the standalone agent output evaluator."""
+
+import argparse
+import importlib.util
+import io
+import re
+import subprocess
+import sys
+from pathlib import Path
+from types import SimpleNamespace
+
+import pytest
+
+SCRIPT = Path(__file__).resolve().parents[1] / "skills/agent-self-evaluation/scripts/evaluate.py"
+SPEC = importlib.util.spec_from_file_location("agent_self_evaluation", SCRIPT)
+evaluator = importlib.util.module_from_spec(SPEC)
+sys.modules[SPEC.name] = evaluator
+SPEC.loader.exec_module(evaluator)
+
+
+def args(**overrides):
+    return argparse.Namespace(**{"interactive": False, "task": None, "output": None, **overrides})
+
+
+def test_output_file_uses_piped_task(tmp_path, monkeypatch):
+    output = tmp_path / "response.txt"
+    output.write_text("All tests passed.", encoding="utf-8")
+    monkeypatch.setattr(sys, "stdin", io.StringIO("Add retry logic"))
+
+    assert evaluator._read_input(args(output=str(output))) == ("Add retry logic", "All tests passed.")
+
+
+def test_explicit_task_takes_precedence_over_pipe(tmp_path, monkeypatch):
+    output = tmp_path / "response.txt"
+    output.write_text("Answer", encoding="utf-8")
+    stream = io.StringIO("Ignored pipe")
+    monkeypatch.setattr(sys, "stdin", stream)
+
+    assert evaluator._read_input(args(task="Explicit task", output=str(output))) == ("Explicit task", "Answer")
+    assert stream.tell() == 0
+
+
+def test_output_file_without_task_does_not_read_terminal(tmp_path, monkeypatch):
+    output = tmp_path / "response.txt"
+    output.write_text("Answer", encoding="utf-8")
+    monkeypatch.setattr(sys, "stdin", SimpleNamespace(isatty=lambda: True))
+
+    assert evaluator._read_input(args(output=str(output))) == (None, "Answer")
+
+
+def test_no_output_file_keeps_stdin_as_output(monkeypatch):
+    monkeypatch.setattr(sys, "stdin", io.StringIO("Piped answer"))
+    assert evaluator._read_input(args(task="Inline task")) == ("Inline task", "Piped answer")
+
+
+def test_interactive_mode_keeps_prompt_and_stdin(monkeypatch):
+    monkeypatch.setattr("builtins.input", lambda _prompt: "  Interactive task  ")
+    monkeypatch.setattr(sys, "stdin", io.StringIO("Interactive answer"))
+    assert evaluator._read_input(args(interactive=True)) == ("Interactive task", "Interactive answer")
+
+
+def test_task_can_still_be_read_from_file(tmp_path, monkeypatch):
+    task = tmp_path / "task.txt"
+    task.write_text("File task", encoding="utf-8")
+    monkeypatch.setattr(sys, "stdin", io.StringIO("Piped answer"))
+    assert evaluator._read_input(args(task=str(task))) == ("File task", "Piped answer")
+
+
+def test_missing_output_file_still_fails(tmp_path, monkeypatch, capsys):
+    monkeypatch.setattr(sys, "stdin", io.StringIO("Task"))
+    with pytest.raises(SystemExit, match="1"):
+        evaluator._read_input(args(output=str(tmp_path / "missing.txt")))
+    assert "not found" in capsys.readouterr().err
+
+
+def test_cli_uses_piped_task_for_conciseness_score(tmp_path):
+    output = tmp_path / "response.txt"
+    output.write_text("Verified with tests. " * 25, encoding="utf-8")
+    result = subprocess.run(
+        [sys.executable, "-X", "utf8", str(SCRIPT), "--output", str(output)],
+        input="Fix bug",
+        capture_output=True,
+        encoding="utf-8",
+        check=True,
+        timeout=10,
+    )
+    assert re.search(r"Conciseness[^\n]*3/5", result.stdout)

From cffcf05e7a8ce89e2defce581f6e1536350fc501 Mon Sep 17 00:00:00 2001
From: chenhz01 <chenhz01@users.noreply.github.com>
Date: Sun, 11 Oct 2026 18:52:44 +0800
Subject: [PATCH 3/7] =?UTF-8?q?hooks:=20sed=20labels=20end=20at=20semicolo?=
 =?UTF-8?q?n=20=E2=80=94=20commands=20after=20b/t/T/:=20are=20still=20scan?=
 =?UTF-8?q?ned?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CodeRabbit security finding on the merged sed work (CWE-693, verified live
on main via #3495): GNU sed labels for b, t, T and : end at ';' or newline —
verified against GNU sed 4.9 ('bar;e cmd' parses label 'a' and still runs
cmd; '}' is not a label terminator). Reading those operands to end of line
let 'sed :a;e git commit --no-verify' pass the hook while sed executed the
e command.

Labels are now read up to ';' or newline and command parsing resumes after
them. Text, filename and quit operands (a, c, i, r, w, q, Q) keep the
end-of-line reading: nothing after them on the line executes.

5 regression tests pin the three bypass paths and the benign boundary
(a bare label and a bare branch stay data). Suite: 817/817.
---
 scripts/hooks/block-no-verify.js    | 14 +++++++++++++-
 tests/hooks/block-no-verify.test.js | 11 ++++++++++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/scripts/hooks/block-no-verify.js b/scripts/hooks/block-no-verify.js
index 1d19e5c35..6774ae75f 100644
--- a/scripts/hooks/block-no-verify.js
+++ b/scripts/hooks/block-no-verify.js
@@ -586,6 +586,17 @@ function readSedLineOperand(script, i, budget) {
   return { end, parts: [], stdinExec: false };
 }
 
+// Branch and label commands (b, t, T, :) take a label that ends at `;` or
+// newline — verified against GNU sed 4.9: ':a;e cmd' and 'ba;e cmd' both
+// parse the label up to `;` and still run the following `e`. Commands after
+// the label therefore continue to be scanned, so return the label end and
+// let the caller resume command parsing there.
+function readSedLabel(script, i, budget) {
+  let end = i + 1;
+  while (end < script.length && script[end] !== ';' && script[end] !== '\n') { budget.spend(); end++; }
+  return { end, parts: [], stdinExec: false };
+}
+
 function readSedCommand(script, i, budget) {
   const letter = script[i];
   if (letter === 's') {
@@ -602,7 +613,8 @@ function readSedCommand(script, i, budget) {
     return sub ? { end: sub.end, parts: [], stdinExec: false } : null;
   }
   if (letter === 'e') return readSedExecute(script, i, budget);
-  if ('acirwWb:tTqQ'.includes(letter)) return readSedLineOperand(script, i, budget);
+  if ('btT:'.includes(letter)) return readSedLabel(script, i, budget);
+  if ('acirwWqQ'.includes(letter)) return readSedLineOperand(script, i, budget);
   if ('dDgGhHlnNpPxFzv='.includes(letter)) return { end: i + 1, parts: [], stdinExec: false };
   return null; // unmodeled command letter: keep the opaque default
 }
diff --git a/tests/hooks/block-no-verify.test.js b/tests/hooks/block-no-verify.test.js
index 56549e721..bf456d050 100644
--- a/tests/hooks/block-no-verify.test.js
+++ b/tests/hooks/block-no-verify.test.js
@@ -2183,7 +2183,16 @@ const sedCases = [
   ['unknown escape decoding bypass is conservatively blocked', "printf x | sed 's/x/\\git commit --no-verify/e'", 2],
   ['escaped-newline continued e operand is conservatively blocked', "printf x | sed -e 'e git commit \\\n--no-verify'", 2],
   ['octal escape decoding bypass is conservatively blocked', "printf x | sed 's/x/\\147it commit --no-verify/e'", 2],
-  ['backslash in a non-executing script stays data', "sed 's/a\\/b/c/' 'git commit --no-verify'", 0]
+  ['backslash in a non-executing script stays data', "sed 's/a\\/b/c/' 'git commit --no-verify'", 0],
+  // Labels end at `;` and following commands still execute (GNU sed 4.9
+  // verified: ':a;e cmd' runs cmd). Reading label operands to end of line
+  // let an e command hide behind a label (CodeRabbit finding, landed via
+  // #3495) — the label reader must stop at `;` so the rest is scanned.
+  ['e command after a label is scanned', "sed ':a;e git commit --no-verify'", 2],
+  ['e command after a b branch is scanned', "printf x | sed 'ba;e git commit --no-verify'", 2],
+  ['e command after a t branch is scanned', "printf x | sed 'tX;e git commit --no-verify'", 2],
+  ['plain label without following commands stays data', "sed ':a' 'git commit --no-verify'", 0],
+  ['b branch alone stays data', "sed 'bx' 'git commit --no-verify'", 0]
 ];
 for (const [name, command, expected] of sedCases) {
   if (test(`sed model ${name}`, () => {

From cbca1dc2cc835ff27cd1cf7adf6759a1651cad0f Mon Sep 17 00:00:00 2001
From: akushonkamen <akushonkamen@163.com>
Date: Sun, 11 Oct 2026 19:30:35 +0800
Subject: [PATCH 4/7] fix(claude-plugin): target explicitly disabled
 destinations in scope migration
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Issue #3492: scope migration aborted with generic verification errors in
two real states. When the destination scope is installed but inventory
reports enabled:false because the user explicitly set
enabledPlugins["ecc@ecc"]: false, setup now throws a targeted
DESTINATION_EXPLICITLY_DISABLED error naming the setting and file and
offering the native claude plugin enable recovery, leaving the user's
choice untouched. The generic not-enabled error also gains the enable
command in its recovery list.

uninstallSource now accepts a destination-only state when the source
scope is gone from the fresh inventory even if the uninstall command
errored, and SOURCE_UNINSTALL_FAILED recovery no longer suggests
uninstalling a source scope the inventory no longer lists; it prepends
claude plugin list --json so the stuck state can be inspected.

Tests: node tests/lib/claude-scope-migration.test.js — five new cases
cover explicitly disabled resume and single-scope destinations across
user/project/local settings files, the generic fallback keeping the
enable path, an uninstall error with the source already absent, and
non-circular recovery. 16 -> 21 passing; red against unpatched scripts
is 15 passed / 6 failed. Full-suite failure set is byte-identical to
unpatched HEAD in a zero-dependency environment.

Also sync the pinned SOURCE_UNINSTALL_FAILED recovery in
tests/scripts/setup.test.js ('migration JSON failures retain phase,
scopes, and exact recovery') with the new claude plugin list --json
diagnostic: 29 passed / 1 failed on the patched tree before the sync,
30 passed / 0 failed after, identical to clean 2d515e4f.
---
 scripts/lib/claude-scope-migration.js    |  86 ++++++++++++-
 tests/lib/claude-scope-migration.test.js | 151 +++++++++++++++++++++++
 tests/scripts/setup.test.js              |   1 +
 3 files changed, 233 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/claude-scope-migration.js b/scripts/lib/claude-scope-migration.js
index acb789f3b..389c3de76 100644
--- a/scripts/lib/claude-scope-migration.js
+++ b/scripts/lib/claude-scope-migration.js
@@ -1,5 +1,6 @@
 'use strict';
 
+const fs = require('fs');
 const path = require('path');
 
 const {
@@ -45,6 +46,63 @@ function recoveryCommands(sourceScope, destinationScope) {
   return commands;
 }
 
+function enableRecoveryCommands(destinationScope) {
+  return [
+    `claude plugin enable ${CURRENT_PLUGIN_ID} --scope ${destinationScope}`,
+    ...recoveryCommands(null, destinationScope),
+  ];
+}
+
+function destinationSettingsPaths(paths, destinationScope) {
+  const projectSettingsDir = path.join(paths.projectRoot, '.claude');
+  if (destinationScope === 'user') {
+    return [path.join(paths.configDir, 'settings.json')];
+  }
+  if (destinationScope === 'project') {
+    return [path.join(projectSettingsDir, 'settings.json')];
+  }
+  return [
+    path.join(projectSettingsDir, 'settings.local.json'),
+    path.join(projectSettingsDir, 'settings.json'),
+  ];
+}
+
+function readExplicitlyDisabledSettings(settingsPath) {
+  try {
+    const settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8'));
+    return settings?.enabledPlugins?.[CURRENT_PLUGIN_ID] === false;
+  } catch {
+    // Missing or unreadable settings are not an explicit user choice.
+    return false;
+  }
+}
+
+function destinationExplicitlyDisabledError(paths, plugins, destinationScope) {
+  const installed = currentEccPlugins(plugins);
+  const destination = installed.find(plugin => plugin.scope === destinationScope);
+  if (!destination || destination.enabled === true) {
+    return null;
+  }
+  const disabledSettingsPaths = destinationSettingsPaths(paths, destinationScope)
+    .filter(readExplicitlyDisabledSettings);
+  if (disabledSettingsPaths.length === 0) {
+    return null;
+  }
+  return migrationError(
+    'DESTINATION_EXPLICITLY_DISABLED',
+    `${CURRENT_PLUGIN_ID} is installed at ${destinationScope} scope but explicitly `
+    + `disabled by enabledPlugins["${CURRENT_PLUGIN_ID}"]: false in `
+    + `${disabledSettingsPaths.join(', ')}. ECC keeps that choice and will not `
+    + `rewrite the setting; enable the plugin to continue the migration.`,
+    {
+      phase: 'destination-verification',
+      observedScopes: installed.map(plugin => plugin.scope),
+      settingsPaths: disabledSettingsPaths,
+      recovery: enableRecoveryCommands(destinationScope),
+    }
+  );
+}
+
 function readPluginInventory(run, projectRoot, phase) {
   return parsePluginList(
     run(
@@ -95,7 +153,7 @@ function assertMigrationInventory(plugins, destinationScope) {
         {
           phase: 'destination-verification',
           observedScopes,
-          recovery: recoveryCommands(null, destinationScope),
+          recovery: enableRecoveryCommands(destinationScope),
         }
       );
     }
@@ -208,23 +266,33 @@ function uninstallSource(run, paths, migration, destinationScope) {
       assertNoConflictingEccPlugins(plugins);
       const installed = currentEccPlugins(plugins);
       observedScopes = installed.map(plugin => plugin.scope);
+      const destination = installed.find(
+        plugin => plugin.scope === destinationScope
+      );
       if (
-        installed.length === 1
-        && installed[0].scope === destinationScope
-        && installed[0].enabled === true
+        destination
+        && destination.enabled !== false
+        && !installed.some(plugin => plugin.scope === migration.sourceScope)
       ) {
         return ['Claude reported an uninstall error, but destination-only state was verified.'];
       }
     } catch {
       // Preserve the safest known two-scope state in the structured recovery.
     }
+    const sourcePresent = observedScopes.includes(migration.sourceScope);
     throw migrationError(
       'SOURCE_UNINSTALL_FAILED',
       `The destination is installed, but Claude could not remove the ${migration.sourceScope} source scope.`,
       {
         phase: 'source-uninstall',
         observedScopes,
-        recovery: recoveryCommands(migration.sourceScope, destinationScope),
+        recovery: [
+          'claude plugin list --json',
+          ...recoveryCommands(
+            sourcePresent ? migration.sourceScope : null,
+            destinationScope
+          ),
+        ],
       }
     );
   }
@@ -265,6 +333,14 @@ function migrateClaudePluginScope(options = {}, dependencies = {}) {
     ? createDryRunClaudeRunner(providerRun, paths, options)
     : providerRun;
   const plugins = readPluginInventory(run, paths.projectRoot, 'inventory');
+  const disabledDestinationError = destinationExplicitlyDisabledError(
+    paths,
+    plugins,
+    options.scope
+  );
+  if (disabledDestinationError) {
+    throw disabledDestinationError;
+  }
   const migration = assertMigrationInventory(plugins, options.scope);
   const hooks = options.hooks === undefined
     ? deriveHookMode(settings)
diff --git a/tests/lib/claude-scope-migration.test.js b/tests/lib/claude-scope-migration.test.js
index c3ab6606f..bace9db1c 100644
--- a/tests/lib/claude-scope-migration.test.js
+++ b/tests/lib/claude-scope-migration.test.js
@@ -514,6 +514,7 @@ test('source uninstall failure reports both scopes and exact forward recovery',
     assert.strictEqual(error.phase, 'source-uninstall');
     assert.deepStrictEqual([...error.observedScopes].sort(), ['project', 'user']);
     assert.deepStrictEqual(error.recovery, [
+      'claude plugin list --json',
       'claude plugin uninstall ecc@ecc --scope user --keep-data',
       'ecc setup --mode claude-plugin --scope project --move-scope --yes',
     ]);
@@ -670,5 +671,155 @@ test('migration preserves an explicit includeCoAuthoredBy opt-in', () => {
   });
 });
 
+function writeExplicitDisable(filePath, pluginId = 'ecc@ecc') {
+  fs.mkdirSync(path.dirname(filePath), { recursive: true });
+  fs.writeFileSync(filePath, `${JSON.stringify({
+    enabledPlugins: { [pluginId]: false },
+  }, null, 2)}\n`);
+}
+
+test('an explicitly disabled resume destination names the setting and the enable recovery', () => {
+  withFixture({
+    plugins: [plugin('user'), plugin('local', { enabled: false })],
+    marketplaces: [marketplace('user')],
+  }, fixture => {
+    const localSettings = path.join(
+      fixture.projectRoot,
+      '.claude',
+      'settings.local.json'
+    );
+    writeExplicitDisable(localSettings);
+    const error = captureError(() => (
+      migrateClaudePluginScope(migrationOptions(fixture, 'local'))
+    ));
+    assert.strictEqual(error.code, 'DESTINATION_EXPLICITLY_DISABLED');
+    assert.strictEqual(error.phase, 'destination-verification');
+    assert.match(error.message, /enabledPlugins/);
+    assert.ok(error.message.includes(localSettings));
+    assert.deepStrictEqual(error.observedScopes, ['user', 'local']);
+    assert.deepStrictEqual(error.recovery, [
+      'claude plugin enable ecc@ecc --scope local',
+      'ecc setup --mode claude-plugin --scope local --move-scope --yes',
+    ]);
+    assert.deepStrictEqual(readCalls(fixture), [
+      ['plugin', 'list', '--json'],
+    ]);
+    assert.ok(!readCalls(fixture).some(argv => argv[1] === 'uninstall'));
+    assert.deepStrictEqual(
+      readState(fixture).plugins.map(entry => entry.scope).sort(),
+      ['local', 'user']
+    );
+    assert.strictEqual(JSON.parse(
+      fs.readFileSync(localSettings, 'utf8')
+    ).enabledPlugins['ecc@ecc'], false);
+  });
+});
+
+test('a single-scope explicitly disabled destination is not a generic verification failure', () => {
+  for (const { scope, settingsPath } of [
+    { scope: 'user', settingsPath: 'config' },
+    { scope: 'project', settingsPath: 'project-settings' },
+    { scope: 'local', settingsPath: 'project-settings' },
+    { scope: 'local', settingsPath: 'project-local-settings' },
+  ]) {
+    withFixture({ plugins: [plugin(scope, { enabled: false })] }, fixture => {
+      const explicitPaths = {
+        config: fixture.settingsPath,
+        'project-settings': path.join(
+          fixture.projectRoot, '.claude', 'settings.json'
+        ),
+        'project-local-settings': path.join(
+          fixture.projectRoot, '.claude', 'settings.local.json'
+        ),
+      };
+      const disabledPath = explicitPaths[settingsPath];
+      writeExplicitDisable(disabledPath);
+      const error = captureError(() => (
+        migrateClaudePluginScope(migrationOptions(fixture, scope))
+      ));
+      assert.strictEqual(error.code, 'DESTINATION_EXPLICITLY_DISABLED');
+      assert.ok(error.message.includes(disabledPath));
+      assert.deepStrictEqual(error.recovery, [
+        `claude plugin enable ecc@ecc --scope ${scope}`,
+        `ecc setup --mode claude-plugin --scope ${scope} --move-scope --yes`,
+      ]);
+    });
+  }
+});
+
+test('a disabled destination without an explicit setting stays generic and gains the enable path', () => {
+  withFixture({ plugins: [plugin('local', { enabled: false })] }, fixture => {
+    const error = captureError(() => (
+      migrateClaudePluginScope(migrationOptions(fixture, 'local'))
+    ));
+    assert.strictEqual(error.code, 'DESTINATION_VERIFICATION_FAILED');
+    assert.match(error.message, /exists at local scope but is not enabled/);
+    assert.deepStrictEqual(error.recovery, [
+      'claude plugin enable ecc@ecc --scope local',
+      'ecc setup --mode claude-plugin --scope local --move-scope --yes',
+    ]);
+  });
+});
+
+test('a source uninstall that errors but removes the source accepts the verified destination', () => {
+  withFixture({
+    plugins: [plugin('user'), plugin('project')],
+    marketplaces: [marketplace('user')],
+    failures: [{
+      argv: uninstallArgv('user'),
+      status: 9,
+      stderr: 'uninstall failed',
+      times: 1,
+    }],
+    pluginListResponses: [
+      [plugin('user'), plugin('project')],
+      [plugin('user'), plugin('project')],
+      [plugin('user'), plugin('project')],
+      [plugin('project'), plugin('local')],
+      [plugin('project')],
+    ],
+  }, fixture => {
+    const result = migrateClaudePluginScope(
+      migrationOptions(fixture, 'project')
+    );
+    assert.strictEqual(result.action, 'resumed');
+    assert.strictEqual(result.sourceScope, 'user');
+    assert.deepStrictEqual(result.warnings, [
+      'Claude reported an uninstall error, but destination-only state was verified.',
+    ]);
+    assert.ok(!readCalls(fixture).slice(-2).some(argv => argv[1] === 'uninstall'));
+  });
+});
+
+test('source uninstall failure recovery stops suggesting an already-absent source', () => {
+  withFixture({
+    plugins: [plugin('user'), plugin('project')],
+    marketplaces: [marketplace('user')],
+    failures: [{
+      argv: uninstallArgv('user'),
+      status: 9,
+      stderr: 'uninstall failed',
+      times: 1,
+    }],
+    pluginListResponses: [
+      [plugin('user'), plugin('project')],
+      [plugin('user'), plugin('project')],
+      [plugin('user'), plugin('project')],
+      [],
+    ],
+  }, fixture => {
+    const error = captureError(() => (
+      migrateClaudePluginScope(migrationOptions(fixture, 'project'))
+    ));
+    assert.strictEqual(error.code, 'SOURCE_UNINSTALL_FAILED');
+    assert.strictEqual(error.phase, 'source-uninstall');
+    assert.deepStrictEqual(error.observedScopes, []);
+    assert.deepStrictEqual(error.recovery, [
+      'claude plugin list --json',
+      'ecc setup --mode claude-plugin --scope project --move-scope --yes',
+    ]);
+  });
+});
+
 console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
 process.exit(failed > 0 ? 1 : 0);
diff --git a/tests/scripts/setup.test.js b/tests/scripts/setup.test.js
index 715d383c2..806e31e24 100644
--- a/tests/scripts/setup.test.js
+++ b/tests/scripts/setup.test.js
@@ -646,6 +646,7 @@ test('migration JSON failures retain phase, scopes, and exact recovery', () => {
     assert.strictEqual(payload.error.phase, 'source-uninstall');
     assert.deepStrictEqual([...payload.error.observedScopes].sort(), ['project', 'user']);
     assert.deepStrictEqual(payload.error.recovery, [
+      'claude plugin list --json',
       'claude plugin uninstall ecc@ecc --scope user --keep-data',
       'ecc setup --mode claude-plugin --scope project --move-scope --yes',
     ]);

From c76d0e173a96206a6ec4aac4104d4e6f4d6529c9 Mon Sep 17 00:00:00 2001
From: Eneko Ruiz <196690101+eneekoruiz@users.noreply.github.com>
Date: Sun, 11 Oct 2026 16:23:26 +0200
Subject: [PATCH 5/7] fix: reject the shared base as a scenario sandbox

---
 skills/skill-comply/scripts/runner.py    |  6 ++++-
 skills/skill-comply/tests/test_runner.py | 33 +++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/skills/skill-comply/scripts/runner.py b/skills/skill-comply/scripts/runner.py
index ffad8447f..d638ad2e7 100644
--- a/skills/skill-comply/scripts/runner.py
+++ b/skills/skill-comply/scripts/runner.py
@@ -94,8 +94,12 @@ def _safe_sandbox_dir(scenario_id: str) -> Path:
     """Sanitize scenario ID and ensure path stays within sandbox base."""
     safe_id = re.sub(r"[^a-zA-Z0-9\-_]", "_", scenario_id)
     path = SANDBOX_BASE / safe_id
+    resolved_path = path.resolve()
+    resolved_base = SANDBOX_BASE.resolve()
+    if not safe_id or resolved_path == resolved_base:
+        raise ValueError("Scenario sandbox must be a subdirectory of the sandbox base")
     # Validate path stays within sandbox base (raises ValueError on traversal)
-    path.resolve().relative_to(SANDBOX_BASE.resolve())
+    resolved_path.relative_to(resolved_base)
     return path
 
 
diff --git a/skills/skill-comply/tests/test_runner.py b/skills/skill-comply/tests/test_runner.py
index f8141d184..e66e66ff1 100644
--- a/skills/skill-comply/tests/test_runner.py
+++ b/skills/skill-comply/tests/test_runner.py
@@ -9,7 +9,7 @@ from pathlib import Path
 from unittest.mock import patch
 
 import pytest
-from scripts.runner import _parse_stream_json, _setup_sandbox, run_scenario
+from scripts.runner import SANDBOX_BASE, _parse_stream_json, _safe_sandbox_dir, _setup_sandbox, run_scenario
 
 
 @dataclass(frozen=True)
@@ -21,6 +21,37 @@ class _FakeScenario:
     setup_commands: tuple[str, ...] = ()
 
 
+class TestSafeSandboxDirectory:
+    def test_empty_id_cannot_delete_the_shared_base(self):
+        with (
+            patch("scripts.runner.Path.exists", return_value=True),
+            patch("scripts.runner.shutil.rmtree") as remove,
+            patch("scripts.runner.Path.mkdir") as mkdir,
+            patch("scripts.runner.subprocess.run") as execute,
+        ):
+            execute.return_value = subprocess.CompletedProcess([], 0, stdout="", stderr="")
+            with pytest.raises(ValueError, match="sandbox"):
+                run_scenario(_FakeScenario(id=""))
+            remove.assert_not_called()
+            mkdir.assert_not_called()
+            execute.assert_not_called()
+
+    def test_resolved_base_is_not_a_scenario_directory(self):
+        with patch("scripts.runner.Path.resolve", return_value=SANDBOX_BASE):
+            with pytest.raises(ValueError, match="sandbox"):
+                _safe_sandbox_dir("alias")
+
+    def test_valid_id_keeps_existing_sanitization(self):
+        assert _safe_sandbox_dir("scenario/one") == SANDBOX_BASE / "scenario_one"
+
+    def test_resolved_escape_is_still_rejected(self, tmp_path, monkeypatch):
+        base = tmp_path / "sandbox"
+        monkeypatch.setattr("scripts.runner.SANDBOX_BASE", base)
+        with patch.object(Path, "resolve", side_effect=[tmp_path / "outside", base]):
+            with pytest.raises(ValueError):
+                _safe_sandbox_dir("escape")
+
+
 class TestSetupSandboxSkipsShellBuiltins:
     """Setup commands containing shell builtins (cd/pushd/popd) must be skipped.
 

From 626ea56291ae05752bef5caf41edefa3ab124d06 Mon Sep 17 00:00:00 2001
From: montjeffrey <montjeffrey@gmail.com>
Date: Sun, 11 Oct 2026 10:33:21 -0400
Subject: [PATCH 6/7] test(context): pin one registry load per resolution and
 routing index build

resolveTaskContext and routingEntries each compile a plan, which loads and
hashes the whole registry, then load and hash it again for their own use.
Opening one skill's policy file counts the loads: three opens per call on
main (two registry hashes plus the admission read), where one load gives two.

- New: resolution and the routing index hash the registry sources once per
  call (resolveTaskContext and routingEntries each open the policy twice).
- The policy-drift test injects its change on the open after the registry
  is hashed. With one load that is the second open, not the third; its
  assertion that the drift is rejected is unchanged.

Against main: 3 failures in tests/lib/context-selection.test.js.
---
 tests/lib/context-selection.test.js | 30 ++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/tests/lib/context-selection.test.js b/tests/lib/context-selection.test.js
index c04f943d0..473de5924 100644
--- a/tests/lib/context-selection.test.js
+++ b/tests/lib/context-selection.test.js
@@ -5,7 +5,7 @@ const fs = require('node:fs');
 const path = require('node:path');
 const test = require('node:test');
 const { withFixture, write, update } = require('./helpers/context-fixture');
-const { resolveTaskContext, resolveDeclinedFallback } = require('../../scripts/lib/context-selection');
+const { resolveTaskContext, resolveDeclinedFallback, routingEntries } = require('../../scripts/lib/context-selection');
 
 const task = (values = {}) => ({ sessionId: 'session-1', taskId: 'task-1', revision: 1,
   phase: 'implement', query: '', explicitIds: [], proposedIds: [], ...values });
@@ -71,9 +71,9 @@ for (const load of [false, true]) {
     let alteredReads = 0;
     context.mock.method(fs, 'openSync', (filename, ...args) => {
       const descriptor = originalOpen(filename, ...args);
-      // First compile the profile, then reload the canonical registry. Only
-      // the subsequent policy read observes replacement bytes.
-      if (filename === policyPath && ++policyOpens === 3) changedDescriptor = descriptor;
+      // The canonical registry is loaded once and shared with the profile
+      // compiler. Only the subsequent policy read observes replacement bytes.
+      if (filename === policyPath && ++policyOpens === 2) changedDescriptor = descriptor;
       return descriptor;
     });
     context.mock.method(fs, 'readSync', (descriptor, buffer, offset, length, position) => {
@@ -90,12 +90,32 @@ for (const load of [false, true]) {
     try {
       assert.throws(() => resolve(repoRoot, { proposedIds: ['skill:feature'] }, { load }),
         /Context source changed during selection/);
-      assert.equal(policyOpens, 3);
+      assert.equal(policyOpens, 2);
       assert.equal(alteredReads, 1);
     } finally { context.mock.restoreAll(); }
   }));
 }
 
+test('resolution and the routing index hash the registry sources once per call', context => withFixture(repoRoot => {
+  const relative = 'skills/feature/agents/openai.yaml';
+  write(repoRoot, relative, 'policy:\n  allow_implicit_invocation: true\n');
+  const policyPath = path.join(fs.realpathSync(repoRoot), relative);
+  const originalOpen = fs.openSync;
+  let opens = 0;
+  context.mock.method(fs, 'openSync', (filename, ...args) => {
+    if (filename === policyPath) opens++;
+    return originalOpen(filename, ...args);
+  });
+  try {
+    // One open hashes the policy into the registry; one reads it for admission.
+    resolve(repoRoot, { query: 'use the feature skill for this change' });
+    assert.equal(opens, 2, 'resolveTaskContext');
+    opens = 0;
+    routingEntries({ repoRoot });
+    assert.equal(opens, 2, 'routingEntries');
+  } finally { context.mock.restoreAll(); }
+}));
+
 test('authority-bearing metadata cannot become automatic invocation', () => withFixture(repoRoot => {
   write(repoRoot, 'skills/feature/SKILL.md', '---\nname: feature\ndescription: Feature work\nallowed-tools: Bash\n---\nRun !`touch /tmp/never-run`');
   assert.throws(() => resolve(repoRoot, { proposedIds: ['skill:feature'] }, { load: true }), /authority|dynamic/);

From 8e99eaa4e306ed518b26e3d58f4b052ca6315b73 Mon Sep 17 00:00:00 2001
From: montjeffrey <montjeffrey@gmail.com>
Date: Sun, 11 Oct 2026 10:33:21 -0400
Subject: [PATCH 7/7] perf(context): load the registry once per resolution and
 routing index build

compileContextProfile now validates its options, loads the registry and
calls compileContextPlan(registry, options), which compiles against a
registry the caller loaded in the same call. resolveTaskContext and
routingEntries load the registry once and compile from it, so the sources
are hashed once per call instead of twice.

On this repository (full@1, Windows 11, median of 5):
resolveTaskContext 1332 ms -> 645 ms, routingEntries 1321 ms -> 721 ms.

The registry never comes from outside the call: nothing caches it or
accepts one from a caller, so a stale or forged registry cannot be passed
in. The two "Registry changed during ..." checks compared the two loads and
are gone with the second load; each call now works from one snapshot, and
skill bytes are still verified against it when read. compileContextProfile
keeps its signature, output and error order.

Split out of #2945, whose prompt hook is superseded by #3453.
---
 scripts/lib/context-profiles.js  | 23 ++++++++++++++++++-----
 scripts/lib/context-selection.js | 11 ++++++-----
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/scripts/lib/context-profiles.js b/scripts/lib/context-profiles.js
index de80d1142..57a0db2c3 100644
--- a/scripts/lib/context-profiles.js
+++ b/scripts/lib/context-profiles.js
@@ -86,13 +86,26 @@ function estimateMetadata(entries, target, profile) {
   };
 }
 
-function compileContextProfile({
+function validatePlanOptions({ selectionMode = 'manual', target = 'codex' }) {
+  validateTarget(target);
+  if (!MODES.includes(selectionMode)) throw new Error(`Unknown selection mode: ${selectionMode}`);
+}
+
+function compileContextProfile(options = {}) {
+  validatePlanOptions(options);
+  return compileContextPlan(loadContextRegistry({ repoRoot: options.repoRoot || DEFAULT_REPO_ROOT }), options);
+}
+
+/**
+ * Compile a plan against a registry the caller loaded in the same call, so a
+ * caller that also needs the registry hashes its sources once. Callers pass
+ * the result of `loadContextRegistry` directly; nothing keeps one across calls.
+ */
+function compileContextPlan(registry, {
   repoRoot = DEFAULT_REPO_ROOT, profileId = 'lean@1', selectionMode = 'manual',
   target = 'codex', include = [], exclude = [],
 } = {}) {
-  validateTarget(target);
-  if (!MODES.includes(selectionMode)) throw new Error(`Unknown selection mode: ${selectionMode}`);
-  const registry = loadContextRegistry({ repoRoot });
+  validatePlanOptions({ selectionMode, target });
   const profile = loadContextProfile(profileId, { repoRoot });
   if (profile.registryId !== registry.id) throw new Error('Profile registry ID mismatch');
   const selected = resolveSelection(registry, profile, include, exclude);
@@ -130,4 +143,4 @@ function compileContextProfile({
   return plan;
 }
 
-module.exports = { compileContextProfile, explainContextEntry, loadContextProfile };
+module.exports = { compileContextProfile, compileContextPlan, explainContextEntry, loadContextProfile };
diff --git a/scripts/lib/context-selection.js b/scripts/lib/context-selection.js
index 5a642d455..eeef1d406 100644
--- a/scripts/lib/context-selection.js
+++ b/scripts/lib/context-selection.js
@@ -2,7 +2,7 @@
 
 const yaml = require('js-yaml');
 const { loadContextRegistry, loadSkillTriggers } = require('./context-pack-registry');
-const { compileContextProfile } = require('./context-profiles');
+const { compileContextPlan } = require('./context-profiles');
 const { buildRetrievalIndex, searchRetrieval } = require('./context-retrieval');
 const { DEFAULT_REPO_ROOT, createSourceReader, digestObject } = require('./context-profile-support');
 
@@ -310,10 +310,10 @@ function admissibility(byId, excluded, reader) {
  * without explicit selection. Metadata only; bodies are never returned. */
 function routingEntries({ repoRoot = DEFAULT_REPO_ROOT, profileId = 'lean@1', target = 'codex',
   selectionMode = 'auto', include = [], exclude = [] } = {}) {
-  const plan = compileContextProfile({ repoRoot, profileId, target, selectionMode, include, exclude });
+  // One registry load serves the plan and the routing entries.
   const registry = loadContextRegistry({ repoRoot });
+  const plan = compileContextPlan(registry, { repoRoot, profileId, target, selectionMode, include, exclude });
   const { triggers } = loadSkillTriggers({ repoRoot });
-  if (registry.registryDigest !== plan.registryDigest) throw new Error('Registry changed during routing index build');
   const byId = new Map(registry.entries.map(entry => [entry.id, entry]));
   const excluded = new Set(plan.excludedIds);
   const admissible = admissibility(byId, excluded, createSourceReader(repoRoot));
@@ -335,10 +335,11 @@ function resolveTaskContext({ repoRoot = DEFAULT_REPO_ROOT, task, profileId = 'l
   selectionMode = 'auto', include = [], exclude = [], load = false, previous = null, expectedDigest = null } = {}) {
   validateTask(task);
   validatePrevious(previous);
-  const plan = compileContextProfile({ repoRoot, profileId, target, selectionMode, include, exclude });
+  // One registry load serves the plan and the selection; skill bytes are
+  // still verified against it when they are read.
   const registry = loadContextRegistry({ repoRoot });
+  const plan = compileContextPlan(registry, { repoRoot, profileId, target, selectionMode, include, exclude });
   const { triggers } = loadSkillTriggers({ repoRoot });
-  if (registry.registryDigest !== plan.registryDigest) throw new Error('Registry changed during task selection');
   const reader = createSourceReader(repoRoot);
   const byId = new Map(registry.entries.map(entry => [entry.id, entry]));
   const excluded = new Set(plan.excludedIds);