mirror of
https://github.com/affaan-m/ECC.git
synced 2026-10-23 03:27:50 +09:00
fix: integrate six focused workflow repairs with canonical registry identity
This commit is contained in:
16 files changed
+541
-35
No files matched your search
@@ -10,7 +10,7 @@
|
||||
"id": "hand-seeded",
|
||||
"source": "manual-curation-pending-regeneration"
|
||||
},
|
||||
"registryDigest": "72cdbf4ed32b9b08a536099ba7c621208b9b9b5aaae2210301d11224ee442a76",
|
||||
"registryDigest": "18d816c6f7d8b3639edfe7b086210d12634d79b6a27b2b4ff0973f47ab22752c",
|
||||
"schemaVersion": 1,
|
||||
"triggers": {
|
||||
"skill:agent-runtime-gateway-smoke-test": [
|
||||
|
||||
@@ -19,6 +19,7 @@ For production use, pair with an LLM judge for semantic understanding.
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import errno
|
||||
import re
|
||||
import sys
|
||||
from dataclasses import dataclass, field
|
||||
@@ -368,7 +369,9 @@ def _read_file_or_text(path: Optional[str], *, required: bool = False) -> Option
|
||||
try:
|
||||
with open(path) as f:
|
||||
return f.read()
|
||||
except FileNotFoundError:
|
||||
except OSError as error:
|
||||
if error.errno not in (errno.ENOENT, errno.ENAMETOOLONG):
|
||||
raise
|
||||
if required:
|
||||
print(f"Error: output file '{path}' not found", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
@@ -382,7 +385,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()
|
||||
|
||||
|
||||
|
||||
@@ -579,13 +579,24 @@ function readSedExecute(script, i, budget) {
|
||||
return { end: j, parts: [], stdinExec: true };
|
||||
}
|
||||
|
||||
// Text, label and filename operands run to end of line; all are data.
|
||||
// Text and filename operands run to end of line; all are data.
|
||||
function readSedLineOperand(script, i, budget) {
|
||||
let end = i + 1;
|
||||
while (end < script.length && script[end] !== '\n') { budget.spend(); end++; }
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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),
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -171,8 +229,10 @@ function plannedActions(migration, destinationScope, marketplaceAction) {
|
||||
|
||||
function verifySourceAndDestination(run, paths, migration, destinationScope, phase) {
|
||||
const expectedScopes = [migration.sourceScope, destinationScope];
|
||||
const plugins = readPluginInventory(run, paths.projectRoot, phase);
|
||||
const destination = currentEccPlugins(plugins).find(plugin => plugin.scope === destinationScope);
|
||||
return validateExpectedScopes(
|
||||
readPluginInventory(run, paths.projectRoot, phase),
|
||||
plugins,
|
||||
expectedScopes,
|
||||
{
|
||||
code: phase === 'concurrency-check'
|
||||
@@ -183,7 +243,9 @@ function verifySourceAndDestination(run, paths, migration, destinationScope, pha
|
||||
? 'Claude plugin scopes changed during migration; the source was not removed.'
|
||||
: `Could not verify ${CURRENT_PLUGIN_ID} at the destination before source cleanup.`,
|
||||
phase,
|
||||
recovery: recoveryCommands(null, destinationScope),
|
||||
recovery: destination && destination.enabled !== true
|
||||
? enableRecoveryCommands(destinationScope)
|
||||
: recoveryCommands(null, destinationScope),
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -208,23 +270,32 @@ 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',
|
||||
...(observedScopes.length === 0
|
||||
? [`ecc setup --mode claude-plugin --scope ${destinationScope} --yes`]
|
||||
: recoveryCommands(sourcePresent ? migration.sourceScope : null, destinationScope)),
|
||||
],
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -265,6 +336,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)
|
||||
|
||||
@@ -86,13 +86,28 @@ 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 = {}) {
|
||||
return compileContextProfileWithRegistry(options).plan;
|
||||
}
|
||||
|
||||
// Keep registry reuse inside this call to the canonical loader. Callers can
|
||||
// consume the snapshot, but cannot supply a cached or caller-built registry.
|
||||
function compileContextProfileWithRegistry({ repoRoot = DEFAULT_REPO_ROOT, ...options } = {}) {
|
||||
validatePlanOptions(options);
|
||||
const registry = loadContextRegistry({ repoRoot });
|
||||
return { registry, plan: compileContextPlan(registry, { repoRoot, ...options }) };
|
||||
}
|
||||
|
||||
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 +145,4 @@ function compileContextProfile({
|
||||
return plan;
|
||||
}
|
||||
|
||||
module.exports = { compileContextProfile, explainContextEntry, loadContextProfile };
|
||||
module.exports = { compileContextProfile, compileContextProfileWithRegistry, explainContextEntry, loadContextProfile };
|
||||
@@ -1,8 +1,8 @@
|
||||
'use strict';
|
||||
|
||||
const yaml = require('js-yaml');
|
||||
const { loadContextRegistry, loadSkillTriggers } = require('./context-pack-registry');
|
||||
const { compileContextProfile } = require('./context-profiles');
|
||||
const { loadSkillTriggers } = require('./context-pack-registry');
|
||||
const { compileContextProfileWithRegistry } = require('./context-profiles');
|
||||
const { buildRetrievalIndex, searchRetrieval } = require('./context-retrieval');
|
||||
const { DEFAULT_REPO_ROOT, createSourceReader, digestObject } = require('./context-profile-support');
|
||||
|
||||
@@ -310,10 +310,9 @@ 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 });
|
||||
const registry = loadContextRegistry({ repoRoot });
|
||||
// One registry load serves the plan and the routing entries.
|
||||
const { registry, plan } = compileContextProfileWithRegistry({ 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 +334,10 @@ 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 });
|
||||
const registry = loadContextRegistry({ repoRoot });
|
||||
// One registry load serves the plan and the selection; skill bytes are
|
||||
// still verified against it when they are read.
|
||||
const { registry, plan } = compileContextProfileWithRegistry({ 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);
|
||||
|
||||
@@ -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 failureReasons = [input.failure_reason, input.failureReason];
|
||||
|
||||
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 (failureReasons.some(value => value != null && typeof value !== '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');
|
||||
}
|
||||
|
||||
@@ -19,6 +19,7 @@ For production use, pair with an LLM judge for semantic understanding.
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import errno
|
||||
import re
|
||||
import sys
|
||||
from dataclasses import dataclass, field
|
||||
@@ -368,7 +369,9 @@ def _read_file_or_text(path: Optional[str], *, required: bool = False) -> Option
|
||||
try:
|
||||
with open(path) as f:
|
||||
return f.read()
|
||||
except FileNotFoundError:
|
||||
except OSError as error:
|
||||
if error.errno not in (errno.ENOENT, errno.ENAMETOOLONG):
|
||||
raise
|
||||
if required:
|
||||
print(f"Error: output file '{path}' not found", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
@@ -382,7 +385,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()
|
||||
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -9,7 +9,13 @@ 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 +27,37 @@ class _FakeScenario:
|
||||
setup_commands: tuple[str, ...] = ()
|
||||
|
||||
|
||||
class TestSafeSandboxDirectory:
|
||||
def test_empty_id_cannot_delete_the_shared_base(self) -> None:
|
||||
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) -> None:
|
||||
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) -> None:
|
||||
assert _safe_sandbox_dir("scenario/one") == SANDBOX_BASE / "scenario_one"
|
||||
|
||||
def test_resolved_escape_is_still_rejected(self, tmp_path, monkeypatch) -> None:
|
||||
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.
|
||||
|
||||
|
||||
@@ -2183,7 +2183,18 @@ 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],
|
||||
['e command after a newline label is scanned', "sed ':a\ne 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}`, () => {
|
||||
|
||||
@@ -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,170 @@ 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 --yes',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
test('a two-scope disabled destination offers enable recovery without removing the source', () => {
|
||||
withFixture({
|
||||
plugins: [plugin('user'), plugin('project', { enabled: false })],
|
||||
marketplaces: [marketplace('user')],
|
||||
}, fixture => {
|
||||
const error = captureError(() => migrateClaudePluginScope(migrationOptions(fixture, 'project')));
|
||||
assert.strictEqual(error.code, 'DESTINATION_VERIFICATION_FAILED');
|
||||
assert.deepStrictEqual(error.recovery, [
|
||||
'claude plugin enable ecc@ecc --scope project',
|
||||
'ecc setup --mode claude-plugin --scope project --move-scope --yes',
|
||||
]);
|
||||
assert.ok(!readCalls(fixture).some(argv => argv[1] === 'uninstall'));
|
||||
});
|
||||
});
|
||||
|
||||
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
|
||||
process.exit(failed > 0 ? 1 : 0);
|
||||
@@ -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/);
|
||||
@@ -727,3 +747,11 @@ test('singular forms do not create exact directive admission', () => {
|
||||
assert.ok(result.candidates.some(candidate => candidate.id === 'skill:database-migrations'));
|
||||
assert.ok(!result.receipt.autoSelection || !result.receipt.autoSelection.exact);
|
||||
});
|
||||
|
||||
test('invalid resolution and routing options are rejected before reading a missing registry', () => {
|
||||
const repoRoot = path.join(__dirname, 'missing-registry');
|
||||
const values = { repoRoot, target: 'unknown-host' };
|
||||
assert.throws(() => resolveTaskContext({ ...values, task: task() }), /target/i);
|
||||
assert.throws(() => routingEntries(values), /target/i);
|
||||
assert.throws(() => routingEntries({ repoRoot, selectionMode: 'invalid' }), /Unknown selection mode/);
|
||||
});
|
||||
@@ -445,11 +445,41 @@ 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
|
||||
);
|
||||
}
|
||||
for (const snake of ['', null, undefined]) {
|
||||
assert.strictEqual(tracker.normalizeExecutionRecord({
|
||||
...valid, failure_reason: snake, failureReason: 'Useful detail',
|
||||
}).failure_reason, 'Useful detail');
|
||||
}
|
||||
assert.strictEqual(tracker.normalizeExecutionRecord({
|
||||
...valid, failure_reason: 'Primary detail', failureReason: 'Other detail',
|
||||
}).failure_reason, 'Primary detail');
|
||||
for (const value of [42, 0, false, {}, []]) {
|
||||
for (const payload of [
|
||||
{ ...valid, failure_reason: value, failureReason: 'Useful detail' },
|
||||
{ ...valid, failure_reason: '', failureReason: value },
|
||||
]) assert.throws(() => tracker.normalizeExecutionRecord(payload), /failure_reason must be a string or 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', () => {
|
||||
|
||||
@@ -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',
|
||||
]);
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
"""Input-mode regressions for the standalone agent output evaluator."""
|
||||
|
||||
import argparse
|
||||
import errno
|
||||
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)
|
||||
|
||||
|
||||
def test_long_inline_task_remains_text(tmp_path, monkeypatch):
|
||||
output = tmp_path / "response.txt"
|
||||
output.write_text("Answer", encoding="utf-8")
|
||||
task = "Long explicit task description. " * 60
|
||||
monkeypatch.setattr(sys, "stdin", io.StringIO("Ignored pipe"))
|
||||
assert evaluator._read_input(args(task=task, output=str(output))) == (task, "Answer")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("error_code", [errno.EACCES, errno.EISDIR])
|
||||
def test_task_read_does_not_hide_other_file_errors(monkeypatch, error_code):
|
||||
def fail_open(_path):
|
||||
raise OSError(error_code, "Cannot read task")
|
||||
|
||||
monkeypatch.setattr("builtins.open", fail_open)
|
||||
with pytest.raises(OSError) as captured:
|
||||
evaluator._read_file_or_text("task-file")
|
||||
assert captured.value.errno == error_code
|
||||
Reference in new issue
Block a user