From 03b441792e61914c7a6a792ebe97fe8b9acaa5d2 Mon Sep 17 00:00:00 2001 From: benno0o Date: Sun, 26 Jul 2026 23:08:48 +0200 Subject: [PATCH 01/46] fix: resolve pnpm in Git Bash pre-push hook --- scripts/codex-git-hooks/pre-push | 14 +++- tests/scripts/codex-hooks.test.js | 129 +++++++++++++++++++++++++++++- 2 files changed, 138 insertions(+), 5 deletions(-) diff --git a/scripts/codex-git-hooks/pre-push b/scripts/codex-git-hooks/pre-push index 82a6b0261..3d388e8c3 100644 --- a/scripts/codex-git-hooks/pre-push +++ b/scripts/codex-git-hooks/pre-push @@ -60,11 +60,21 @@ has_node_script() { node -e 'const fs=require("fs"); const p=JSON.parse(fs.readFileSync("package.json","utf8")); process.exit(p.scripts && p.scripts[process.argv[1]] ? 0 : 1)' "$script_name" >/dev/null 2>&1 } +run_pnpm() { + if command -v corepack >/dev/null 2>&1; then + corepack pnpm "$@" + elif command -v pnpm >/dev/null 2>&1; then + pnpm "$@" + else + fail "pnpm could not be resolved from PATH or Corepack" + fi +} + run_node_script() { local pm="$1" local script_name="$2" case "$pm" in - pnpm) pnpm run "$script_name" ;; + pnpm) run_pnpm run "$script_name" ;; bun) bun run "$script_name" ;; yarn) yarn "$script_name" ;; npm) npm run "$script_name" ;; @@ -90,7 +100,7 @@ if [[ -f "package.json" ]]; then ran_any_check=1 log "Running dependency audit (ECC_PREPUSH_AUDIT=1)" case "$pm" in - pnpm) pnpm audit --prod || fail "pnpm audit failed" ;; + pnpm) run_pnpm audit --prod || fail "pnpm audit failed" ;; bun) bun audit || fail "bun audit failed" ;; yarn) yarn npm audit --recursive || fail "yarn audit failed" ;; npm) npm audit --omit=dev || fail "npm audit failed" ;; diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 1c49f4c63..a61c5bfea 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -11,6 +11,7 @@ const TOML = require('@iarna/toml'); const repoRoot = path.join(__dirname, '..', '..'); const installScript = path.join(repoRoot, 'scripts', 'codex', 'install-global-git-hooks.sh'); +const prePushHook = path.join(repoRoot, 'scripts', 'codex-git-hooks', 'pre-push'); const pluginCacheCheckScript = path.join(repoRoot, 'scripts', 'codex', 'check-plugin-cache.js'); const mergeCodexConfigScript = path.join(repoRoot, 'scripts', 'codex', 'merge-codex-config.js'); const mergeMcpConfigScript = path.join(repoRoot, 'scripts', 'codex', 'merge-mcp-config.js'); @@ -42,18 +43,30 @@ function cleanup(dirPath) { fs.rmSync(dirPath, { recursive: true, force: true }); } -function runBash(scriptPath, args = [], env = {}, cwd = repoRoot) { - return spawnSync('bash', [scriptPath, ...args], { +function runBash(scriptPath, args = [], env = {}, cwd = repoRoot, input = undefined, preservePath = true) { + const bash = process.platform === 'win32' && fs.existsSync('C:\\Program Files\\Git\\bin\\bash.exe') + ? 'C:\\Program Files\\Git\\bin\\bash.exe' + : fs.existsSync('/bin/bash') + ? '/bin/bash' + : 'bash'; + return spawnSync(bash, [scriptPath, ...args], { cwd, env: { - ...process.env, + ...(preservePath ? process.env : {}), ...env, }, encoding: 'utf8', + input, stdio: ['pipe', 'pipe', 'pipe'], }); } +function toBashPath(filePath) { + return process.platform === 'win32' + ? `/${filePath[0].toLowerCase()}${filePath.slice(2).replaceAll('\\', '/')}` + : filePath; +} + function runNode(scriptPath, args = [], env = {}, cwd = repoRoot) { return spawnSync('node', [scriptPath, ...args], { cwd, @@ -116,6 +129,116 @@ const cacheManifestWithLocalRefs = { let passed = 0; let failed = 0; +function makeExecutable(filePath, content) { + fs.writeFileSync(filePath, content, { mode: 0o755 }); + fs.chmodSync(filePath, 0o755); +} + +function runHermeticPrePush({ failScript = null, includeCorepack = true, includePnpm = false } = {}) { + const tempDir = createTempDir('codex-pre-push-'); + const binDir = path.join(tempDir, 'bin'); + const projectDir = path.join(tempDir, 'project'); + const callsPath = path.join(tempDir, 'calls.txt'); + const bashEnv = path.join(tempDir, 'bash-env'); + fs.mkdirSync(binDir); + fs.mkdirSync(projectDir); + const functionStub = (name, corepack) => `${name}() { +printf '%s\\n' "${corepack ? '' : 'pnpm '}$*" >> "${toBashPath(callsPath)}" +${corepack ? 'shift' : ':'} +shift +test "$1" != "${failScript || '__never__'}" +}`; + fs.writeFileSync( + bashEnv, + `git() { return 0; } +node() { "${toBashPath(process.execPath)}" "$@"; } +${includeCorepack ? functionStub('corepack', true) : ''} +${includePnpm ? functionStub('pnpm', false) : ''} +`, + ); + fs.writeFileSync(path.join(projectDir, 'pnpm-lock.yaml'), 'lockfileVersion: 9\n'); + const initialized = spawnSync('git', ['init', '--quiet'], { cwd: projectDir }); + assert.strictEqual(initialized.status, 0, initialized.stderr?.toString()); + writeJson(path.join(projectDir, 'package.json'), { + packageManager: 'pnpm@11.9.0', + scripts: { lint: 'x', typecheck: 'x', test: 'x', build: 'x' }, + }); + const result = runBash( + prePushHook, + [], + { + PATH: toBashPath(binDir), + BASH_ENV: toBashPath(bashEnv), + ECC_PREPUSH_AUDIT: '0', + ECC_SKIP_GIT_HOOKS: '0', + ECC_SKIP_PREPUSH: '0', + MSYS_NO_PATHCONV: '1', + }, + projectDir, + Buffer.from('refs/heads/main 1111111111111111111111111111111111111111 refs/heads/main 0000000000000000000000000000000000000000\n'), + false, + ); + const calls = fs.existsSync(callsPath) + ? fs.readFileSync(callsPath, 'utf8').trim().split(/\r?\n/) + : []; + cleanup(tempDir); + return { result, calls }; +} + +if ( + test('pre-push uses Corepack pinned pnpm and runs every required verification script', () => { + const { result, calls } = runHermeticPrePush(); + assert.strictEqual(result.status, 0, JSON.stringify(result, null, 2)); + assert.deepStrictEqual(calls, [ + 'pnpm run lint', + 'pnpm run typecheck', + 'pnpm run test', + 'pnpm run build', + ], JSON.stringify(result, null, 2)); + }) +) + passed++; +else failed++; + +if ( + test('pre-push falls back to direct pnpm when Corepack is absent', () => { + const { result, calls } = runHermeticPrePush({ + includeCorepack: false, + includePnpm: true, + }); + assert.strictEqual(result.status, 0, `${result.stdout}\n${result.stderr}`); + assert.deepStrictEqual(calls, [ + 'pnpm run lint', + 'pnpm run typecheck', + 'pnpm run test', + 'pnpm run build', + ]); + }) +) + passed++; +else failed++; + +if ( + test('pre-push fails closed when pnpm and Corepack cannot resolve', () => { + const { result } = runHermeticPrePush({ includeCorepack: false }); + assert.notStrictEqual(result.status, 0, `${result.stdout}\n${result.stderr}`); + assert.match(result.stderr, /pnpm.*(?:resolve|found)/i); + }) +) + passed++; +else failed++; + +if ( + test('pre-push stops immediately when a required verification script fails', () => { + const { result, calls } = runHermeticPrePush({ failScript: 'typecheck' }); + assert.notStrictEqual(result.status, 0, `${result.stdout}\n${result.stderr}`); + assert.deepStrictEqual(calls, ['pnpm run lint', 'pnpm run typecheck']); + assert.match(result.stderr, /typecheck failed/); + }) +) + passed++; +else failed++; + if ( test('check-plugin-cache fails when the installed cache is missing manifest-referenced files', () => { const homeDir = createTempDir('codex-plugin-cache-home-'); From b48f22f08c7e697e9df74037bb89b2e1526fa35d Mon Sep 17 00:00:00 2001 From: benno0o Date: Wed, 29 Jul 2026 14:37:45 +0200 Subject: [PATCH 02/46] fix: address pre-push pnpm review findings --- scripts/codex-git-hooks/pre-push | 2 ++ tests/scripts/codex-hooks.test.js | 41 +++++++++++++++++-------------- 2 files changed, 24 insertions(+), 19 deletions(-) mode change 100644 => 100755 scripts/codex-git-hooks/pre-push diff --git a/scripts/codex-git-hooks/pre-push b/scripts/codex-git-hooks/pre-push old mode 100644 new mode 100755 index 3d388e8c3..2ee23c7f4 --- a/scripts/codex-git-hooks/pre-push +++ b/scripts/codex-git-hooks/pre-push @@ -62,6 +62,8 @@ has_node_script() { run_pnpm() { if command -v corepack >/dev/null 2>&1; then + # Corepack may download the pinned pnpm version on a cache miss. Set + # COREPACK_ENABLE_NETWORK=0 to make an offline cache miss fail immediately. corepack pnpm "$@" elif command -v pnpm >/dev/null 2>&1; then pnpm "$@" diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index a61c5bfea..37ccfe1c8 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -43,7 +43,10 @@ function cleanup(dirPath) { fs.rmSync(dirPath, { recursive: true, force: true }); } -function runBash(scriptPath, args = [], env = {}, cwd = repoRoot, input = undefined, preservePath = true) { +function runBash( + scriptPath, + { args = [], env = {}, cwd = repoRoot, input = undefined, preservePath = true } = {}, +) { const bash = process.platform === 'win32' && fs.existsSync('C:\\Program Files\\Git\\bin\\bash.exe') ? 'C:\\Program Files\\Git\\bin\\bash.exe' : fs.existsSync('/bin/bash') @@ -129,11 +132,6 @@ const cacheManifestWithLocalRefs = { let passed = 0; let failed = 0; -function makeExecutable(filePath, content) { - fs.writeFileSync(filePath, content, { mode: 0o755 }); - fs.chmodSync(filePath, 0o755); -} - function runHermeticPrePush({ failScript = null, includeCorepack = true, includePnpm = false } = {}) { const tempDir = createTempDir('codex-pre-push-'); const binDir = path.join(tempDir, 'bin'); @@ -163,10 +161,8 @@ ${includePnpm ? functionStub('pnpm', false) : ''} packageManager: 'pnpm@11.9.0', scripts: { lint: 'x', typecheck: 'x', test: 'x', build: 'x' }, }); - const result = runBash( - prePushHook, - [], - { + const result = runBash(prePushHook, { + env: { PATH: toBashPath(binDir), BASH_ENV: toBashPath(bashEnv), ECC_PREPUSH_AUDIT: '0', @@ -174,10 +170,10 @@ ${includePnpm ? functionStub('pnpm', false) : ''} ECC_SKIP_PREPUSH: '0', MSYS_NO_PATHCONV: '1', }, - projectDir, - Buffer.from('refs/heads/main 1111111111111111111111111111111111111111 refs/heads/main 0000000000000000000000000000000000000000\n'), - false, - ); + cwd: projectDir, + input: Buffer.from('refs/heads/main 1111111111111111111111111111111111111111 refs/heads/main 0000000000000000000000000000000000000000\n'), + preservePath: false, + }); const calls = fs.existsSync(callsPath) ? fs.readFileSync(callsPath, 'utf8').trim().split(/\r?\n/) : []; @@ -389,9 +385,11 @@ if (os.platform() === 'win32') { const weirdHooksDir = path.join(homeDir, 'git-hooks "quoted"'); try { - const result = runBash(installScript, [], { - HOME: homeDir, - ECC_GLOBAL_HOOKS_DIR: weirdHooksDir, + const result = runBash(installScript, { + env: { + HOME: homeDir, + ECC_GLOBAL_HOOKS_DIR: weirdHooksDir, + }, }); assert.strictEqual(result.status, 0, result.stderr || result.stdout); @@ -786,7 +784,10 @@ if ( fs.mkdirSync(codexDir, { recursive: true }); fs.writeFileSync(configPath, config); - const syncResult = runBash(syncScript, ['--update-mcp'], makeHermeticCodexEnv(homeDir, codexDir)); + const syncResult = runBash(syncScript, { + args: ['--update-mcp'], + env: makeHermeticCodexEnv(homeDir, codexDir), + }); assert.strictEqual(syncResult.status, 0, `${syncResult.stdout}\n${syncResult.stderr}`); const syncedAgents = fs.readFileSync(agentsPath, 'utf8'); @@ -847,7 +848,9 @@ if ( fs.mkdirSync(codexDir, { recursive: true }); fs.writeFileSync(configPath, config); - const syncResult = runBash(syncScript, [], makeHermeticCodexEnv(homeDir, codexDir)); + const syncResult = runBash(syncScript, { + env: makeHermeticCodexEnv(homeDir, codexDir), + }); assert.strictEqual(syncResult.status, 0, `${syncResult.stdout}\n${syncResult.stderr}`); const parsedConfig = TOML.parse(fs.readFileSync(configPath, 'utf8')); From 7c2bc54be2689bba4278ec6d36cb6edca0242030 Mon Sep 17 00:00:00 2001 From: benno0o Date: Wed, 29 Jul 2026 14:51:55 +0200 Subject: [PATCH 03/46] test: verify Corepack uses the pinned pnpm version --- tests/scripts/codex-hooks.test.js | 43 +++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 37ccfe1c8..628a37da7 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -132,7 +132,12 @@ const cacheManifestWithLocalRefs = { let passed = 0; let failed = 0; -function runHermeticPrePush({ failScript = null, includeCorepack = true, includePnpm = false } = {}) { +function runHermeticPrePush({ + failScript = null, + includeCorepack = true, + includePnpm = false, + audit = false, +} = {}) { const tempDir = createTempDir('codex-pre-push-'); const binDir = path.join(tempDir, 'bin'); const projectDir = path.join(tempDir, 'project'); @@ -141,6 +146,7 @@ function runHermeticPrePush({ failScript = null, includeCorepack = true, include fs.mkdirSync(binDir); fs.mkdirSync(projectDir); const functionStub = (name, corepack) => `${name}() { +${corepack ? 'node -e \'const p=require("./package.json"); process.exit(p.packageManager === "pnpm@11.9.0" ? 0 : 1)\' || return 97' : ':'} printf '%s\\n' "${corepack ? '' : 'pnpm '}$*" >> "${toBashPath(callsPath)}" ${corepack ? 'shift' : ':'} shift @@ -165,7 +171,7 @@ ${includePnpm ? functionStub('pnpm', false) : ''} env: { PATH: toBashPath(binDir), BASH_ENV: toBashPath(bashEnv), - ECC_PREPUSH_AUDIT: '0', + ECC_PREPUSH_AUDIT: audit ? '1' : '0', ECC_SKIP_GIT_HOOKS: '0', ECC_SKIP_PREPUSH: '0', MSYS_NO_PATHCONV: '1', @@ -235,6 +241,39 @@ if ( passed++; else failed++; +if ( + test('pre-push runs the production audit through Corepack pnpm', () => { + const { result, calls } = runHermeticPrePush({ audit: true }); + assert.strictEqual(result.status, 0, `${result.stdout}\n${result.stderr}`); + assert.deepStrictEqual(calls, [ + 'pnpm run lint', + 'pnpm run typecheck', + 'pnpm run test', + 'pnpm run build', + 'pnpm audit --prod', + ]); + }) +) + passed++; +else failed++; + +if ( + test('pre-push fails closed when the production audit fails', () => { + const { result, calls } = runHermeticPrePush({ audit: true, failScript: '--prod' }); + assert.notStrictEqual(result.status, 0, `${result.stdout}\n${result.stderr}`); + assert.deepStrictEqual(calls, [ + 'pnpm run lint', + 'pnpm run typecheck', + 'pnpm run test', + 'pnpm run build', + 'pnpm audit --prod', + ]); + assert.match(result.stderr, /pnpm audit failed/); + }) +) + passed++; +else failed++; + if ( test('check-plugin-cache fails when the installed cache is missing manifest-referenced files', () => { const homeDir = createTempDir('codex-plugin-cache-home-'); From 1444239eec51e63594d0588f7b0c1c4e05f41d6b Mon Sep 17 00:00:00 2001 From: Haoran Zhang Date: Mon, 27 Jul 2026 17:27:57 -0700 Subject: [PATCH 04/46] feat(install): add AdaL CLI install target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following the hermes/openclaw (#2433) and kimi (#2441) adapter recipe. What's included (adal-project adapter, project kind, ./.adal root, same shape as kimi-project/joycode-project): - scripts/lib/install-targets/adal-project.js — 10-line project-kind adapter targeting ./.adal - Registry + helpers platform-ownership wiring - adal target on the 5 shared modules (rules-core, agents-core, commands-core, platform-configs, workflow-quality) + SUPPORTED_INSTALL_TARGETS + legacy-compat module - .adal in platform-configs paths - Both schema enums (install-modules, ecc-install-config), npm files allowlist, installer help text, .adal/README.md stub AdaL (adalagent.ai) is a terminal-based AI coding agent (by SylphAI) built on AdalFlow, with native MCP support and project-scoped config under ./.adal/ (skills, custom tools, memory) plus a root-level AGENTS.md instructions file — matching the shape ECC already installs into other AGENTS.md-based harnesses (Codex, OpenCode, Kimi). Verified: full suite matches main's baseline (3334 passed, same pre-existing failures unrelated to this change — OpenCode build/npm-pack surface tests requiring build tooling not present in this sandbox); catalog check passes (67 agents / 94 commands / 281 skills); dry-run resolves Target: adal / Adapter: adal-project / root ./.adal with all 5 modules planned; doctor reports OK after a real install; uninstall cleanly reverses all 458 operations. Co-Authored-By: AdaL --- .adal/README.md | 22 +++++++++++++++++++ manifests/install-modules.json | 24 ++++++++++++++------- package.json | 1 + schemas/ecc-install-config.schema.json | 3 ++- schemas/install-modules.schema.json | 3 ++- scripts/install-apply.js | 1 + scripts/lib/install-manifests.js | 9 +++++++- scripts/lib/install-targets/adal-project.js | 10 +++++++++ scripts/lib/install-targets/helpers.js | 1 + scripts/lib/install-targets/registry.js | 2 ++ 10 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 .adal/README.md create mode 100644 scripts/lib/install-targets/adal-project.js diff --git a/.adal/README.md b/.adal/README.md new file mode 100644 index 000000000..56e3a4ad3 --- /dev/null +++ b/.adal/README.md @@ -0,0 +1,22 @@ +# ECC for AdaL CLI + +This directory contains the ECC (Everything Claude Code) configuration for the AdaL CLI harness. + +## What is installed + +- `rules/ecc/` — shared coding rules and guidelines +- `skills/ecc/` — reusable skills +- `commands/` — slash commands +- `AGENTS.md` — agent instructions + +## Manual install + +```bash +bash ./install.sh --target adal --profile minimal +``` + +## Notes + +- The `adal` target installs into the project-level `./.adal/` directory. +- AdaL's own config (`~/.adal/settings.json`, MCP servers, plugins) is **not** touched by ECC install. +- Use `npx ecc doctor --target adal` to check install health. diff --git a/manifests/install-modules.json b/manifests/install-modules.json index 1b5ea5a6d..0e45965a5 100644 --- a/manifests/install-modules.json +++ b/manifests/install-modules.json @@ -19,7 +19,8 @@ "zed", "hermes", "openclaw", - "kimi" + "kimi", + "adal" ], "dependencies": [], "defaultInstall": true, @@ -47,7 +48,8 @@ "zed", "hermes", "openclaw", - "kimi" + "kimi", + "adal" ], "dependencies": [], "defaultInstall": true, @@ -75,7 +77,8 @@ "zed", "hermes", "openclaw", - "kimi" + "kimi", + "adal" ], "dependencies": [], "defaultInstall": true, @@ -121,7 +124,8 @@ "scripts/setup-package-manager.js", ".hermes", ".openclaw", - ".kimi" + ".kimi", + ".adal" ], "targets": [ "claude", @@ -137,7 +141,8 @@ "zed", "hermes", "openclaw", - "kimi" + "kimi", + "adal" ], "dependencies": [], "defaultInstall": true, @@ -294,7 +299,8 @@ "zed", "hermes", "openclaw", - "kimi" + "kimi", + "adal" ], "dependencies": [ "platform-configs" @@ -369,7 +375,8 @@ "zed", "hermes", "openclaw", - "kimi" + "kimi", + "adal" ], "dependencies": [ "skill-unified-memory" @@ -627,7 +634,8 @@ "zed", "hermes", "openclaw", - "kimi" + "kimi", + "adal" ], "dependencies": [ "platform-configs" diff --git a/package.json b/package.json index 8be4b5d3f..80c63f25a 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "url": "https://github.com/affaan-m/ECC/issues" }, "files": [ + ".adal/", ".agents/", ".claude-plugin/", ".codex/", diff --git a/schemas/ecc-install-config.schema.json b/schemas/ecc-install-config.schema.json index d4e4bee96..29b57538f 100644 --- a/schemas/ecc-install-config.schema.json +++ b/schemas/ecc-install-config.schema.json @@ -31,7 +31,8 @@ "zed", "hermes", "openclaw", - "kimi" + "kimi", + "adal" ] }, "profile": { diff --git a/schemas/install-modules.schema.json b/schemas/install-modules.schema.json index 3cff4a892..620f616dc 100644 --- a/schemas/install-modules.schema.json +++ b/schemas/install-modules.schema.json @@ -61,7 +61,8 @@ "zed", "hermes", "openclaw", - "kimi" + "kimi", + "adal" ] } }, diff --git a/scripts/install-apply.js b/scripts/install-apply.js index 97d8279c9..128085c6c 100755 --- a/scripts/install-apply.js +++ b/scripts/install-apply.js @@ -47,6 +47,7 @@ Targets: hermes - Install shared rules/skills/commands into ~/.hermes/ kimi - Install Kimi Code project instructions, skills, and MCP config into ./.kimi-code/ (ECC hooks not configured) openclaw - Install shared rules/skills/commands into ~/.openclaw/ + adal - Install shared rules/skills/commands into ./.adal/ Options: --profile Resolve and install a manifest profile diff --git a/scripts/lib/install-manifests.js b/scripts/lib/install-manifests.js index 2859a9e40..bb16cc93c 100644 --- a/scripts/lib/install-manifests.js +++ b/scripts/lib/install-manifests.js @@ -5,7 +5,7 @@ const { getInstallTargetAdapter, planInstallTargetScaffold } = require('./instal const { resolveInvocationEnvironment } = require('./invocation-environment'); const DEFAULT_REPO_ROOT = path.join(__dirname, '../..'); -const SUPPORTED_INSTALL_TARGETS = ['claude', 'claude-project', 'cursor', 'antigravity', 'codex', 'gemini', 'opencode', 'codebuddy', 'joycode', 'qwen', 'zed', 'hermes', 'openclaw', 'kimi']; +const SUPPORTED_INSTALL_TARGETS = ['claude', 'claude-project', 'cursor', 'antigravity', 'codex', 'gemini', 'opencode', 'codebuddy', 'joycode', 'qwen', 'zed', 'hermes', 'openclaw', 'kimi', 'adal']; const COMPONENT_FAMILY_PREFIXES = { baseline: 'baseline:', language: 'lang:', @@ -99,6 +99,13 @@ const LEGACY_COMPAT_BASE_MODULE_IDS_BY_TARGET = Object.freeze({ 'platform-configs', 'workflow-quality', ], + adal: [ + 'rules-core', + 'agents-core', + 'commands-core', + 'platform-configs', + 'workflow-quality', + ], }); const LEGACY_LANGUAGE_ALIAS_TO_CANONICAL = Object.freeze({ c: 'c', diff --git a/scripts/lib/install-targets/adal-project.js b/scripts/lib/install-targets/adal-project.js new file mode 100644 index 000000000..313f5437a --- /dev/null +++ b/scripts/lib/install-targets/adal-project.js @@ -0,0 +1,10 @@ +const { createInstallTargetAdapter } = require('./helpers'); + +module.exports = createInstallTargetAdapter({ + id: 'adal-project', + target: 'adal', + kind: 'project', + rootSegments: ['.adal'], + installStatePathSegments: ['ecc-install-state.json'], + nativeRootRelativePath: '.adal', +}); diff --git a/scripts/lib/install-targets/helpers.js b/scripts/lib/install-targets/helpers.js index cb8f05898..9dedcf50a 100644 --- a/scripts/lib/install-targets/helpers.js +++ b/scripts/lib/install-targets/helpers.js @@ -16,6 +16,7 @@ const PLATFORM_SOURCE_PATH_OWNERS = Object.freeze({ '.codebuddy': 'codebuddy', '.qwen': 'qwen', '.zed': 'zed', + '.adal': 'adal', }); function normalizeRelativePath(relativePath) { diff --git a/scripts/lib/install-targets/registry.js b/scripts/lib/install-targets/registry.js index 6861a63e9..8dd7cf848 100644 --- a/scripts/lib/install-targets/registry.js +++ b/scripts/lib/install-targets/registry.js @@ -1,3 +1,4 @@ +const adalProject = require('./adal-project'); const antigravityProject = require('./antigravity-project'); const claudeHome = require('./claude-home'); const claudeProject = require('./claude-project'); @@ -29,6 +30,7 @@ const ADAPTERS = Object.freeze([ kimiProject, qwenHome, zedProject, + adalProject, ]); function listInstallTargetAdapters() { From 1e7493595c386506161892ac1b8f76f115ea5c8a Mon Sep 17 00:00:00 2001 From: Haoran Zhang Date: Mon, 27 Jul 2026 21:01:45 -0700 Subject: [PATCH 05/46] fix(adal): correct README install paths and add adapter regression tests Address CodeRabbit review feedback on PR #2607: - Fix .adal/README.md to document actual install paths (rules/, skills/) instead of the incorrect namespaced rules/ecc/, skills/ecc/ paths. - Add regression tests for the adal-project install target adapter: root/install-state path resolution, dual id/target registry lookup, native .adal root sync-root-children behavior, and foreign platform path filtering. Co-Authored-By: AdaL --- .adal/README.md | 4 +- tests/lib/install-targets.test.js | 113 ++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 2 deletions(-) diff --git a/.adal/README.md b/.adal/README.md index 56e3a4ad3..1052757c1 100644 --- a/.adal/README.md +++ b/.adal/README.md @@ -4,8 +4,8 @@ This directory contains the ECC (Everything Claude Code) configuration for the A ## What is installed -- `rules/ecc/` — shared coding rules and guidelines -- `skills/ecc/` — reusable skills +- `rules/` — shared coding rules and guidelines +- `skills/` — reusable skills - `commands/` — slash commands - `AGENTS.md` — agent instructions diff --git a/tests/lib/install-targets.test.js b/tests/lib/install-targets.test.js index 121ed0753..295898aae 100644 --- a/tests/lib/install-targets.test.js +++ b/tests/lib/install-targets.test.js @@ -975,6 +975,119 @@ function runTests() { ); })) passed++; else failed++; + if (test('resolves adal adapter root and install-state path from project root', () => { + const adapter = getInstallTargetAdapter('adal'); + const projectRoot = '/workspace/app'; + const root = adapter.resolveRoot({ projectRoot }); + const statePath = adapter.getInstallStatePath({ projectRoot }); + + assert.strictEqual(adapter.id, 'adal-project'); + assert.strictEqual(adapter.target, 'adal'); + assert.strictEqual(adapter.kind, 'project'); + assert.strictEqual(root, path.join(projectRoot, '.adal')); + assert.strictEqual(statePath, path.join(projectRoot, '.adal', 'ecc-install-state.json')); + })) passed++; else failed++; + + if (test('adal adapter supports lookup by target and adapter id', () => { + const byTarget = getInstallTargetAdapter('adal'); + const byId = getInstallTargetAdapter('adal-project'); + + assert.strictEqual(byTarget.id, 'adal-project'); + assert.strictEqual(byId.id, 'adal-project'); + assert.ok(byTarget.supports('adal')); + assert.ok(byTarget.supports('adal-project')); + })) passed++; else failed++; + + if (test('plans adal project rules, skills, and native root sync', () => { + const repoRoot = path.join(__dirname, '..', '..'); + const projectRoot = '/workspace/app'; + + const plan = planInstallTargetScaffold({ + target: 'adal', + repoRoot, + projectRoot, + modules: [ + { + id: 'rules-core', + paths: ['rules'], + }, + { + id: 'workflow-quality', + paths: ['skills/tdd-workflow'], + }, + { + id: 'platform-configs', + paths: ['.adal', '.cursor', '.zed'], + }, + ], + }); + + assert.strictEqual(plan.adapter.id, 'adal-project'); + assert.strictEqual(plan.targetRoot, path.join(projectRoot, '.adal')); + assert.strictEqual(plan.installStatePath, path.join(projectRoot, '.adal', 'ecc-install-state.json')); + assert.ok( + plan.operations.some(operation => ( + normalizedRelativePath(operation.sourceRelativePath) === 'rules' + && operation.destinationPath === path.join(projectRoot, '.adal', 'rules') + )), + 'Should preserve rules under .adal/rules' + ); + assert.ok( + plan.operations.some(operation => ( + normalizedRelativePath(operation.sourceRelativePath) === 'skills/tdd-workflow' + && operation.destinationPath === path.join(projectRoot, '.adal', 'skills', 'tdd-workflow') + )), + 'Should install skills under .adal/skills' + ); + assert.ok( + plan.operations.some(operation => ( + normalizedRelativePath(operation.sourceRelativePath) === '.adal' + && operation.destinationPath === path.join(projectRoot, '.adal') + && operation.strategy === 'sync-root-children' + )), + 'Should sync native .adal root children in place' + ); + })) passed++; else failed++; + + if (test('adal adapter skips foreign platform source paths', () => { + const repoRoot = path.join(__dirname, '..', '..'); + const projectRoot = '/workspace/app'; + + const plan = planInstallTargetScaffold({ + target: 'adal', + repoRoot, + projectRoot, + modules: [ + { + id: 'platform-configs', + paths: ['.cursor', '.zed', 'rules'], + }, + ], + }); + + assert.ok( + plan.operations.some(operation => ( + normalizedRelativePath(operation.sourceRelativePath) === 'rules' + && operation.destinationPath === path.join(projectRoot, '.adal', 'rules') + )), + 'Should still include non-foreign rules path (guards against empty-plan regression)' + ); + assert.ok( + !plan.operations.some(operation => ( + normalizedRelativePath(operation.sourceRelativePath) === '.cursor' + || normalizedRelativePath(operation.sourceRelativePath).startsWith('.cursor/') + )), + 'Should skip foreign Cursor platform paths' + ); + assert.ok( + !plan.operations.some(operation => ( + normalizedRelativePath(operation.sourceRelativePath) === '.zed' + || normalizedRelativePath(operation.sourceRelativePath).startsWith('.zed/') + )), + 'Should skip foreign Zed platform paths' + ); + })) passed++; else failed++; + if (test('exposes validate and planOperations on codebuddy adapter', () => { const codebuddyAdapter = getInstallTargetAdapter('codebuddy'); From 73c29bbd08bc31de994bc1f283971906e1edd737 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Tue, 11 Aug 2026 13:34:47 -0400 Subject: [PATCH 06/46] fix(install): register AdaL capability metadata --- scripts/lib/harness-capabilities.js | 13 +++++++++++++ tests/lib/harness-capabilities.test.js | 9 +++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/scripts/lib/harness-capabilities.js b/scripts/lib/harness-capabilities.js index 2dd265a26..c42f0488e 100644 --- a/scripts/lib/harness-capabilities.js +++ b/scripts/lib/harness-capabilities.js @@ -201,6 +201,19 @@ const HARNESS_CAPABILITIES = deepFreeze([ hooks: hooks('not-configured', false, 'ECC hooks are not configured by this adapter.'), aliases: [], }, + { + id: 'adal', + label: 'AdaL CLI', + targetIds: ['adal'], + channel: 'managed-project', + installMode: 'managed-project', + guidedReady: false, + availability: 'advanced', + destination: './.adal', + scopes: [scope('project', 'adal', './.adal')], + hooks: hooks('not-configured', false, 'ECC hooks are not configured by this adapter.'), + aliases: ['adal-cli'], + }, { id: 'hermes', label: 'Hermes', diff --git a/tests/lib/harness-capabilities.test.js b/tests/lib/harness-capabilities.test.js index 98264111e..4591403d4 100644 --- a/tests/lib/harness-capabilities.test.js +++ b/tests/lib/harness-capabilities.test.js @@ -33,12 +33,12 @@ function runTests() { let passed = 0; let failed = 0; - if (test('represents all 14 registered targets exactly once across 13 harnesses', () => { + if (test('represents all 15 registered targets exactly once across 14 harnesses', () => { const catalogTargetIds = HARNESS_CAPABILITIES.flatMap(harness => harness.targetIds); const adapterTargetIds = listInstallTargetAdapters().map(adapter => adapter.target); - assert.strictEqual(HARNESS_CAPABILITIES.length, 13); - assert.strictEqual(new Set(catalogTargetIds).size, 14); + assert.strictEqual(HARNESS_CAPABILITIES.length, 14); + assert.strictEqual(new Set(catalogTargetIds).size, 15); assert.deepStrictEqual([...catalogTargetIds].sort(), [...SUPPORTED_INSTALL_TARGETS].sort()); assert.deepStrictEqual([...catalogTargetIds].sort(), [...adapterTargetIds].sort()); })) passed++; else failed++; @@ -100,6 +100,7 @@ function runTests() { joycode: ['project', './.joycode'], qwen: ['home', '~/.qwen'], zed: ['project', './.zed'], + adal: ['project', './.adal'], hermes: ['home', '~/.hermes'], openclaw: ['home', '~/.openclaw'], }; @@ -170,7 +171,7 @@ function runTests() { const first = listHarnessCapabilities(); first.pop(); - assert.strictEqual(listHarnessCapabilities().length, 13); + assert.strictEqual(listHarnessCapabilities().length, 14); const guided = listGuidedHarnesses(); guided.reverse(); From 70eb0f68aeecb306b8fed94ff5960deb97435b7c Mon Sep 17 00:00:00 2001 From: CaoBochun Date: Wed, 5 Aug 2026 15:32:11 +0800 Subject: [PATCH 07/46] fix: make GAN harness score parsing portable --- scripts/gan-harness.sh | 23 ++++++++---- tests/gan-harness.test.js | 74 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 tests/gan-harness.test.js diff --git a/scripts/gan-harness.sh b/scripts/gan-harness.sh index 9aa4289ca..093e696d1 100755 --- a/scripts/gan-harness.sh +++ b/scripts/gan-harness.sh @@ -61,11 +61,18 @@ phase() { echo -e "\n${PURPLE}════════════════ extract_score() { # Extract the TOTAL weighted score from a feedback file local file="$1" - # Look for **TOTAL** or **X.X/10** pattern - grep -oP '(?<=\*\*TOTAL\*\*.*\*\*)[0-9]+\.[0-9]+' "$file" 2>/dev/null \ - || grep -oP '(?<=TOTAL.*\|.*\| \*\*)[0-9]+\.[0-9]+' "$file" 2>/dev/null \ - || grep -oP 'Verdict:.*([0-9]+\.[0-9]+)' "$file" 2>/dev/null | grep -oP '[0-9]+\.[0-9]+' \ - || echo "0.0" + awk ' + /\*\*TOTAL\*\*/ || /Verdict:/ { + if (match($0, /[0-9]+[.][0-9]+/)) { + print substr($0, RSTART, RLENGTH) + found = 1 + exit + } + } + END { + if (!found) print "0.0" + } + ' "$file" 2>/dev/null } score_passes() { @@ -241,8 +248,12 @@ done phase "PHASE 3: Build Report" -FINAL_SCORE="${SCORES[-1]:-0.0}" NUM_ITERATIONS=${#SCORES[@]} +if [ "$NUM_ITERATIONS" -gt 0 ]; then + FINAL_SCORE="${SCORES[$((NUM_ITERATIONS - 1))]}" +else + FINAL_SCORE="0.0" +fi ELAPSED=$(elapsed) # Build score progression table diff --git a/tests/gan-harness.test.js b/tests/gan-harness.test.js new file mode 100644 index 000000000..74bbcb239 --- /dev/null +++ b/tests/gan-harness.test.js @@ -0,0 +1,74 @@ +/** + * Regression tests for the standalone GAN harness helpers. + */ + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const repoRoot = path.resolve(__dirname, '..'); +const harnessPath = path.join(repoRoot, 'scripts', 'gan-harness.sh'); +const harnessSource = fs.readFileSync(harnessPath, 'utf8'); + +let passed = 0; +let failed = 0; + +function test(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + passed += 1; + } catch (error) { + console.log(` ✗ ${name}`); + console.log(` Error: ${error.message}`); + failed += 1; + } +} + +function extractScore(feedback) { + const functionMatch = harnessSource.match(/extract_score\(\) \{[\s\S]*?\n\}/); + assert.ok(functionMatch, 'expected scripts/gan-harness.sh to define extract_score'); + + const temporaryDirectory = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-gan-harness-')); + const feedbackPath = path.join(temporaryDirectory, 'feedback.md'); + fs.writeFileSync(feedbackPath, feedback, 'utf8'); + + try { + const result = spawnSync( + '/bin/bash', + ['-c', `${functionMatch[0]}\nextract_score "$1"`, 'gan-harness-score-test', feedbackPath], + { encoding: 'utf8' } + ); + assert.strictEqual(result.status, 0, result.stderr || 'extract_score failed'); + return result.stdout.trim(); + } finally { + fs.rmSync(temporaryDirectory, { recursive: true, force: true }); + } +} + +console.log('\n=== GAN harness helpers ===\n'); + +test('extract_score reads the documented TOTAL table format', () => { + assert.strictEqual(extractScore('| **TOTAL** | | | **7.5** |\n'), '7.5'); +}); + +test('extract_score reads the compact TOTAL format', () => { + assert.strictEqual(extractScore('**TOTAL** | **8.3**\n'), '8.3'); +}); + +test('extract_score reads a Verdict score', () => { + assert.strictEqual(extractScore('Verdict: PASS with score 9.1\n'), '9.1'); +}); + +test('final score lookup is compatible with the macOS Bash 3.2 runtime', () => { + assert.ok(!harnessSource.includes('SCORES[-1]'), 'negative array subscripts require Bash 4.3+'); +}); + +console.log(`\nPassed: ${passed}`); +console.log(`Failed: ${failed}`); + +process.exit(failed > 0 ? 1 : 0); From 37e9683161cf2de593a576acfe7ddc1bf85c94e4 Mon Sep 17 00:00:00 2001 From: CaoBochun Date: Wed, 5 Aug 2026 15:44:27 +0800 Subject: [PATCH 08/46] test: address GAN harness review feedback --- tests/gan-harness.test.js | 40 ++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/tests/gan-harness.test.js b/tests/gan-harness.test.js index 74bbcb239..a6451a5ae 100644 --- a/tests/gan-harness.test.js +++ b/tests/gan-harness.test.js @@ -14,18 +14,15 @@ const repoRoot = path.resolve(__dirname, '..'); const harnessPath = path.join(repoRoot, 'scripts', 'gan-harness.sh'); const harnessSource = fs.readFileSync(harnessPath, 'utf8'); -let passed = 0; -let failed = 0; - function test(name, fn) { try { fn(); console.log(` ✓ ${name}`); - passed += 1; + return true; } catch (error) { console.log(` ✗ ${name}`); console.log(` Error: ${error.message}`); - failed += 1; + return false; } } @@ -52,21 +49,30 @@ function extractScore(feedback) { console.log('\n=== GAN harness helpers ===\n'); -test('extract_score reads the documented TOTAL table format', () => { - assert.strictEqual(extractScore('| **TOTAL** | | | **7.5** |\n'), '7.5'); -}); +const results = Object.freeze([ + test('extract_score reads the documented TOTAL table format', () => { + assert.strictEqual(extractScore('| **TOTAL** | | | **7.5** |\n'), '7.5'); + }), -test('extract_score reads the compact TOTAL format', () => { - assert.strictEqual(extractScore('**TOTAL** | **8.3**\n'), '8.3'); -}); + test('extract_score reads the compact TOTAL format', () => { + assert.strictEqual(extractScore('**TOTAL** | **8.3**\n'), '8.3'); + }), -test('extract_score reads a Verdict score', () => { - assert.strictEqual(extractScore('Verdict: PASS with score 9.1\n'), '9.1'); -}); + test('extract_score reads a Verdict score', () => { + assert.strictEqual(extractScore('Verdict: PASS with score 9.1\n'), '9.1'); + }), -test('final score lookup is compatible with the macOS Bash 3.2 runtime', () => { - assert.ok(!harnessSource.includes('SCORES[-1]'), 'negative array subscripts require Bash 4.3+'); -}); + test('extract_score returns the fallback when no supported score exists', () => { + assert.strictEqual(extractScore('Other score: 9.9\n'), '0.0'); + }), + + test('final score lookup is compatible with the macOS Bash 3.2 runtime', () => { + assert.ok(!harnessSource.includes('SCORES[-1]'), 'negative array subscripts require Bash 4.3+'); + }), +]); + +const passed = results.filter(Boolean).length; +const failed = results.length - passed; console.log(`\nPassed: ${passed}`); console.log(`Failed: ${failed}`); From 8a396ef54276950df6d39ad353807144ef09c0b1 Mon Sep 17 00:00:00 2001 From: CaoBochun Date: Wed, 5 Aug 2026 15:54:54 +0800 Subject: [PATCH 09/46] test: strengthen GAN harness assertions --- tests/gan-harness.test.js | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/gan-harness.test.js b/tests/gan-harness.test.js index a6451a5ae..3be72ca6b 100644 --- a/tests/gan-harness.test.js +++ b/tests/gan-harness.test.js @@ -51,23 +51,39 @@ console.log('\n=== GAN harness helpers ===\n'); const results = Object.freeze([ test('extract_score reads the documented TOTAL table format', () => { - assert.strictEqual(extractScore('| **TOTAL** | | | **7.5** |\n'), '7.5'); + const feedback = '| **TOTAL** | | | **7.5** |\n'; + const result = extractScore(feedback); + + assert.strictEqual(result, '7.5'); }), test('extract_score reads the compact TOTAL format', () => { - assert.strictEqual(extractScore('**TOTAL** | **8.3**\n'), '8.3'); + const feedback = '**TOTAL** | **8.3**\n'; + const result = extractScore(feedback); + + assert.strictEqual(result, '8.3'); }), test('extract_score reads a Verdict score', () => { - assert.strictEqual(extractScore('Verdict: PASS with score 9.1\n'), '9.1'); + const feedback = 'Verdict: PASS with score 9.1\n'; + const result = extractScore(feedback); + + assert.strictEqual(result, '9.1'); }), test('extract_score returns the fallback when no supported score exists', () => { - assert.strictEqual(extractScore('Other score: 9.9\n'), '0.0'); + const feedback = 'Other score: 9.9\n'; + const result = extractScore(feedback); + + assert.strictEqual(result, '0.0'); }), test('final score lookup is compatible with the macOS Bash 3.2 runtime', () => { - assert.ok(!harnessSource.includes('SCORES[-1]'), 'negative array subscripts require Bash 4.3+'); + assert.doesNotMatch( + harnessSource, + /\bSCORES\[\s*-\s*\d+\s*\]/, + 'negative array subscripts require Bash 4.3+' + ); }), ]); From 91e846dfe2e4455395fca14a5b5c3f119c55a5f2 Mon Sep 17 00:00:00 2001 From: CaoBochun Date: Wed, 5 Aug 2026 16:06:47 +0800 Subject: [PATCH 10/46] test: execute final GAN score selection --- tests/gan-harness.test.js | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/tests/gan-harness.test.js b/tests/gan-harness.test.js index 3be72ca6b..5118ffa5d 100644 --- a/tests/gan-harness.test.js +++ b/tests/gan-harness.test.js @@ -26,6 +26,14 @@ function test(name, fn) { } } +function runHarnessScript(script, args = []) { + const result = spawnSync('/bin/bash', ['-c', script, 'gan-harness-test', ...args], { + encoding: 'utf8', + }); + assert.strictEqual(result.status, 0, result.stderr || 'GAN harness script failed'); + return result.stdout.trim(); +} + function extractScore(feedback) { const functionMatch = harnessSource.match(/extract_score\(\) \{[\s\S]*?\n\}/); assert.ok(functionMatch, 'expected scripts/gan-harness.sh to define extract_score'); @@ -35,13 +43,7 @@ function extractScore(feedback) { fs.writeFileSync(feedbackPath, feedback, 'utf8'); try { - const result = spawnSync( - '/bin/bash', - ['-c', `${functionMatch[0]}\nextract_score "$1"`, 'gan-harness-score-test', feedbackPath], - { encoding: 'utf8' } - ); - assert.strictEqual(result.status, 0, result.stderr || 'extract_score failed'); - return result.stdout.trim(); + return runHarnessScript(`${functionMatch[0]}\nextract_score "$1"`, [feedbackPath]); } finally { fs.rmSync(temporaryDirectory, { recursive: true, force: true }); } @@ -79,11 +81,25 @@ const results = Object.freeze([ }), test('final score lookup is compatible with the macOS Bash 3.2 runtime', () => { + const finalScoreBlock = harnessSource.match( + /NUM_ITERATIONS=\$\{#SCORES\[@\]\}\nif \[ "\$NUM_ITERATIONS"[\s\S]*?\nfi/ + ); + const scoreOutput = harnessSource.match(/echo -e "\s{2}Score:[^\n]+/); + + assert.ok(finalScoreBlock, 'expected scripts/gan-harness.sh to select a final score'); + assert.ok(scoreOutput, 'expected scripts/gan-harness.sh to print the final score'); assert.doesNotMatch( harnessSource, /\bSCORES\[\s*-\s*\d+\s*\]/, 'negative array subscripts require Bash 4.3+' ); + + const output = runHarnessScript( + [`SCORES=("$@")`, 'CYAN=""', 'NC=""', finalScoreBlock[0], scoreOutput[0]].join('\n'), + ['6.2', '8.7'] + ); + + assert.match(output, /Score:\s+8\.7\s+\/\s+10\.0/); }), ]); From cce8f602065394d9da9192402e9d6aac599a4e14 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:16:18 -0400 Subject: [PATCH 11/46] test(gan): use portable Bash lookup on Windows --- tests/gan-harness.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/gan-harness.test.js b/tests/gan-harness.test.js index 5118ffa5d..d2c404a47 100644 --- a/tests/gan-harness.test.js +++ b/tests/gan-harness.test.js @@ -27,7 +27,8 @@ function test(name, fn) { } function runHarnessScript(script, args = []) { - const result = spawnSync('/bin/bash', ['-c', script, 'gan-harness-test', ...args], { + const bashExecutable = process.platform === 'win32' ? 'bash' : '/bin/bash'; + const result = spawnSync(bashExecutable, ['-c', script, 'gan-harness-test', ...args], { encoding: 'utf8', }); assert.strictEqual(result.status, 0, result.stderr || 'GAN harness script failed'); From fcef85cb87dd6ab53b40221d02bb7e79db6f88ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=B9=E5=8D=9A=E6=B7=B3?= Date: Wed, 12 Aug 2026 12:22:07 +0800 Subject: [PATCH 12/46] test: skip GAN shell checks on Windows --- tests/gan-harness.test.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/gan-harness.test.js b/tests/gan-harness.test.js index d2c404a47..4c42111fc 100644 --- a/tests/gan-harness.test.js +++ b/tests/gan-harness.test.js @@ -14,6 +14,14 @@ const repoRoot = path.resolve(__dirname, '..'); const harnessPath = path.join(repoRoot, 'scripts', 'gan-harness.sh'); const harnessSource = fs.readFileSync(harnessPath, 'utf8'); +if (process.platform === 'win32') { + console.log('\n=== GAN harness helpers ===\n'); + console.log(' - skipped on Windows; GAN harness shell helpers are Unix-only'); + console.log('\nPassed: 0'); + console.log('Failed: 0'); + process.exit(0); +} + function test(name, fn) { try { fn(); From b0cd531a3ed406ebccf4ae399c3af83e047543bf Mon Sep 17 00:00:00 2001 From: van3hardy Date: Mon, 10 Aug 2026 23:13:50 -0400 Subject: [PATCH 13/46] fix(plugin): export only plugin function for opencode loader compatibility opencode's legacy plugin loader (getLegacyPlugins) iterates every module export and throws 'Plugin export is not a function' if any export is not a plugin function. The bundle exported VERSION (string) and metadata (object) alongside the plugin, breaking plugin loading. Export only the plugin function so opencode can load ecc-universal. --- .opencode/index.ts | 46 +++------------------------------------------- 1 file changed, 3 insertions(+), 43 deletions(-) diff --git a/.opencode/index.ts b/.opencode/index.ts index 9bb5bf0cb..fa6cadc58 100644 --- a/.opencode/index.ts +++ b/.opencode/index.ts @@ -35,46 +35,6 @@ */ // Export the main plugin -export { ECCHooksPlugin, default } from "./plugins/index.js" - -// Export individual components for selective use -export * from "./plugins/index.js" - -// Version export -export const VERSION = "1.6.0" - -// Plugin metadata -export const metadata = { - name: "ecc-universal", - version: VERSION, - description: "ECC plugin for OpenCode", - author: "affaan-m", - features: { - agents: 13, - commands: 31, - skills: 37, - configAssets: true, - hookEvents: [ - "file.edited", - "tool.execute.before", - "tool.execute.after", - "session.created", - "session.idle", - "session.deleted", - "file.watcher.updated", - "permission.ask", - "todo.updated", - "shell.env", - "experimental.session.compacting", - ], - customTools: [ - "run-tests", - "check-coverage", - "security-audit", - "format-code", - "lint-check", - "git-summary", - "changed-files", - ], - }, -} +// opencode's legacy plugin loader iterates every module export and throws if +// any is not a plugin function, so only the plugin function may be exported. +export { default } from "./plugins/index.js" From f0684fda32cd3f7813d4eff8f66638e3f2cb6a20 Mon Sep 17 00:00:00 2001 From: van3hardy Date: Mon, 10 Aug 2026 23:56:33 -0400 Subject: [PATCH 14/46] test(opencode): assert built entry exports only the plugin function --- tests/scripts/build-opencode.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/scripts/build-opencode.test.js b/tests/scripts/build-opencode.test.js index f3f973ca9..abc55a275 100644 --- a/tests/scripts/build-opencode.test.js +++ b/tests/scripts/build-opencode.test.js @@ -46,6 +46,25 @@ function main() { assert.strictEqual(result.status, 0, result.stderr) assert.ok(fs.existsSync(distEntry), ".opencode/dist/index.js should exist after build") }], + ["built OpenCode entry exports only the plugin function", () => { + const check = ` + const assert = require("assert") + const { pathToFileURL } = require("url") + const file = process.argv[1] + import(pathToFileURL(file).href).then((mod) => { + assert.deepStrictEqual(Object.keys(mod).sort(), ["default"]) + assert.strictEqual(typeof mod.default, "function") + }).catch((error) => { + console.error(error) + process.exit(1) + }) + ` + const result = spawnSync(process.execPath, ["-e", check, distEntry], { + cwd: repoRoot, + encoding: "utf8", + }) + assert.strictEqual(result.status, 0, result.stderr) + }], ["npm pack includes the compiled OpenCode dist payload", () => { const result = spawnSync("npm", ["pack", "--dry-run", "--json"], { cwd: repoRoot, From f932e63b0a317b43a55f344e6f0a0498bcf3794b Mon Sep 17 00:00:00 2001 From: van3hardy Date: Tue, 11 Aug 2026 22:03:01 -0400 Subject: [PATCH 15/46] test(opencode): assert built entry is a working plugin, not just an export shape --- tests/scripts/build-opencode.test.js | 39 +++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/tests/scripts/build-opencode.test.js b/tests/scripts/build-opencode.test.js index abc55a275..b05fa8d1c 100644 --- a/tests/scripts/build-opencode.test.js +++ b/tests/scripts/build-opencode.test.js @@ -50,11 +50,44 @@ function main() { const check = ` const assert = require("assert") const { pathToFileURL } = require("url") - const file = process.argv[1] - import(pathToFileURL(file).href).then((mod) => { + + async function main() { + let mod + try { + mod = await import(pathToFileURL(process.argv[1]).href) + } catch (error) { + console.error(error) + process.exit(1) + } assert.deepStrictEqual(Object.keys(mod).sort(), ["default"]) assert.strictEqual(typeof mod.default, "function") - }).catch((error) => { + + const plugin = await mod.default({ + client: { app: { log: () => {} } }, + $: async () => { throw new Error("$ must not be called during plugin init") }, + directory: process.cwd(), + worktree: process.cwd(), + }) + assert.ok(plugin && typeof plugin === "object", "default export must return a plugin record") + const expectedHooks = [ + "file.edited", + "tool.execute.after", + "tool.execute.before", + "session.created", + "session.idle", + "session.deleted", + "file.watcher.updated", + "todo.updated", + "shell.env", + "experimental.session.compacting", + "permission.ask", + ] + for (const hook of expectedHooks) { + assert.strictEqual(typeof plugin[hook], "function", "missing hook: " + hook) + } + } + + main().catch((error) => { console.error(error) process.exit(1) }) From e72f1e68248167518564f37a7a4f0fd8aae1f9c6 Mon Sep 17 00:00:00 2001 From: van3hardy Date: Tue, 11 Aug 2026 22:09:32 -0400 Subject: [PATCH 16/46] test(opencode): assert init does not call shell and root plugin tools --- tests/scripts/build-opencode.test.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/scripts/build-opencode.test.js b/tests/scripts/build-opencode.test.js index b05fa8d1c..4a3aac797 100644 --- a/tests/scripts/build-opencode.test.js +++ b/tests/scripts/build-opencode.test.js @@ -62,12 +62,17 @@ function main() { assert.deepStrictEqual(Object.keys(mod).sort(), ["default"]) assert.strictEqual(typeof mod.default, "function") + let shellCalls = 0 const plugin = await mod.default({ client: { app: { log: () => {} } }, - $: async () => { throw new Error("$ must not be called during plugin init") }, + $: async () => { + shellCalls += 1 + throw new Error("$ must not be called during plugin init") + }, directory: process.cwd(), worktree: process.cwd(), }) + assert.strictEqual(shellCalls, 0, "$ must not be called during plugin init") assert.ok(plugin && typeof plugin === "object", "default export must return a plugin record") const expectedHooks = [ "file.edited", @@ -85,6 +90,18 @@ function main() { for (const hook of expectedHooks) { assert.strictEqual(typeof plugin[hook], "function", "missing hook: " + hook) } + assert.deepStrictEqual( + Object.keys(plugin.tool).sort(), + ["changed-files", "dependency-analyzer"], + "plugin.tool must expose exactly the custom tools" + ) + for (const toolName of ["changed-files", "dependency-analyzer"]) { + const toolDefinition = plugin.tool[toolName] + assert.ok(toolDefinition && typeof toolDefinition === "object", "missing tool: " + toolName) + assert.strictEqual(typeof toolDefinition.description, "string", toolName + " must declare a description") + assert.ok(toolDefinition.args && typeof toolDefinition.args === "object", toolName + " must declare args") + assert.strictEqual(typeof toolDefinition.execute, "function", toolName + " must declare an execute function") + } } main().catch((error) => { From b2a8091440f8b9b4a3dcc80f3850a03f937db941 Mon Sep 17 00:00:00 2001 From: van3hardy Date: Tue, 11 Aug 2026 22:12:35 -0400 Subject: [PATCH 17/46] test(opencode): guard plugin.tool existence before shape assertion --- tests/scripts/build-opencode.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scripts/build-opencode.test.js b/tests/scripts/build-opencode.test.js index 4a3aac797..469165883 100644 --- a/tests/scripts/build-opencode.test.js +++ b/tests/scripts/build-opencode.test.js @@ -90,6 +90,7 @@ function main() { for (const hook of expectedHooks) { assert.strictEqual(typeof plugin[hook], "function", "missing hook: " + hook) } + assert.ok(plugin.tool && typeof plugin.tool === "object", "plugin record must expose a tool object") assert.deepStrictEqual( Object.keys(plugin.tool).sort(), ["changed-files", "dependency-analyzer"], From 4c7e965209842cd66d73b956afa3cb02f0514b94 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:24:16 -0400 Subject: [PATCH 18/46] fix(gan): distinguish scores from verdict thresholds --- scripts/gan-harness.sh | 23 +++++++++++++++++++---- tests/gan-harness.test.js | 17 +++++++++++++++++ tests/scripts/codex-hooks.test.js | 26 +++++++++++++++++++++----- 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/scripts/gan-harness.sh b/scripts/gan-harness.sh index 093e696d1..79dd5038f 100755 --- a/scripts/gan-harness.sh +++ b/scripts/gan-harness.sh @@ -62,15 +62,30 @@ extract_score() { # Extract the TOTAL weighted score from a feedback file local file="$1" awk ' - /\*\*TOTAL\*\*/ || /Verdict:/ { - if (match($0, /[0-9]+[.][0-9]+/)) { - print substr($0, RSTART, RLENGTH) + /\*\*TOTAL\*\*/ { + total_line = $0 + total = "" + while (match(total_line, /[0-9]+[.][0-9]+/)) { + total = substr(total_line, RSTART, RLENGTH) + total_line = substr(total_line, RSTART + RLENGTH) + } + if (total != "") { + print total found = 1 exit } } + /Verdict:/ && /[Ss]core[[:space:]]*[:=]?[[:space:]]*[0-9]+[.][0-9]+/ { + verdict = $0 + sub(/^.*[Ss]core[[:space:]]*[:=]?[[:space:]]*/, "", verdict) + if (match(verdict, /^[0-9]+[.][0-9]+/)) { + verdict = substr(verdict, RSTART, RLENGTH) + } else { + verdict = "" + } + } END { - if (!found) print "0.0" + if (!found) print (verdict != "" ? verdict : "0.0") } ' "$file" 2>/dev/null } diff --git a/tests/gan-harness.test.js b/tests/gan-harness.test.js index 4c42111fc..36c7255ce 100644 --- a/tests/gan-harness.test.js +++ b/tests/gan-harness.test.js @@ -82,6 +82,23 @@ const results = Object.freeze([ assert.strictEqual(result, '9.1'); }), + test('extract_score does not treat a Verdict threshold as a score', () => { + const feedback = '## Verdict: PASS / FAIL (threshold: 7.0)\n'; + const result = extractScore(feedback); + + assert.strictEqual(result, '0.0'); + }), + + test('extract_score prefers a TOTAL score after a Verdict threshold', () => { + const feedback = [ + '## Verdict: PASS / FAIL (threshold: 7.0)', + '| **TOTAL** | **1.0** | **9.0** |', + ].join('\n'); + const result = extractScore(feedback); + + assert.strictEqual(result, '9.0'); + }), + test('extract_score returns the fallback when no supported score exists', () => { const feedback = 'Other score: 9.9\n'; const result = extractScore(feedback); diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 628a37da7..353c64efe 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -43,15 +43,20 @@ function cleanup(dirPath) { fs.rmSync(dirPath, { recursive: true, force: true }); } +function resolveBashExecutable(env = process.env) { + return env.BASH_PATH + || (process.platform === 'win32' && fs.existsSync('C:\\Program Files\\Git\\bin\\bash.exe') + ? 'C:\\Program Files\\Git\\bin\\bash.exe' + : fs.existsSync('/bin/bash') + ? '/bin/bash' + : 'bash'); +} + function runBash( scriptPath, { args = [], env = {}, cwd = repoRoot, input = undefined, preservePath = true } = {}, ) { - const bash = process.platform === 'win32' && fs.existsSync('C:\\Program Files\\Git\\bin\\bash.exe') - ? 'C:\\Program Files\\Git\\bin\\bash.exe' - : fs.existsSync('/bin/bash') - ? '/bin/bash' - : 'bash'; + const bash = resolveBashExecutable(); return spawnSync(bash, [scriptPath, ...args], { cwd, env: { @@ -132,6 +137,17 @@ const cacheManifestWithLocalRefs = { let passed = 0; let failed = 0; +if ( + test('shell test runner honors an explicit BASH_PATH override', () => { + assert.strictEqual( + resolveBashExecutable({ BASH_PATH: '/custom/git/bin/bash' }), + '/custom/git/bin/bash', + ); + }) +) + passed++; +else failed++; + function runHermeticPrePush({ failScript = null, includeCorepack = true, From e82e47703486f09d2798a52ae82514dd25af8946 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:53:54 -0400 Subject: [PATCH 19/46] test(pack): tolerate slow Windows extraction --- tests/scripts/ecc-universal-bin.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/scripts/ecc-universal-bin.test.js b/tests/scripts/ecc-universal-bin.test.js index 5cb6dba1d..c4336d26a 100644 --- a/tests/scripts/ecc-universal-bin.test.js +++ b/tests/scripts/ecc-universal-bin.test.js @@ -32,6 +32,7 @@ const windowsPackageCommands = new Set([ ]); const unsafeWindowsShellChars = /[\r\n"&|<>^%!()]/; const commandTimeoutMs = 90_000; +const archiveExtractionTimeoutMs = 180_000; let passed = 0; let failed = 0; @@ -96,7 +97,7 @@ function run(command, args, options = {}) { env: options.env || process.env, maxBuffer: 10 * 1024 * 1024, shell: invocation.shell || false, - timeout: commandTimeoutMs, + timeout: options.timeout ?? commandTimeoutMs, windowsHide: true, }); @@ -171,6 +172,7 @@ function prepareLocalPackedProject(packageManager) { fs.mkdirSync(modulesDirectory, { recursive: true }); run('tar', ['-xzf', fixture.archivePath, '-C', modulesDirectory], { cwd: projectDirectory, + timeout: archiveExtractionTimeoutMs, }); fs.renameSync(extractedDirectory, packageDirectory); fs.mkdirSync(binDirectory, { recursive: true }); From ecdd517765bda149c8b2f95131b1067be5bb6d22 Mon Sep 17 00:00:00 2001 From: Tanel Date: Mon, 27 Jul 2026 22:28:23 +0300 Subject: [PATCH 20/46] fix(suggest-compact): don't quote a percentage against an assumed window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The context signal always rendered "N% of window", including when the window size was the assumed 200k default rather than a detected value. On a 1M session whose transcript carries no [1m] marker, that produced lines like: [StrategicCompact] Context ~194k tokens (97% of 200k window) while actual usage was ~19%. The user compacts on a false alarm, loses context, and the resulting quality drop reads as a model regression. The gap is structural: the context threshold defaults to 80% of the window (160k on 200k), so the signal fires precisely in the 160k-200k band where the size cannot be determined — above 200k the observed-tokens fallback correctly infers 1M, and below 160k nothing fires. Model id alone cannot close this. A tier may ship both a 200k and a 1M variant under one id, so neither the known-family table nor a new entry can distinguish them, and the transcript records no window field. So stop asserting what isn't known: resolveContextWindow() now reports whether the size was detected (env override, [1m] marker, known family, or observed tokens > 200k) or assumed, and the hook omits the percentage and window label when it was assumed. The token count, threshold, and firing behaviour are unchanged. resolveContextWindowTokens() keeps its existing signature and semantics. Note: 3 pre-existing failures in tests/hooks/suggest-compact.test.js reproduce identically on unmodified main and are untouched here. --- scripts/hooks/suggest-compact.js | 12 +++++-- scripts/lib/transcript-context.js | 51 ++++++++++++++++++++++------ tests/lib/transcript-context.test.js | 34 ++++++++++++++++++- 3 files changed, 82 insertions(+), 15 deletions(-) diff --git a/scripts/hooks/suggest-compact.js b/scripts/hooks/suggest-compact.js index 2a104df3a..b8a163e9b 100644 --- a/scripts/hooks/suggest-compact.js +++ b/scripts/hooks/suggest-compact.js @@ -38,7 +38,8 @@ const { resolveContextThreshold, resolveContextInterval, computeContextBucket, - formatWindowLabel + formatWindowLabel, + isContextWindowInferred } = require('../lib/transcript-context'); const COUNTER_FILE_PREFIX = 'claude-tool-count-'; @@ -185,8 +186,13 @@ function buildContextSuggestion(transcriptPath, bucketFile, env) { writeFile(bucketFile, String(bucket)); const approxTokens = `${Math.round(usage.tokens / 1000)}k`; - const percent = Math.round((usage.tokens / windowTokens) * 100); - return `[StrategicCompact] Context ~${approxTokens} tokens (${percent}% of ${formatWindowLabel(windowTokens)} window) - consider /compact at the next logical boundary`; + // Only quote a percentage when the window size was actually detected. + // Against an assumed 200k default the denominator is a guess, and a + // "97% of 200k window" line on a 1M session triggers needless compaction. + const scale = isContextWindowInferred(usage.tokens, usage.model) + ? '' + : ` (${Math.round((usage.tokens / windowTokens) * 100)}% of ${formatWindowLabel(windowTokens)} window)`; + return `[StrategicCompact] Context ~${approxTokens} tokens${scale} - consider /compact at the next logical boundary`; } catch (err) { log(`[StrategicCompact] Context signal skipped: ${err.message}`); return null; diff --git a/scripts/lib/transcript-context.js b/scripts/lib/transcript-context.js index 201861487..d0a944330 100644 --- a/scripts/lib/transcript-context.js +++ b/scripts/lib/transcript-context.js @@ -158,24 +158,29 @@ function readLatestContextTokens(transcriptPath, options = {}) { } /** - * Detect the context window size for a turn. - * 1M when the model id carries the `[1m]` marker, matches a known large-window - * model family, or when the observed token count already exceeds the standard - * 200k window (covers logs that drop the suffix); otherwise the standard 200k - * window. + * Detect the context window size for a turn, and report whether that size was + * positively detected or merely assumed. + * + * `inferred: false` means the size came from evidence — an explicit env + * override, the `[1m]` marker, a known large-window family, or an observed + * token count that already exceeds the standard window. `inferred: true` means + * every check fell through and the standard 200k default was assumed; the + * window may actually be larger and callers must not present it as fact. + * + * @returns {{ windowTokens: number, inferred: boolean }} */ -function resolveContextWindowTokens(tokens, model) { +function resolveContextWindow(tokens, model) { // Explicit window override wins: 400k models (e.g. Opus 4.x) match neither the // 200k default nor the 1M marker and would otherwise report ~double usage (#2290). // Honor ECC's own knob and Claude Code's native CLAUDE_CODE_AUTO_COMPACT_WINDOW. const env = (typeof process !== 'undefined' && process.env) || {}; const envWindow = Number.parseInt(env.ECC_CONTEXT_WINDOW_TOKENS || env.CLAUDE_CODE_AUTO_COMPACT_WINDOW || '', 10); if (Number.isInteger(envWindow) && envWindow > 0) { - return envWindow; + return { windowTokens: envWindow, inferred: false }; } if (typeof model === 'string' && model.includes(LARGE_WINDOW_MODEL_MARKER)) { - return LARGE_CONTEXT_WINDOW_TOKENS; + return { windowTokens: LARGE_CONTEXT_WINDOW_TOKENS, inferred: false }; } // Large-window model families without a [1m] marker fall through the checks @@ -183,15 +188,37 @@ function resolveContextWindowTokens(tokens, model) { if (typeof model === 'string') { const known = KNOWN_MODEL_WINDOW_TOKENS.find(([familyId]) => isKnownModelFamilyMatch(model, familyId)); if (known) { - return known[1]; + return { windowTokens: known[1], inferred: false }; } } if (Number.isFinite(tokens) && tokens > STANDARD_CONTEXT_WINDOW_TOKENS) { - return LARGE_CONTEXT_WINDOW_TOKENS; + return { windowTokens: LARGE_CONTEXT_WINDOW_TOKENS, inferred: false }; } - return STANDARD_CONTEXT_WINDOW_TOKENS; + return { windowTokens: STANDARD_CONTEXT_WINDOW_TOKENS, inferred: true }; +} + +/** + * Detect the context window size for a turn. + * 1M when the model id carries the `[1m]` marker, matches a known large-window + * model family, or when the observed token count already exceeds the standard + * 200k window (covers logs that drop the suffix); otherwise the standard 200k + * window. + */ +function resolveContextWindowTokens(tokens, model) { + return resolveContextWindow(tokens, model).windowTokens; +} + +/** + * True when the resolved window is the assumed 200k default rather than a + * detected size. Opt-in large-window models that ship no `[1m]` marker in the + * transcript (e.g. a 1M-context Opus tier, where the base tier is 200k and the + * two are indistinguishable by model id) land here, so a percentage computed + * against 200k can be wildly wrong while usage sits below that mark. + */ +function isContextWindowInferred(tokens, model) { + return resolveContextWindow(tokens, model).inferred; } /** @@ -254,7 +281,9 @@ module.exports = { DEFAULT_CONTEXT_INTERVAL_TOKENS, DEFAULT_TRANSCRIPT_TAIL_BYTES, readLatestContextTokens, + resolveContextWindow, resolveContextWindowTokens, + isContextWindowInferred, resolveContextThreshold, resolveContextInterval, computeContextBucket, diff --git a/tests/lib/transcript-context.test.js b/tests/lib/transcript-context.test.js index 1d335f131..f10f62c76 100644 --- a/tests/lib/transcript-context.test.js +++ b/tests/lib/transcript-context.test.js @@ -23,7 +23,8 @@ const { resolveContextThreshold, resolveContextInterval, computeContextBucket, - formatWindowLabel + formatWindowLabel, + isContextWindowInferred } = require('../../scripts/lib/transcript-context'); console.log('=== Testing transcript-context.js ===\n'); @@ -218,6 +219,37 @@ test('treats an empty model id as standard window', () => { assert.strictEqual(resolveContextWindowTokens(100000, ''), STANDARD_CONTEXT_WINDOW_TOKENS); }); +// ── isContextWindowInferred ── +console.log('\nisContextWindowInferred:'); + +delete process.env.ECC_CONTEXT_WINDOW_TOKENS; +delete process.env.CLAUDE_CODE_AUTO_COMPACT_WINDOW; + +test('flags the assumed 200k default as inferred', () => { + assert.strictEqual(isContextWindowInferred(187000, 'claude-opus-9'), true); +}); + +test('an env override is a detected window, not inferred', () => { + process.env.ECC_CONTEXT_WINDOW_TOKENS = '1000000'; + try { + assert.strictEqual(isContextWindowInferred(187000, 'claude-opus-9'), false); + } finally { + delete process.env.ECC_CONTEXT_WINDOW_TOKENS; + } +}); + +test('a [1m] marker is a detected window, not inferred', () => { + assert.strictEqual(isContextWindowInferred(187000, 'claude-opus-4-5[1m]'), false); +}); + +test('a known large-window family is a detected window, not inferred', () => { + assert.strictEqual(isContextWindowInferred(187000, 'claude-fable-5'), false); +}); + +test('tokens above the standard window make the size detected, not inferred', () => { + assert.strictEqual(isContextWindowInferred(220000, 'claude-opus-9'), false); +}); + // ── resolveContextThreshold ── console.log('\nresolveContextThreshold:'); From 2f8a5a271dfe2614b08672201c8e04987d7dfd93 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:32:19 -0400 Subject: [PATCH 21/46] test: cover inferred-window hook output --- scripts/hooks/suggest-compact.js | 9 ++++----- tests/hooks/suggest-compact.test.js | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/scripts/hooks/suggest-compact.js b/scripts/hooks/suggest-compact.js index b8a163e9b..dc1a414f0 100644 --- a/scripts/hooks/suggest-compact.js +++ b/scripts/hooks/suggest-compact.js @@ -34,12 +34,11 @@ const { } = require('../lib/utils'); const { readLatestContextTokens, - resolveContextWindowTokens, + resolveContextWindow, resolveContextThreshold, resolveContextInterval, computeContextBucket, - formatWindowLabel, - isContextWindowInferred + formatWindowLabel } = require('../lib/transcript-context'); const COUNTER_FILE_PREFIX = 'claude-tool-count-'; @@ -172,7 +171,7 @@ function buildContextSuggestion(transcriptPath, bucketFile, env) { const usage = readLatestContextTokens(transcriptPath); if (!usage) return null; - const windowTokens = resolveContextWindowTokens(usage.tokens, usage.model); + const { windowTokens, inferred } = resolveContextWindow(usage.tokens, usage.model); const threshold = resolveContextThreshold(env, windowTokens); if (threshold <= 0) return null; // COMPACT_CONTEXT_THRESHOLD=0 disables @@ -189,7 +188,7 @@ function buildContextSuggestion(transcriptPath, bucketFile, env) { // Only quote a percentage when the window size was actually detected. // Against an assumed 200k default the denominator is a guess, and a // "97% of 200k window" line on a 1M session triggers needless compaction. - const scale = isContextWindowInferred(usage.tokens, usage.model) + const scale = inferred ? '' : ` (${Math.round((usage.tokens / windowTokens) * 100)}% of ${formatWindowLabel(windowTokens)} window)`; return `[StrategicCompact] Context ~${approxTokens} tokens${scale} - consider /compact at the next logical boundary`; diff --git a/tests/hooks/suggest-compact.test.js b/tests/hooks/suggest-compact.test.js index 0389f70e5..6036442d3 100644 --- a/tests/hooks/suggest-compact.test.js +++ b/tests/hooks/suggest-compact.test.js @@ -694,7 +694,7 @@ function runTests() { }; } - if (test('suggests compact when context exceeds the 200k-window threshold', () => { + if (test('omits the percentage when the context window is assumed', () => { const ctx = createContextContext(); const transcript = writeTranscriptFixture(170000); try { @@ -704,7 +704,7 @@ function runTests() { const parsed = JSON.parse(result.stdout); const context = parsed.hookSpecificOutput.additionalContext; assert.ok(context.includes('Context ~170k tokens'), `Expected token estimate. Got: ${context}`); - assert.ok(context.includes('85% of 200k window'), `Expected window percentage. Got: ${context}`); + assert.ok(!context.includes('% of'), `Expected no percentage for an assumed window. Got: ${context}`); } finally { try { fs.unlinkSync(transcript); } catch (_err) { /* ignore */ } ctx.cleanup(); From 4377ea1753c4ec6e6de2a3dc9ffcb4b8d1bc73b5 Mon Sep 17 00:00:00 2001 From: Souptik Chakraborty Date: Tue, 28 Jul 2026 23:49:03 +0530 Subject: [PATCH 22/46] docs(gateguard): document the graduated gate controls GateGuard reads five GATEGUARD_* environment variables that were absent from skills/gateguard/SKILL.md, so the only discoverable escape hatch was ECC_GATEGUARD=off - disabling the load-bearing destructive-Bash gate along with the noisy ones (#2573). Documented, with defaults and exact accepted values read from the hook: - GATEGUARD_BASH_ROUTINE_DISABLED (was undocumented everywhere) - GATEGUARD_EXEMPT_GLOBS (previously only in a 2.1.0 release note) - GATEGUARD_BASH_EXTRA_DESTRUCTIVE (was undocumented) - GATEGUARD_DISABLED (was undocumented) - GATEGUARD_STATE_DIR (was undocumented; named in a runtime warning) - GATEGUARD_FACT_FORCE_FULL_DENIALS (already documented; folded into the same table for one lookup point) Adds tests/ci/gateguard-env-documented.test.js, which asserts every GATEGUARD_* variable the hook reads appears in the skill doc, and that the doc names no variable the hook has stopped reading. That surface test is what found the three knobs beyond the two the issue reported. Docs and test only; no hook behaviour changes. Refs #2573 --- skills/gateguard/SKILL.md | 32 +++++++++ tests/ci/gateguard-env-documented.test.js | 88 +++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 tests/ci/gateguard-env-documented.test.js diff --git a/skills/gateguard/SKILL.md b/skills/gateguard/SKILL.md index 9a4bb0314..f244fa667 100644 --- a/skills/gateguard/SKILL.md +++ b/skills/gateguard/SKILL.md @@ -106,6 +106,38 @@ near-identical blocks cannot accumulate in the context window and amplify model repetition loops (#2142). Retrying the same file or command after presenting facts never re-triggers the gate. +#### Graduated controls + +`ECC_GATEGUARD=off` disables the whole gate. The variables below narrow it +instead, so the load-bearing destructive-Bash checks keep running: + +| Variable | Default | Effect | +|---|---|---| +| `GATEGUARD_BASH_ROUTINE_DISABLED` | unset (gate on) | Disables the **routine-Bash** gate only. The destructive-Bash gate (`rm -rf`, `git reset --hard`, `drop table`, `dd if=`, …) is unaffected. | +| `GATEGUARD_EXEMPT_GLOBS` | unset (no exemptions) | Comma-separated globs; a matching Edit/Write/MultiEdit target skips first-touch fact-forcing. Intended for low-import-value trees (tests, generated artifacts, scratch dirs) where "who imports this / what schema" carries no signal. | +| `GATEGUARD_FACT_FORCE_FULL_DENIALS` | `3` | How many denials emit the full four-fact block before later ones condense to a single line. `0` condenses from the very first denial. | +| `GATEGUARD_BASH_EXTRA_DESTRUCTIVE` | unset | Extra destructive-command patterns, as regex source, added to the built-in set. A malformed regex is treated as unset (built-ins still apply) and logged once to stderr. | +| `GATEGUARD_DISABLED` | unset | `1` disables the gate entirely — equivalent to `ECC_GATEGUARD=off`. | +| `GATEGUARD_STATE_DIR` | `~/.gateguard` | Where per-session gate state is kept. If state cannot be persisted the gate allows the operation rather than looping, and names this variable in the warning. | + +`GATEGUARD_BASH_ROUTINE_DISABLED` accepts `1`, `true`, `on`, `enabled`, +`enable`, or `yes` (case- and whitespace-insensitive); any other value +leaves the gate on. `GATEGUARD_DISABLED` recognises `1` only. + +`GATEGUARD_EXEMPT_GLOBS` patterns are matched against the normalized +(forward-slash, lowercased) file path: `*` matches within a path segment, +`**` across segments, `?` a single character. Matching is fail-open — a +malformed pattern is dropped rather than raising. + +```json +{ + "env": { + "GATEGUARD_BASH_ROUTINE_DISABLED": "1", + "GATEGUARD_EXEMPT_GLOBS": "**/tests/**,**/*.test.*,**/docs/**,**/dist/**" + } +} +``` + ### Option B: Full package with config ```bash diff --git a/tests/ci/gateguard-env-documented.test.js b/tests/ci/gateguard-env-documented.test.js new file mode 100644 index 000000000..4f6b4744b --- /dev/null +++ b/tests/ci/gateguard-env-documented.test.js @@ -0,0 +1,88 @@ +/** + * Surface test for #2573: every GATEGUARD_* environment variable the hook + * reads must be documented in the GateGuard skill doc. + * + * `GATEGUARD_BASH_ROUTINE_DISABLED` shipped with no documentation at all and + * `GATEGUARD_EXEMPT_GLOBS` was mentioned only in a release note, so operators + * had no discoverable way to narrow the gate short of disabling it outright. + * This pins the surface: adding a knob to the hook without documenting it + * fails here. + * + * Run with: node tests/ci/gateguard-env-documented.test.js + */ + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const repoRoot = path.join(__dirname, '..', '..'); +const hookPath = path.join(repoRoot, 'scripts', 'hooks', 'gateguard-fact-force.js'); +const skillPath = path.join(repoRoot, 'skills', 'gateguard', 'SKILL.md'); + +let passed = 0; +let failed = 0; + +function test(name, fn) { + try { + fn(); + console.log(` \u2713 ${name}`); + return true; + } catch (err) { + console.log(` \u2717 ${name}`); + console.log(` Error: ${err.message}`); + return false; + } +} + +function readGateguardEnvNames(source) { + // process.env.GATEGUARD_X and process.env['GATEGUARD_X'] + const names = new Set(); + const dotted = /process\.env\.(GATEGUARD_[A-Z0-9_]+)/g; + const bracketed = /process\.env\[\s*['"](GATEGUARD_[A-Z0-9_]+)['"]\s*\]/g; + let m; + while ((m = dotted.exec(source)) !== null) names.add(m[1]); + while ((m = bracketed.exec(source)) !== null) names.add(m[1]); + return names; +} + +console.log('\nGateGuard env-var documentation surface\n'); + +if (test('hook and skill doc both exist', () => { + assert.ok(fs.existsSync(hookPath), `missing ${hookPath}`); + assert.ok(fs.existsSync(skillPath), `missing ${skillPath}`); +})) passed++; else failed++; + +const hookSource = fs.existsSync(hookPath) ? fs.readFileSync(hookPath, 'utf8') : ''; +const skillDoc = fs.existsSync(skillPath) ? fs.readFileSync(skillPath, 'utf8') : ''; +const envNames = readGateguardEnvNames(hookSource); + +if (test('hook reads at least one GATEGUARD_* variable', () => { + assert.ok(envNames.size > 0, 'no GATEGUARD_* env reads found - has the hook moved?'); +})) passed++; else failed++; + +if (test('every GATEGUARD_* variable the hook reads is documented', () => { + const undocumented = [...envNames].filter(name => !skillDoc.includes(name)).sort(); + assert.deepStrictEqual( + undocumented, + [], + `undocumented in skills/gateguard/SKILL.md: ${undocumented.join(', ')}` + ); +})) passed++; else failed++; + +if (test('the documented knobs are the ones the hook actually reads', () => { + // Guards the reverse drift: a doc naming a knob the hook no longer reads. + const documented = [...new Set( + (skillDoc.match(/GATEGUARD_[A-Z0-9_]+/g) || []) + )]; + const stale = documented.filter(name => !hookSource.includes(name)).sort(); + assert.deepStrictEqual(stale, [], `documented but unread by the hook: ${stale.join(', ')}`); +})) passed++; else failed++; + +console.log(`\nPassed: ${passed}`); +console.log(`Failed: ${failed}\n`); + +if (failed > 0) { + process.exit(1); +} From c4253805b5c57126a4e27c46996a812e12a96665 Mon Sep 17 00:00:00 2001 From: Souptik Chakraborty Date: Wed, 29 Jul 2026 08:36:12 +0530 Subject: [PATCH 23/46] docs(gateguard): address review - split full-disable, pin glob semantics CodeRabbit review on #2611, all four findings: - GATEGUARD_DISABLED sat in a table introduced as 'these do not disable the gate'. Moved to its own full-disable section with ECC_GATEGUARD, and corrected the accepted values against ECC_DISABLE_VALUES (0/false/off/disabled/disable - the earlier draft would have implied 'no' works, which it does not). - Documented that a leading **/ compiles to .*/ and so needs a preceding separator: verified by reproducing the hook's glob->regex translation, **/tests/** matches /repo/tests/foo.js but not a bare relative tests/foo.js. Docs now say so and the example carries both forms. Matcher behaviour deliberately unchanged - widening it is a behaviour change, not a docs fix. - Reverse-drift check now compares documented names against the parsed env reads instead of hookSource.includes(), so a name surviving only in a comment or error string no longer satisfies it. - readGateguardEnvNames builds one Set from collected matches instead of mutating via Set#add, per the repo's no-in-place-mutation guideline. --- skills/gateguard/SKILL.md | 35 +++++++++++++++++------ tests/ci/gateguard-env-documented.test.js | 21 +++++++------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/skills/gateguard/SKILL.md b/skills/gateguard/SKILL.md index f244fa667..2c37994a7 100644 --- a/skills/gateguard/SKILL.md +++ b/skills/gateguard/SKILL.md @@ -108,8 +108,9 @@ command after presenting facts never re-triggers the gate. #### Graduated controls -`ECC_GATEGUARD=off` disables the whole gate. The variables below narrow it -instead, so the load-bearing destructive-Bash checks keep running: +`ECC_GATEGUARD=off` (or `GATEGUARD_DISABLED=1`) turns the gate off entirely. +The variables in this table do **not** — each narrows one behaviour while the +load-bearing destructive-Bash checks keep running: | Variable | Default | Effect | |---|---|---| @@ -117,23 +118,39 @@ instead, so the load-bearing destructive-Bash checks keep running: | `GATEGUARD_EXEMPT_GLOBS` | unset (no exemptions) | Comma-separated globs; a matching Edit/Write/MultiEdit target skips first-touch fact-forcing. Intended for low-import-value trees (tests, generated artifacts, scratch dirs) where "who imports this / what schema" carries no signal. | | `GATEGUARD_FACT_FORCE_FULL_DENIALS` | `3` | How many denials emit the full four-fact block before later ones condense to a single line. `0` condenses from the very first denial. | | `GATEGUARD_BASH_EXTRA_DESTRUCTIVE` | unset | Extra destructive-command patterns, as regex source, added to the built-in set. A malformed regex is treated as unset (built-ins still apply) and logged once to stderr. | -| `GATEGUARD_DISABLED` | unset | `1` disables the gate entirely — equivalent to `ECC_GATEGUARD=off`. | | `GATEGUARD_STATE_DIR` | `~/.gateguard` | Where per-session gate state is kept. If state cannot be persisted the gate allows the operation rather than looping, and names this variable in the warning. | `GATEGUARD_BASH_ROUTINE_DISABLED` accepts `1`, `true`, `on`, `enabled`, `enable`, or `yes` (case- and whitespace-insensitive); any other value -leaves the gate on. `GATEGUARD_DISABLED` recognises `1` only. +leaves the gate on. -`GATEGUARD_EXEMPT_GLOBS` patterns are matched against the normalized -(forward-slash, lowercased) file path: `*` matches within a path segment, -`**` across segments, `?` a single character. Matching is fail-open — a -malformed pattern is dropped rather than raising. +#### Turning the gate off completely + +| Variable | Effect | +|---|---| +| `ECC_GATEGUARD=off` | Disables GateGuard for the session. Accepts `0`, `false`, `off`, `disabled`, or `disable`. | +| `GATEGUARD_DISABLED=1` | Same effect. Recognises `1` only — the spellings above do **not** apply here. | + +For hook-level control, keep using `ECC_DISABLED_HOOKS` with the GateGuard hook ID. + +#### Glob semantics for `GATEGUARD_EXEMPT_GLOBS` + +Patterns are matched, unanchored, against the target path with backslashes +normalized to `/` and the whole string lowercased — the path exactly as the +hook receives it, which for Claude Code tool payloads is absolute. `*` matches +within a path segment, `**` across segments, `?` a single character. Matching +is fail-open: a malformed pattern is dropped rather than raising. + +Note that a leading `**/` compiles to `.*/`, so it requires at least one +preceding separator: `**/tests/**` exempts `/repo/tests/foo.js` but would not +match a bare relative `tests/foo.js`. Add the separator-free form too if you +pass relative paths: ```json { "env": { "GATEGUARD_BASH_ROUTINE_DISABLED": "1", - "GATEGUARD_EXEMPT_GLOBS": "**/tests/**,**/*.test.*,**/docs/**,**/dist/**" + "GATEGUARD_EXEMPT_GLOBS": "**/tests/**,tests/**,**/*.test.*,**/docs/**,**/dist/**" } } ``` diff --git a/tests/ci/gateguard-env-documented.test.js b/tests/ci/gateguard-env-documented.test.js index 4f6b4744b..6148a9c7e 100644 --- a/tests/ci/gateguard-env-documented.test.js +++ b/tests/ci/gateguard-env-documented.test.js @@ -38,13 +38,12 @@ function test(name, fn) { function readGateguardEnvNames(source) { // process.env.GATEGUARD_X and process.env['GATEGUARD_X'] - const names = new Set(); - const dotted = /process\.env\.(GATEGUARD_[A-Z0-9_]+)/g; - const bracketed = /process\.env\[\s*['"](GATEGUARD_[A-Z0-9_]+)['"]\s*\]/g; - let m; - while ((m = dotted.exec(source)) !== null) names.add(m[1]); - while ((m = bracketed.exec(source)) !== null) names.add(m[1]); - return names; + const dotted = source.match(/process\.env\.GATEGUARD_[A-Z0-9_]+/g) || []; + const bracketed = source.match(/process\.env\[\s*['"]GATEGUARD_[A-Z0-9_]+['"]\s*\]/g) || []; + const names = [...dotted, ...bracketed] + .map(hit => (hit.match(/GATEGUARD_[A-Z0-9_]+/) || [])[0]) + .filter(Boolean); + return new Set(names); } console.log('\nGateGuard env-var documentation surface\n'); @@ -73,10 +72,10 @@ if (test('every GATEGUARD_* variable the hook reads is documented', () => { if (test('the documented knobs are the ones the hook actually reads', () => { // Guards the reverse drift: a doc naming a knob the hook no longer reads. - const documented = [...new Set( - (skillDoc.match(/GATEGUARD_[A-Z0-9_]+/g) || []) - )]; - const stale = documented.filter(name => !hookSource.includes(name)).sort(); + // Compared against the parsed env reads, not raw source — a name surviving + // only in a comment or error string must not satisfy this. + const documented = [...new Set(skillDoc.match(/GATEGUARD_[A-Z0-9_]+/g) || [])]; + const stale = documented.filter(name => !envNames.has(name)).sort(); assert.deepStrictEqual(stale, [], `documented but unread by the hook: ${stale.join(', ')}`); })) passed++; else failed++; From f1521c893760b170ee6fcb9cf352339ae7e8cfb8 Mon Sep 17 00:00:00 2001 From: Souptik Chakraborty Date: Thu, 30 Jul 2026 18:32:41 +0530 Subject: [PATCH 24/46] test(gateguard): read env knobs from code and pin the access convention The documentation surface test scanned the hook's raw source with two regexes. That had two holes, both confirmed against the shipped parser: - a GATEGUARD_* name appearing only in a comment or a string was counted as a real read, and - destructured, aliased and computed reads were invisible, so an undocumented knob added in one of those forms would pass silently. Blank comments, string literals, template-literal text and regex literals before scanning, so only real code contributes. Blanking preserves length, so `process.env[...]` keys are located in the blanked code and read back from the raw source at the same offset. Rather than chase every possible access form with regexes, the supported forms are now enforced: destructuring, aliasing, spreading, enumerating and computed keys fail the guard with instructions to either keep the convention or extend the parser. Six self-checks cover the blanker and the guard, including a regex literal containing a slash. Refs #2573 --- tests/ci/gateguard-env-documented.test.js | 248 +++++++++++++++++++++- 1 file changed, 242 insertions(+), 6 deletions(-) diff --git a/tests/ci/gateguard-env-documented.test.js b/tests/ci/gateguard-env-documented.test.js index 6148a9c7e..2a9da88c7 100644 --- a/tests/ci/gateguard-env-documented.test.js +++ b/tests/ci/gateguard-env-documented.test.js @@ -8,6 +8,18 @@ * This pins the surface: adding a knob to the hook without documenting it * fails here. * + * The env reads are extracted from *code only* — comments, string literals, + * template-literal text and regex literals are blanked out first, so a knob + * named in a comment or an error message is never mistaken for a read. And + * because a regex scanner cannot see every possible access form, the supported + * forms are enforced as a convention rather than assumed: any other way of + * reaching `process.env` fails the guard below with instructions, instead of + * silently letting an undocumented knob through. + * + * Supported (and enforced) read forms: + * process.env.GATEGUARD_X + * process.env['GATEGUARD_X'] // or "GATEGUARD_X" + * * Run with: node tests/ci/gateguard-env-documented.test.js */ @@ -36,14 +48,170 @@ function test(name, fn) { } } +/** A `/` here starts a regex literal, not a division. */ +const REGEX_CAN_FOLLOW = new Set([ + '', '(', ',', '=', ':', '[', '!', '&', '|', '?', '{', '}', ';', '+', '-', '*', '%', '~', '^', '<', '>', +]); + +/** + * Blank out comments and literal text, preserving length and line breaks so + * offsets stay comparable with the raw source. + * + * Code inside a template literal's `${...}` is preserved — it is real code and + * may contain an env read — while the surrounding literal text is blanked. + */ +function blankCommentsAndLiterals(source) { + const out = []; + const emit = (ch) => out.push(ch === '\n' ? '\n' : ' '); + const keep = (ch) => out.push(ch); + + let i = 0; + let prev = ''; + // Stack of open template literals. 0 = in literal text, >=1 = inside `${...}` + // (the number tracks brace nesting within the expression). + const templates = []; + const inTemplateText = () => templates.length > 0 && templates[templates.length - 1] === 0; + + while (i < source.length) { + const ch = source[i]; + const next = source[i + 1]; + + // Template-literal TEXT is handled first: inside it, `//`, quotes and `/` + // are literal characters, not comments, strings or regexes. + if (inTemplateText()) { + if (ch === '\\') { emit(ch); if (i + 1 < source.length) { emit(source[i + 1]); } i += 2; continue; } + if (ch === '`') { templates.pop(); emit(ch); prev = '`'; i += 1; continue; } + if (ch === '$' && next === '{') { + templates[templates.length - 1] = 1; + keep(ch); keep(next); prev = '{'; i += 2; + continue; + } + emit(ch); i += 1; + continue; + } + + if (ch === '/' && next === '/') { + while (i < source.length && source[i] !== '\n') { emit(source[i]); i += 1; } + continue; + } + + if (ch === '/' && next === '*') { + emit(ch); emit(next); i += 2; + while (i < source.length && !(source[i] === '*' && source[i + 1] === '/')) { emit(source[i]); i += 1; } + if (i < source.length) { emit('*'); emit('/'); i += 2; } + continue; + } + + if (ch === '/' && REGEX_CAN_FOLLOW.has(prev)) { + emit(ch); i += 1; + let inClass = false; + while (i < source.length) { + const r = source[i]; + if (r === '\\') { emit(r); if (i + 1 < source.length) { emit(source[i + 1]); } i += 2; continue; } + if (r === '[') { inClass = true; } + else if (r === ']') { inClass = false; } + else if (r === '/' && !inClass) { emit(r); i += 1; break; } + else if (r === '\n') { break; } + emit(r); i += 1; + } + prev = '/'; + continue; + } + + if (ch === '"' || ch === "'") { + const quote = ch; + emit(ch); i += 1; + while (i < source.length) { + const s = source[i]; + if (s === '\\') { emit(s); if (i + 1 < source.length) { emit(source[i + 1]); } i += 2; continue; } + if (s === quote) { emit(s); i += 1; break; } + if (s === '\n') { break; } + emit(s); i += 1; + } + prev = quote; + continue; + } + + if (ch === '`') { + templates.push(0); + emit(ch); i += 1; + continue; + } + + if (templates.length > 0 && ch === '}') { + const depth = templates[templates.length - 1]; + if (depth === 1) { templates[templates.length - 1] = 0; keep(ch); i += 1; prev = '}'; continue; } + if (depth > 1) { templates[templates.length - 1] = depth - 1; } + } + if (templates.length > 0 && ch === '{' && templates[templates.length - 1] >= 1) { + templates[templates.length - 1] += 1; + } + + keep(ch); + if (!/\s/.test(ch)) { prev = ch; } + i += 1; + } + + return out.join(''); +} + +const DOTTED_READ = /process\.env\.(GATEGUARD_[A-Z0-9_]+)/g; +const QUOTED_KEY = /^(['"])(GATEGUARD_[A-Z0-9_]+)\1$/; +const PLAIN_QUOTED_KEY = /^(['"])[A-Za-z0-9_]+\1$/; + +function matchAll(source, pattern) { + return [...source.matchAll(pattern)].map(m => m[1]); +} + +/** + * Keys used in `process.env[...]`, located in code but read from the raw source. + * + * Blanking replaces literal *text* with spaces, which would erase the key + * itself — so the bracket positions are found in the blanked code (proving the + * access is real code, not a comment or a doc string) and the key is then read + * back out of the raw source at the same offset. `blankCommentsAndLiterals` + * preserves length, which is what makes the offsets interchangeable. + */ +function bracketedEnvKeys(source) { + const code = blankCommentsAndLiterals(source); + return [...code.matchAll(/process\.env\s*\[/g)] + .map((m) => { + const at = source.slice(m.index).match(/^process\.env\s*\[\s*([^\]]*?)\s*\]/); + return at ? at[1] : null; + }) + .filter(key => key !== null); +} + +/** GATEGUARD_* env reads present in real code (comments and literals excluded). */ function readGateguardEnvNames(source) { - // process.env.GATEGUARD_X and process.env['GATEGUARD_X'] - const dotted = source.match(/process\.env\.GATEGUARD_[A-Z0-9_]+/g) || []; - const bracketed = source.match(/process\.env\[\s*['"]GATEGUARD_[A-Z0-9_]+['"]\s*\]/g) || []; - const names = [...dotted, ...bracketed] - .map(hit => (hit.match(/GATEGUARD_[A-Z0-9_]+/) || [])[0]) + const code = blankCommentsAndLiterals(source); + const bracketed = bracketedEnvKeys(source) + .map(key => (key.match(QUOTED_KEY) || [])[2]) .filter(Boolean); - return new Set(names); + return new Set([...matchAll(code, DOTTED_READ), ...bracketed]); +} + +/** + * Access forms this parser cannot follow. Each would let a GATEGUARD_* read + * escape the documentation check, so they are rejected outright. + */ +const UNSUPPORTED_ACCESS = [ + { label: 'destructuring from process.env', pattern: /\}\s*=\s*process\.env\b/ }, + { label: 'process.env aliased to a binding', pattern: /(?:const|let|var)\s+[A-Za-z_$][\w$]*\s*=\s*process\.env\s*(?:[;,)\]]|$)/m }, + { label: 'spread of process.env', pattern: /\.\.\.\s*process\.env\b/ }, + { label: 'enumeration of process.env', pattern: /Object\.(?:keys|values|entries|assign|fromEntries)\(\s*process\.env\b/ }, +]; + +/** `process.env[...]` whose key is not a plain quoted string. */ +function findComputedEnvAccess(source) { + return bracketedEnvKeys(source).filter(key => !PLAIN_QUOTED_KEY.test(key)); +} + +function findUnsupportedAccess(source) { + const code = blankCommentsAndLiterals(source); + const structural = UNSUPPORTED_ACCESS.filter(rule => rule.pattern.test(code)).map(rule => rule.label); + const computed = findComputedEnvAccess(source).map(key => `computed process.env[${key}]`); + return [...structural, ...computed]; } console.log('\nGateGuard env-var documentation surface\n'); @@ -79,6 +247,74 @@ if (test('the documented knobs are the ones the hook actually reads', () => { assert.deepStrictEqual(stale, [], `documented but unread by the hook: ${stale.join(', ')}`); })) passed++; else failed++; +if (test('the hook reaches process.env only through the supported literal forms', () => { + const unsupported = findUnsupportedAccess(hookSource).sort(); + assert.deepStrictEqual( + unsupported, + [], + 'the hook uses an env access form this test cannot follow, so an undocumented ' + + 'GATEGUARD_* knob could bypass the check. Either keep to ' + + "`process.env.GATEGUARD_X` / `process.env['GATEGUARD_X']`, or teach " + + `readGateguardEnvNames the new form. Found: ${unsupported.join(', ')}` + ); +})) passed++; else failed++; + +// --- parser self-checks: the convention above is only worth as much as these --- + +if (test('blanking preserves offsets and line count', () => { + const blanked = blankCommentsAndLiterals(hookSource); + assert.strictEqual(blanked.length, hookSource.length, 'blanking changed the source length'); + assert.strictEqual( + blanked.split('\n').length, + hookSource.split('\n').length, + 'blanking changed the line count' + ); +})) passed++; else failed++; + +if (test('env reads are read from code, not from comments, strings or regexes', () => { + const fixture = [ + "const a = process.env.GATEGUARD_REAL_ONE;", + "const b = process.env['GATEGUARD_REAL_TWO'];", + '// process.env.GATEGUARD_IN_LINE_COMMENT is only mentioned here', + '/* process.env.GATEGUARD_IN_BLOCK_COMMENT */', + "const msg = 'process.env.GATEGUARD_IN_STRING';", + 'const tpl = `process.env.GATEGUARD_IN_TEMPLATE ${process.env.GATEGUARD_REAL_THREE}`;', + 'const re = /process\\.env\\.GATEGUARD_IN_REGEX\\/\\//;', + ].join('\n'); + const found = [...readGateguardEnvNames(fixture)].sort(); + assert.deepStrictEqual(found, ['GATEGUARD_REAL_ONE', 'GATEGUARD_REAL_THREE', 'GATEGUARD_REAL_TWO']); +})) passed++; else failed++; + +if (test('a regex literal containing a slash does not swallow the code after it', () => { + const fixture = 'const re = /a\\/\\/b/;\nconst x = process.env.GATEGUARD_AFTER_REGEX;'; + assert.deepStrictEqual([...readGateguardEnvNames(fixture)], ['GATEGUARD_AFTER_REGEX']); +})) passed++; else failed++; + +if (test('the access guard rejects every form the parser cannot follow', () => { + const cases = [ + ['destructuring', 'const { GATEGUARD_HIDDEN } = process.env;'], + ['alias', 'const env = process.env;\nconst v = env.GATEGUARD_HIDDEN;'], + ['computed template', 'const v = process.env[`GATEGUARD_${suffix}`];'], + ['computed variable', 'const v = process.env[name];'], + ['spread', 'const all = { ...process.env };'], + ['enumeration', 'const ks = Object.keys(process.env);'], + ]; + const missed = cases.filter(([, code]) => findUnsupportedAccess(code).length === 0).map(([label]) => label); + assert.deepStrictEqual(missed, [], `access guard missed: ${missed.join(', ')}`); +})) passed++; else failed++; + +if (test('the access guard accepts the supported forms and ignores commented ones', () => { + const ok = [ + 'const v = process.env.GATEGUARD_STATE_DIR;', + "const v = process.env['GATEGUARD_STATE_DIR'];", + 'const v = process.env["GATEGUARD_STATE_DIR"];', + '// const { GATEGUARD_HIDDEN } = process.env;', + "const doc = 'const { GATEGUARD_HIDDEN } = process.env;';", + ]; + const wrong = ok.filter(code => findUnsupportedAccess(code).length > 0); + assert.deepStrictEqual(wrong, [], `false positives from the access guard: ${wrong.join(' | ')}`); +})) passed++; else failed++; + console.log(`\nPassed: ${passed}`); console.log(`Failed: ${failed}\n`); From 51dc76ee07bd8154ab07d70b9586aa2771c81bdd Mon Sep 17 00:00:00 2001 From: Souptik Chakraborty Date: Fri, 14 Aug 2026 10:17:54 +0530 Subject: [PATCH 25/46] test(gateguard): reject Reflect access on process.env Greptile flagged that the env-access guard in gateguard-env-documented.test.js could be bypassed via reflective reads of process.env (Reflect.get/has/set/deleteProperty/defineProperty/ getOwnPropertyDescriptor/ownKeys), since none of the existing UNSUPPORTED_ACCESS patterns matched that form. Add a rule that rejects Reflect.get/has/set/deleteProperty/ defineProperty/getOwnPropertyDescriptor/ownKeys(process.env, ...) and three self-check fixture cases (Reflect.get, Reflect.has, Reflect.ownKeys) so the guard is pinned against silently missing them again. Negative control: commenting out only the new rule reproduces exactly the reported gap (the 3 new fixture cases fail with "access guard missed: Reflect.get, Reflect.has, Reflect.ownKeys"); restoring it goes back to 10/10. --- tests/ci/gateguard-env-documented.test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/ci/gateguard-env-documented.test.js b/tests/ci/gateguard-env-documented.test.js index 2a9da88c7..6ee96754c 100644 --- a/tests/ci/gateguard-env-documented.test.js +++ b/tests/ci/gateguard-env-documented.test.js @@ -200,6 +200,7 @@ const UNSUPPORTED_ACCESS = [ { label: 'process.env aliased to a binding', pattern: /(?:const|let|var)\s+[A-Za-z_$][\w$]*\s*=\s*process\.env\s*(?:[;,)\]]|$)/m }, { label: 'spread of process.env', pattern: /\.\.\.\s*process\.env\b/ }, { label: 'enumeration of process.env', pattern: /Object\.(?:keys|values|entries|assign|fromEntries)\(\s*process\.env\b/ }, + { label: 'Reflect access on process.env', pattern: /Reflect\.(?:get|has|set|deleteProperty|defineProperty|getOwnPropertyDescriptor|ownKeys)\(\s*process\.env\b/ }, ]; /** `process.env[...]` whose key is not a plain quoted string. */ @@ -298,6 +299,9 @@ if (test('the access guard rejects every form the parser cannot follow', () => { ['computed variable', 'const v = process.env[name];'], ['spread', 'const all = { ...process.env };'], ['enumeration', 'const ks = Object.keys(process.env);'], + ['Reflect.get', "const v = Reflect.get(process.env, 'GATEGUARD_HIDDEN');"], + ['Reflect.has', "const v = Reflect.has(process.env, 'GATEGUARD_HIDDEN');"], + ['Reflect.ownKeys', 'const ks = Reflect.ownKeys(process.env);'], ]; const missed = cases.filter(([, code]) => findUnsupportedAccess(code).length === 0).map(([label]) => label); assert.deepStrictEqual(missed, [], `access guard missed: ${missed.join(', ')}`); From 974ccc749f2fb1463c1a340905f5e836978b6a52 Mon Sep 17 00:00:00 2001 From: Juan Pablo Date: Thu, 30 Jul 2026 08:14:29 -0500 Subject: [PATCH 26/46] fix(ci): validate SKILL.md frontmatter under docs/{locale}/skills/ mirrors Extends scripts/ci/validate-skills.js to also scan docs/{locale}/skills/ translated mirrors, not just curated skills/. Adds detection for the YAML defect classes from #2630 without a parser dependency: unquoted values containing ": " (glued next key / dropped quoting), values starting with the reserved '@'/'`' indicators, and missing frontmatter blocks entirely (required only for docs mirrors; curated skills/ keeps its existing tolerant behavior). --- scripts/ci/validate-skills.js | 178 ++++++++++++++++++++++++++++------ tests/ci/validators.test.js | 97 +++++++++++++++++- 2 files changed, 246 insertions(+), 29 deletions(-) diff --git a/scripts/ci/validate-skills.js b/scripts/ci/validate-skills.js index 6ffc85376..1ae0e67ce 100644 --- a/scripts/ci/validate-skills.js +++ b/scripts/ci/validate-skills.js @@ -1,11 +1,13 @@ #!/usr/bin/env node /** - * Validate curated skill directories (skills/ in repo). + * Validate curated skill directories (skills/ in repo) and their + * translated mirrors (docs/{locale}/skills/ in repo). * * Checks: * 1. Each sub-directory of skills/ contains a SKILL.md file. * 2. SKILL.md is non-empty. - * 3. SKILL.md frontmatter (if present) declares a `name:` field. + * 3. SKILL.md frontmatter is present and declares both `name:` and + * `description:` fields. * 4. SKILL.md frontmatter `description:` uses an inline scalar — not a * literal block scalar (`|` / `|-` / `|+`), which preserves internal * newlines and breaks flat-table renderers keyed off `description`. @@ -17,14 +19,16 @@ * * Structural findings (missing/empty SKILL.md) are always errors. * - * Scope: curated only. Learned/imported/evolved roots are out of scope. - * If skills/ does not exist, exit 0 (no curated skills to validate). + * Scope: curated skills/ plus translated docs/{locale}/skills/ mirrors. + * Learned/imported/evolved roots are out of scope. If neither root + * exists, exit 0 (nothing to validate). */ const fs = require('fs'); const path = require('path'); const SKILLS_DIR = path.join(__dirname, '../../skills'); +const DOCS_DIR = path.join(__dirname, '../../docs'); const STRICT = process.argv.includes('--strict') || process.env.CI_STRICT_SKILLS === '1'; @@ -66,6 +70,7 @@ function extractFrontmatter(content) { */ function inspectFrontmatter(lines) { const values = Object.create(null); + const syntaxErrors = []; let descriptionIndicator = null; let inBlockScalar = false; let blockScalarIndent = -1; @@ -96,6 +101,27 @@ function inspectFrontmatter(lines) { .trim(); values[key] = valueNoComment; + const isQuoted = /^"(?:[^"\\]|\\.)*"$/.test(valueNoComment) || /^'(?:[^']|'')*'$/.test(valueNoComment); + + if (!isQuoted && valueNoComment !== '') { + // A plain (unquoted) YAML scalar can never contain ": " — that + // sequence starts a new mapping key. When the translation pass + // drops a value's quoting, or glues the next frontmatter key onto + // the end of a value, this is exactly what shows up (see #2630). + if (valueNoComment.includes(': ')) { + syntaxErrors.push( + `${key}: unquoted value contains ': ' — invalid YAML; ` + `quote the value or the next key was likely glued onto this line` + ); + } + + // '@' and '`' are reserved YAML indicators and cannot start a + // plain scalar (see #2630 — a reordering during translation moved + // '@' into the first column of an unquoted description). + if (/^[@`]/.test(valueNoComment)) { + syntaxErrors.push(`${key}: unquoted value starts with reserved character '${valueNoComment[0]}' — quote the value`); + } + } + // Detect literal / folded block-scalar indicators. Accept chomp // modifiers (`-` / `+`) and optional indent-indicator digits in // either order, per YAML 1.2. @@ -108,7 +134,7 @@ function inspectFrontmatter(lines) { } } - return { values, descriptionIndicator }; + return { values, descriptionIndicator, syntaxErrors }; } /** @@ -120,6 +146,10 @@ function inspectFrontmatter(lines) { * `reportFrontmatterFinding`, which owns the WARN/ERROR decision based * on strict mode. * + * Curated skills/ tolerates a SKILL.md with no frontmatter block at all + * (frontmatter checks only apply when a block is present) — this mirrors + * pre-existing behavior and is covered by an explicit regression test. + * * @param {string} dir * @param {string} skillsDir * @param {(msg: string) => void} reportFrontmatterFinding @@ -127,8 +157,35 @@ function inspectFrontmatter(lines) { */ function validateSkillDir(dir, skillsDir, reportFrontmatterFinding) { const skillMd = path.join(skillsDir, dir, 'SKILL.md'); + return validateSkillFile(skillMd, `${dir}/SKILL.md`, reportFrontmatterFinding, { requireFrontmatter: false }); +} + +/** + * Validate a single SKILL.md file at an arbitrary path. + * + * Shared by the curated skills/ scan and the translated + * docs/{locale}/skills/ scan — same checks apply to both, since a + * translated mirror's frontmatter must be just as parseable as the + * English original (see #2630). + * + * `requireFrontmatter: true` (used for docs/{locale}/skills/ mirrors) + * flags a completely missing frontmatter block as a finding — the + * translated mirror must carry the same `name`/`description` as its + * English original. Curated skills/ (requireFrontmatter: false) keeps + * the pre-existing tolerant behavior of skipping checks entirely when no + * block is present. + * + * @param {string} skillMd + * @param {string} label + * @param {(msg: string) => void} reportFrontmatterFinding + * @param {{requireFrontmatter?: boolean}} [opts] + * @returns {{fatal: boolean}} + */ +function validateSkillFile(skillMd, label, reportFrontmatterFinding, opts = {}) { + const { requireFrontmatter = false } = opts; + if (!fs.existsSync(skillMd)) { - console.error(`ERROR: ${dir}/ - Missing SKILL.md`); + console.error(`ERROR: ${label} - Missing SKILL.md`); return { fatal: true }; } @@ -136,42 +193,93 @@ function validateSkillDir(dir, skillsDir, reportFrontmatterFinding) { try { content = fs.readFileSync(skillMd, 'utf-8'); } catch (err) { - console.error(`ERROR: ${dir}/SKILL.md - ${err.message}`); + console.error(`ERROR: ${label} - ${err.message}`); return { fatal: true }; } if (content.trim().length === 0) { - console.error(`ERROR: ${dir}/SKILL.md - Empty file`); + console.error(`ERROR: ${label} - Empty file`); return { fatal: true }; } const fm = extractFrontmatter(content); - if (fm.present) { - const { values, descriptionIndicator } = inspectFrontmatter(fm.lines); - - if (!Object.prototype.hasOwnProperty.call(values, 'name')) { - reportFrontmatterFinding(`${dir}/SKILL.md - frontmatter missing required field: name`); - } else if (values.name === '') { - reportFrontmatterFinding(`${dir}/SKILL.md - frontmatter 'name' is empty`); + if (!fm.present) { + if (requireFrontmatter) { + reportFrontmatterFinding(`${label} - no frontmatter block found (missing name/description)`); } + return { fatal: false }; + } - if (descriptionIndicator && descriptionIndicator.startsWith('|')) { - reportFrontmatterFinding( - `${dir}/SKILL.md - frontmatter description uses literal block scalar ` + `'${descriptionIndicator}' which preserves internal newlines; ` + `use an inline string or folded '>' scalar instead` - ); - } + const { values, descriptionIndicator, syntaxErrors } = inspectFrontmatter(fm.lines); + + if (!Object.prototype.hasOwnProperty.call(values, 'name')) { + reportFrontmatterFinding(`${label} - frontmatter missing required field: name`); + } else if (values.name === '') { + reportFrontmatterFinding(`${label} - frontmatter 'name' is empty`); + } + + if (!Object.prototype.hasOwnProperty.call(values, 'description')) { + reportFrontmatterFinding(`${label} - frontmatter missing required field: description`); + } else if (values.description === '') { + reportFrontmatterFinding(`${label} - frontmatter 'description' is empty`); + } + + if (descriptionIndicator && descriptionIndicator.startsWith('|')) { + reportFrontmatterFinding( + `${label} - frontmatter description uses literal block scalar ` + `'${descriptionIndicator}' which preserves internal newlines; ` + `use an inline string or folded '>' scalar instead` + ); + } + + for (const syntaxError of syntaxErrors) { + reportFrontmatterFinding(`${label} - frontmatter ${syntaxError}`); } return { fatal: false }; } -function validateSkills() { - if (!fs.existsSync(SKILLS_DIR)) { - console.log('No curated skills directory (skills/), skipping'); - process.exit(0); +/** + * Find every SKILL.md under docs/{locale}/skills/*, mirroring the + * curated skills/ layout one locale directory deeper. + * + * @param {string} docsDir + * @returns {Array<{skillMd: string, label: string}>} + */ +function findDocsSkillFiles(docsDir) { + if (!fs.existsSync(docsDir)) return []; + + const files = []; + const locales = fs + .readdirSync(docsDir, { withFileTypes: true }) + .filter(e => e.isDirectory() && !e.name.startsWith('.')) + .map(e => e.name); + + for (const locale of locales) { + const localeSkillsDir = path.join(docsDir, locale, 'skills'); + if (!fs.existsSync(localeSkillsDir)) continue; + + const skillDirs = fs + .readdirSync(localeSkillsDir, { withFileTypes: true }) + .filter(e => e.isDirectory() && !e.name.startsWith('.')) + .map(e => e.name); + + for (const skillDir of skillDirs) { + files.push({ + skillMd: path.join(localeSkillsDir, skillDir, 'SKILL.md'), + label: `docs/${locale}/skills/${skillDir}/SKILL.md` + }); + } } - const entries = fs.readdirSync(SKILLS_DIR, { withFileTypes: true }); - const dirs = entries.filter(e => e.isDirectory() && !e.name.startsWith('.')).map(e => e.name); + return files; +} + +function validateSkills() { + const curatedExists = fs.existsSync(SKILLS_DIR); + const docsSkillFiles = findDocsSkillFiles(DOCS_DIR); + + if (!curatedExists && docsSkillFiles.length === 0) { + console.log('No skills directory (skills/ or docs/*/skills/), skipping'); + process.exit(0); + } let hasErrors = false; let warnCount = 0; @@ -187,8 +295,22 @@ function validateSkills() { } }; - for (const dir of dirs) { - const { fatal } = validateSkillDir(dir, SKILLS_DIR, reportFrontmatterFinding); + if (curatedExists) { + const entries = fs.readdirSync(SKILLS_DIR, { withFileTypes: true }); + const dirs = entries.filter(e => e.isDirectory() && !e.name.startsWith('.')).map(e => e.name); + + for (const dir of dirs) { + const { fatal } = validateSkillDir(dir, SKILLS_DIR, reportFrontmatterFinding); + if (fatal) { + hasErrors = true; + continue; + } + validCount++; + } + } + + for (const { skillMd, label } of docsSkillFiles) { + const { fatal } = validateSkillFile(skillMd, label, reportFrontmatterFinding, { requireFrontmatter: true }); if (fatal) { hasErrors = true; continue; diff --git a/tests/ci/validators.test.js b/tests/ci/validators.test.js index 702ab4cd7..e6d950161 100644 --- a/tests/ci/validators.test.js +++ b/tests/ci/validators.test.js @@ -213,7 +213,7 @@ function runCatalogValidator(overrides = {}) { // Captures stderr on both success and failure (the shared // runSourceViaTempFile helper only surfaces stderr when the child // exits non-zero, which hides WARN lines in the default mode). -function runSkillsValidator(testDir, argv = [], envOverrides = {}) { +function runSkillsValidator(testDir, argv = [], envOverrides = {}, docsDir) { const validatorPath = path.join(validatorsDir, 'validate-skills.js'); let source = fs.readFileSync(validatorPath, 'utf8'); source = stripShebang(source); @@ -221,6 +221,12 @@ function runSkillsValidator(testDir, argv = [], envOverrides = {}) { /const SKILLS_DIR = .*?;/, `const SKILLS_DIR = ${JSON.stringify(testDir)};`, ); + // Default to a nonexistent docs root so tests exercising only + // SKILLS_DIR aren't polluted by this repo's real docs/*/skills/ tree. + source = source.replace( + /const DOCS_DIR = .*?;/, + `const DOCS_DIR = ${JSON.stringify(docsDir || '/nonexistent-docs-dir-for-tests')};`, + ); if (argv.length > 0) { const argvPreamble = argv .map(arg => `process.argv.push(${JSON.stringify(arg)});`) @@ -2801,6 +2807,95 @@ function runTests() { cleanupTestDir(testDir); })) passed++; else failed++; + // ── Round 84: validate-skills docs/{locale}/skills/ mirror scan (#2630) ── + + console.log('\nRound 84: validate-skills.js (docs/{locale}/skills/ frontmatter, #2630):'); + + if (test('flags a glued key onto description as invalid YAML', () => { + const testDir = createTestDir(); + const docsDir = path.join(testDir, 'docs-root'); + const skillDir = path.join(docsDir, 'ja-JP', 'skills', 'example'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), + '---\nname: example\ndescription: some text.license: Apache-2.0\nversion: 1.0.0\n---\n# Example'); + + const result = runSkillsValidator('/nonexistent/skills-dir', ['--strict'], {}, docsDir); + assert.strictEqual(result.code, 1, 'Should fail on glued key'); + assert.ok(result.stderr.includes("unquoted value contains ': '"), + `Should report the glued-key defect, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('flags a dropped-quote description containing a colon as invalid YAML', () => { + const testDir = createTestDir(); + const docsDir = path.join(testDir, 'docs-root'); + const skillDir = path.join(docsDir, 'ja-JP', 'skills', 'example'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), + '---\nname: example\ndescription: Verification loop: migrations, linting\n---\n# Example'); + + const result = runSkillsValidator('/nonexistent/skills-dir', ['--strict'], {}, docsDir); + assert.strictEqual(result.code, 1, 'Should fail on unquoted colon in description'); + assert.ok(result.stderr.includes("unquoted value contains ': '"), + `Should report the dropped-quote defect, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('flags a description starting with the reserved @ indicator', () => { + const testDir = createTestDir(); + const docsDir = path.join(testDir, 'docs-root'); + const skillDir = path.join(docsDir, 'ja-JP', 'skills', 'example'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), + '---\nname: example\ndescription: @Observable state management\n---\n# Example'); + + const result = runSkillsValidator('/nonexistent/skills-dir', ['--strict'], {}, docsDir); + assert.strictEqual(result.code, 1, 'Should fail on leading @'); + assert.ok(result.stderr.includes("reserved character '@'"), + `Should report the reserved-indicator defect, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('flags a docs mirror SKILL.md with no frontmatter block at all', () => { + const testDir = createTestDir(); + const docsDir = path.join(testDir, 'docs-root'); + const skillDir = path.join(docsDir, 'ja-JP', 'skills', 'example'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), '# Example\n\nNo frontmatter here.'); + + const result = runSkillsValidator('/nonexistent/skills-dir', ['--strict'], {}, docsDir); + assert.strictEqual(result.code, 1, 'Should fail when docs mirror has no frontmatter'); + assert.ok(result.stderr.includes('no frontmatter block found'), + `Should report the missing-frontmatter defect, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('curated skills/ still tolerates a SKILL.md with no frontmatter (unchanged)', () => { + const testDir = createTestDir(); + const skillDir = path.join(testDir, 'no-frontmatter-skill'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), '# Example\n\nNo frontmatter here.'); + + const result = runSkillsValidator(testDir, ['--strict']); + assert.strictEqual(result.code, 0, + `Curated skills/ must not require frontmatter, got stderr: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('passes on a valid docs/{locale}/skills/ mirror', () => { + const testDir = createTestDir(); + const docsDir = path.join(testDir, 'docs-root'); + const skillDir = path.join(docsDir, 'zh-CN', 'skills', 'example'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), + '---\nname: example\ndescription: "Well-formed: quoted value"\n---\n# Example'); + + const result = runSkillsValidator('/nonexistent/skills-dir', ['--strict'], {}, docsDir); + assert.strictEqual(result.code, 0, `Should pass on well-formed mirror, got: ${result.stderr}`); + assert.ok(result.stdout.includes('Validated 1'), 'Should count the one docs skill file'); + cleanupTestDir(testDir); + })) passed++; else failed++; + // ========================================== // validate-install-manifests.js // ========================================== From 1c450766a9dfa4583980a875b58a777aa2a8a89f Mon Sep 17 00:00:00 2001 From: Deepu S Nath Date: Fri, 31 Jul 2026 21:27:48 +0530 Subject: [PATCH 27/46] fix(skill-stocktake): follow symlinks and match only SKILL.md in scans scan.sh and quick-diff.sh both used `find "$dir" -name "*.md" -type f`, which missed symlinked skill directories (no -L) and miscounted any non-skill markdown file sitting in a skills directory as a skill (matched *.md instead of SKILL.md). Both call sites now use `find -L "$dir" -name "SKILL.md" -type f`. Repro (temp dir with 1 real skill, 1 symlinked skill, 1 stray .md file): before: 2 skills found (real skill + the stray .md, symlinked skill invisible) after: 2 skills found (real skill + symlinked skill, stray .md excluded) Fixes #2598 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FhDjpSfrbPpnpqT3CZBEX1 --- skills/skill-stocktake/scripts/quick-diff.sh | 2 +- skills/skill-stocktake/scripts/scan.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/skills/skill-stocktake/scripts/quick-diff.sh b/skills/skill-stocktake/scripts/quick-diff.sh index c145100a6..75dbac273 100755 --- a/skills/skill-stocktake/scripts/quick-diff.sh +++ b/skills/skill-stocktake/scripts/quick-diff.sh @@ -74,7 +74,7 @@ process_dir() { '{path:$path,mtime:$mtime,is_new:$is_new}' \ > "$tmpdir/$i.json" i=$((i+1)) - done < <(find "$dir" -name "*.md" -type f 2>/dev/null | sort) + done < <(find -L "$dir" -name "SKILL.md" -type f 2>/dev/null | sort) } [[ -d "$GLOBAL_DIR" ]] && process_dir "$GLOBAL_DIR" diff --git a/skills/skill-stocktake/scripts/scan.sh b/skills/skill-stocktake/scripts/scan.sh index 5f1d12dbd..9a5aca497 100755 --- a/skills/skill-stocktake/scripts/scan.sh +++ b/skills/skill-stocktake/scripts/scan.sh @@ -118,7 +118,7 @@ scan_dir_to_json() { '{path:$path,name:$name,description:$description,use_7d:$use_7d,use_30d:$use_30d,mtime:$mtime}' \ > "$tmpdir/$i.json" i=$((i+1)) - done < <(find "$dir" -name "*.md" -type f 2>/dev/null | sort) + done < <(find -L "$dir" -name "SKILL.md" -type f 2>/dev/null | sort) if [[ $i -eq 0 ]]; then echo "[]" From dfb5da59fb8fed14bade180c1f348711417c8e89 Mon Sep 17 00:00:00 2001 From: Deepu S Nath Date: Fri, 31 Jul 2026 23:18:03 +0530 Subject: [PATCH 28/46] fix(skill-stocktake): surface find errors instead of swallowing them Following up on the -L fix: find -L can now traverse symlinks, but a broken symlink target or an unreadable directory makes find skip that entry and exit non-zero. Both scripts previously redirected find's stderr to /dev/null and never checked its exit status, so a scan could silently under-count skills with no indication anything was wrong. Capture find's exit status and stderr in both scripts; on failure, print a warning (with the underlying find error) to stderr while still emitting the best-effort results for whatever was found. Verified with a permission-denied skill directory: real BSD find exits 1 and reports "Permission denied" on stderr, now surfaced as an explicit warning instead of silently dropped. Addresses CodeRabbit review feedback on PR #2640. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FhDjpSfrbPpnpqT3CZBEX1 --- skills/skill-stocktake/scripts/quick-diff.sh | 13 ++++++++++++- skills/skill-stocktake/scripts/scan.sh | 13 ++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/skills/skill-stocktake/scripts/quick-diff.sh b/skills/skill-stocktake/scripts/quick-diff.sh index 75dbac273..a2177b39a 100755 --- a/skills/skill-stocktake/scripts/quick-diff.sh +++ b/skills/skill-stocktake/scripts/quick-diff.sh @@ -51,6 +51,17 @@ i=0 process_dir() { local dir="$1" + local find_out="$tmpdir/.find-stdout" + local find_err="$tmpdir/.find-stderr" + # Capture find's exit status and stderr instead of discarding them: with -L, + # a broken symlink or unreadable directory makes find skip that entry AND + # exit non-zero, which would otherwise silently under-count skills. + if ! find -L "$dir" -name "SKILL.md" -type f >"$find_out" 2>"$find_err"; then + echo "Warning: find encountered errors while scanning $dir (broken symlinks or permission issues may cause skills to be missed):" >&2 + cat "$find_err" >&2 + fi + sort -o "$find_out" "$find_out" + while IFS= read -r file; do local mtime dp is_new mtime=$(date -u -r "$file" +%Y-%m-%dT%H:%M:%SZ) @@ -74,7 +85,7 @@ process_dir() { '{path:$path,mtime:$mtime,is_new:$is_new}' \ > "$tmpdir/$i.json" i=$((i+1)) - done < <(find -L "$dir" -name "SKILL.md" -type f 2>/dev/null | sort) + done < "$find_out" } [[ -d "$GLOBAL_DIR" ]] && process_dir "$GLOBAL_DIR" diff --git a/skills/skill-stocktake/scripts/scan.sh b/skills/skill-stocktake/scripts/scan.sh index 9a5aca497..4197fff96 100755 --- a/skills/skill-stocktake/scripts/scan.sh +++ b/skills/skill-stocktake/scripts/scan.sh @@ -95,6 +95,17 @@ scan_dir_to_json() { fi local i=0 + local find_out="$tmpdir/.find-stdout" + local find_err="$tmpdir/.find-stderr" + # Capture find's exit status and stderr instead of discarding them: with -L, + # a broken symlink or unreadable directory makes find skip that entry AND + # exit non-zero, which would otherwise silently under-count skills. + if ! find -L "$dir" -name "SKILL.md" -type f >"$find_out" 2>"$find_err"; then + echo "Warning: find encountered errors while scanning $dir (broken symlinks or permission issues may cause skills to be missed):" >&2 + cat "$find_err" >&2 + fi + sort -o "$find_out" "$find_out" + while IFS= read -r file; do local name desc mtime u7 u30 dp name=$(extract_field "$file" "name") @@ -118,7 +129,7 @@ scan_dir_to_json() { '{path:$path,name:$name,description:$description,use_7d:$use_7d,use_30d:$use_30d,mtime:$mtime}' \ > "$tmpdir/$i.json" i=$((i+1)) - done < <(find -L "$dir" -name "SKILL.md" -type f 2>/dev/null | sort) + done < "$find_out" if [[ $i -eq 0 ]]; then echo "[]" From 981f97bff49d2aaac287c5092f1a9ebf57df5834 Mon Sep 17 00:00:00 2001 From: Deepu S Nath Date: Sat, 1 Aug 2026 00:17:28 +0530 Subject: [PATCH 29/46] fix(skill-stocktake): use NUL-delimited paths to avoid newline desync The find -> sort -> read chain in both scripts used newline-delimited records (plain read -r), so a skill directory name containing a literal newline would be split across two records. Verified with a directory literally named "evil\nskill": the old reader produced a truncated "evil" fragment plus an orphan "skill/SKILL.md" fragment, inflating the skill count and throwing awk/date errors on the garbage paths. Switch to -print0 / sort -z / read -r -d '' in both scripts so a path is always read as a single record, regardless of its contents. Paths under scan are untrusted input. Addresses further CodeRabbit review feedback on PR #2640. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FhDjpSfrbPpnpqT3CZBEX1 --- skills/skill-stocktake/scripts/quick-diff.sh | 8 +++++--- skills/skill-stocktake/scripts/scan.sh | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/skills/skill-stocktake/scripts/quick-diff.sh b/skills/skill-stocktake/scripts/quick-diff.sh index a2177b39a..f929b2432 100755 --- a/skills/skill-stocktake/scripts/quick-diff.sh +++ b/skills/skill-stocktake/scripts/quick-diff.sh @@ -56,13 +56,15 @@ process_dir() { # Capture find's exit status and stderr instead of discarding them: with -L, # a broken symlink or unreadable directory makes find skip that entry AND # exit non-zero, which would otherwise silently under-count skills. - if ! find -L "$dir" -name "SKILL.md" -type f >"$find_out" 2>"$find_err"; then + # NUL-delimited (-print0 / sort -z / read -d '') so a path containing a + # literal newline can't desync record boundaries — paths here are untrusted. + if ! find -L "$dir" -name "SKILL.md" -type f -print0 >"$find_out" 2>"$find_err"; then echo "Warning: find encountered errors while scanning $dir (broken symlinks or permission issues may cause skills to be missed):" >&2 cat "$find_err" >&2 fi - sort -o "$find_out" "$find_out" + sort -z -o "$find_out" "$find_out" - while IFS= read -r file; do + while IFS= read -r -d '' file; do local mtime dp is_new mtime=$(date -u -r "$file" +%Y-%m-%dT%H:%M:%SZ) dp="${file/#$HOME/~}" diff --git a/skills/skill-stocktake/scripts/scan.sh b/skills/skill-stocktake/scripts/scan.sh index 4197fff96..76f5523dc 100755 --- a/skills/skill-stocktake/scripts/scan.sh +++ b/skills/skill-stocktake/scripts/scan.sh @@ -100,13 +100,15 @@ scan_dir_to_json() { # Capture find's exit status and stderr instead of discarding them: with -L, # a broken symlink or unreadable directory makes find skip that entry AND # exit non-zero, which would otherwise silently under-count skills. - if ! find -L "$dir" -name "SKILL.md" -type f >"$find_out" 2>"$find_err"; then + # NUL-delimited (-print0 / sort -z / read -d '') so a path containing a + # literal newline can't desync record boundaries — paths here are untrusted. + if ! find -L "$dir" -name "SKILL.md" -type f -print0 >"$find_out" 2>"$find_err"; then echo "Warning: find encountered errors while scanning $dir (broken symlinks or permission issues may cause skills to be missed):" >&2 cat "$find_err" >&2 fi - sort -o "$find_out" "$find_out" + sort -z -o "$find_out" "$find_out" - while IFS= read -r file; do + while IFS= read -r -d '' file; do local name desc mtime u7 u30 dp name=$(extract_field "$file" "name") desc=$(extract_field "$file" "description") From 9542c334543eaaa765ec4efbae00be27e05186c6 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:26:49 -0400 Subject: [PATCH 30/46] test(skill-stocktake): cover canonical symlink discovery Co-authored-by: LKL-ZREO <891878708@qq.com> --- .../scripts/skill-stocktake-discovery.test.js | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 tests/scripts/skill-stocktake-discovery.test.js diff --git a/tests/scripts/skill-stocktake-discovery.test.js b/tests/scripts/skill-stocktake-discovery.test.js new file mode 100644 index 000000000..d1d36c0d5 --- /dev/null +++ b/tests/scripts/skill-stocktake-discovery.test.js @@ -0,0 +1,113 @@ +#!/usr/bin/env node + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const repoRoot = path.resolve(__dirname, '..', '..'); +const scanScript = path.join(repoRoot, 'skills', 'skill-stocktake', 'scripts', 'scan.sh'); +const quickDiffScript = path.join(repoRoot, 'skills', 'skill-stocktake', 'scripts', 'quick-diff.sh'); + +let passed = 0; +let failed = 0; + +function test(description, fn) { + try { + fn(); + console.log(` ✓ ${description}`); + passed++; + } catch (error) { + console.log(` ✗ ${description}: ${error.message}`); + failed++; + } +} + +function writeSkill(skillDir, name) { + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync( + path.join(skillDir, 'SKILL.md'), + `---\nname: ${name}\ndescription: test fixture\n---\n# ${name}\n`, + ); +} + +function runBash(scriptPath, args, env) { + return spawnSync('bash', [scriptPath, ...args], { + encoding: 'utf8', + env: { ...process.env, ...env }, + }); +} + +console.log('\nSkill stocktake discovery tests:'); + +test('both scanners use canonical, error-visible, NUL-delimited discovery', () => { + for (const scriptPath of [scanScript, quickDiffScript]) { + const source = fs.readFileSync(scriptPath, 'utf8'); + assert.match(source, /find -L "\$dir" -name "SKILL\.md" -type f -print0/); + assert.match(source, /sort -z -o "\$find_out" "\$find_out"/); + assert.match(source, /read -r -d '' file/); + assert.doesNotMatch(source, /find [^\n]*2>\/dev\/null/, `${path.basename(scriptPath)} still hides find errors`); + } +}); + +if (process.platform === 'win32') { + console.log(' ↷ POSIX symlink and newline-path integration cases skipped on Windows'); +} else { + const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-skill-stocktake-')); + try { + const projectSkills = path.join(tempRoot, 'project', '.claude', 'skills'); + const directSkill = path.join(projectSkills, 'direct-skill'); + const linkedTarget = path.join(tempRoot, 'shared', 'linked-skill'); + const newlineSkill = path.join(projectSkills, 'newline\nskill'); + const resultsPath = path.join(tempRoot, 'results.json'); + + writeSkill(directSkill, 'direct-skill'); + writeSkill(linkedTarget, 'linked-skill'); + writeSkill(newlineSkill, 'newline-skill'); + fs.symlinkSync(linkedTarget, path.join(projectSkills, 'linked-skill'), 'dir'); + fs.mkdirSync(path.join(directSkill, 'references'), { recursive: true }); + fs.writeFileSync(path.join(directSkill, 'references', 'notes.md'), '# supporting notes\n'); + fs.writeFileSync( + resultsPath, + JSON.stringify({ evaluated_at: '2099-01-01T00:00:00Z', skills: [] }), + ); + + const env = { + SKILL_STOCKTAKE_GLOBAL_DIR: path.join(tempRoot, 'missing-global'), + SKILL_STOCKTAKE_PROJECT_DIR: projectSkills, + SKILL_STOCKTAKE_OBSERVATIONS: path.join(tempRoot, 'missing-observations.jsonl'), + }; + + test('scan follows symlinked skills and ignores nested Markdown assets', () => { + const result = runBash(scanScript, [], env); + assert.strictEqual(result.status, 0, result.stderr); + const output = JSON.parse(result.stdout); + assert.strictEqual(output.scan_summary.project.count, 3); + assert.deepStrictEqual( + output.skills.map(skill => skill.name).sort(), + ['direct-skill', 'linked-skill', 'newline-skill'], + ); + }); + + test('quick diff keeps newline-containing skill paths as one record', () => { + const result = runBash(quickDiffScript, [resultsPath], env); + assert.strictEqual(result.status, 0, result.stderr); + const output = JSON.parse(result.stdout); + assert.strictEqual(output.length, 3); + assert.strictEqual( + output.filter(entry => entry.path.includes('newline\nskill/SKILL.md')).length, + 1, + ); + assert.ok(output.every(entry => entry.is_new === true)); + }); + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }); + } +} + +console.log(`\nPassed: ${passed}`); +console.log(`Failed: ${failed}`); +process.exit(failed > 0 ? 1 : 0); From 2242e4d99d80a4972866f7b4e574fc78021031bb Mon Sep 17 00:00:00 2001 From: cyre Date: Sun, 9 Aug 2026 15:15:43 +0800 Subject: [PATCH 31/46] fix(skill-comply): redact operator home path from compliance reports _parse_stream_json() persisted raw tool_input/tool_response content into ObservationEvents that grade() scores and generate_report() writes to results/.md -- a report meant to be shared and reviewed. --add-dir restricts the agent's additional accessible directory to the sandbox (SANDBOX_BASE = /tmp/skill-comply-sandbox), but that doesn't stop the agent's own tool calls (a Bash command using ~ expansion, a scenario setup_commands entry referencing a dotfile) from emitting the operator's home directory into tool_input/tool_response -- which then lands verbatim, truncated but not sanitized, in the written report. Adds _redact_home_path(), pure stdlib (Path.home()), applied to both input_str and output_str before they're stored on the ObservationEvent. Scoped deliberately to the home directory only -- grade() needs real tool-call semantics for LLM-based compliance classification, so truncating/stripping content the way a pure logging hook could isn't an option here; only the operator-identifying path component needs to go. New TestParseStreamJsonRedactsHomePath class in skills/skill-comply/tests/test_runner.py (3 tests) -- full file now 10/10 passing, up from 7/7. Confirmed tests/test_invariant_runner.py (the sandbox-execution security tests from #2149) still passes clean, 4/4. Fixes #2730 --- skills/skill-comply/scripts/runner.py | 20 +++++++++- skills/skill-comply/tests/test_runner.py | 51 +++++++++++++++++++++++- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/skills/skill-comply/scripts/runner.py b/skills/skill-comply/scripts/runner.py index 84421c4f4..fc052a579 100644 --- a/skills/skill-comply/scripts/runner.py +++ b/skills/skill-comply/scripts/runner.py @@ -122,6 +122,22 @@ def _setup_sandbox(sandbox_dir: Path, scenario: Scenario) -> None: continue +def _redact_home_path(text: str) -> str: + """Replace the operator's home directory with a portable placeholder. + + Observations flow into grade() and then into a written report + (results/.md) that's meant to be read, diffed, and shared — + an absolute path bakes the operator's username into every tool call + that happened to touch anything under $HOME (including the sandbox + itself, which lives under a tempdir but scenario setup_commands or + an agent's own tool calls can still reference $HOME directly). + """ + home = str(Path.home()) + if home and home != "/" and home in text: + return text.replace(home, "~") + return text + + def _parse_stream_json(stdout: str) -> list[ObservationEvent]: """Parse claude -p stream-json output into ObservationEvents. @@ -154,7 +170,7 @@ def _parse_stream_json(stdout: str) -> list[ObservationEvent]: ) pending[tool_use_id] = { "tool": block.get("name", "unknown"), - "input": input_str, + "input": _redact_home_path(input_str), "order": event_counter, } event_counter += 1 @@ -178,7 +194,7 @@ def _parse_stream_json(stdout: str) -> list[ObservationEvent]: tool=info["tool"], session=msg.get("session_id", "unknown"), input=info["input"], - output=output_str, + output=_redact_home_path(output_str), )) for _tool_use_id, info in pending.items(): diff --git a/skills/skill-comply/tests/test_runner.py b/skills/skill-comply/tests/test_runner.py index 59b0700b3..2fef5a23e 100644 --- a/skills/skill-comply/tests/test_runner.py +++ b/skills/skill-comply/tests/test_runner.py @@ -6,9 +6,12 @@ import subprocess from dataclasses import dataclass from unittest.mock import MagicMock, patch +import json +from pathlib import Path + import pytest -from scripts.runner import _setup_sandbox, run_scenario +from scripts.runner import _parse_stream_json, _setup_sandbox, run_scenario @dataclass(frozen=True) @@ -143,6 +146,52 @@ class TestRunScenarioMaxTurnsTermination: run_scenario(scenario, model="haiku") +class TestParseStreamJsonRedactsHomePath: + """Observations feed grade() and then a written report (results/.md) — + a raw absolute path bakes the operator's username into every tool call + that touched anything under $HOME. --add-dir restricts the sandbox, but + scenario setup_commands or the model's own tool calls can still reference + $HOME directly (e.g. a Bash command using ~ expansion, or a scenario that + legitimately needs to read a dotfile). Redact to a portable placeholder + rather than persisting the raw path. + """ + + def _stream_json_for(self, tool_input: dict, output_text: str) -> str: + return ( + '{"type":"assistant","message":{"content":[{"type":"tool_use",' + '"id":"tu1","name":"Read","input":' + json.dumps(tool_input) + "}]}}\n" + '{"type":"user","session_id":"s1","message":{"content":[{"type":' + '"tool_result","tool_use_id":"tu1","content":' + json.dumps(output_text) + "}]}}\n" + ) + + def test_input_home_path_redacted(self): + home = str(Path.home()) + stdout = self._stream_json_for( + {"file_path": f"{home}/notes/secrets.env"}, "irrelevant output" + ) + events = _parse_stream_json(stdout) + assert len(events) == 1 + assert home not in events[0].input + assert "~/notes/secrets.env" in events[0].input + + def test_output_home_path_redacted(self): + home = str(Path.home()) + stdout = self._stream_json_for( + {"file_path": "irrelevant"}, f"wrote to {home}/notes/secrets.env" + ) + events = _parse_stream_json(stdout) + assert len(events) == 1 + assert home not in events[0].output + assert "~/notes/secrets.env" in events[0].output + + def test_paths_outside_home_untouched(self): + stdout = self._stream_json_for( + {"file_path": "/tmp/skill-comply-sandbox/t1/file.txt"}, "ok" + ) + events = _parse_stream_json(stdout) + assert "/tmp/skill-comply-sandbox/t1/file.txt" in events[0].input + + class TestRunScenarioErrorIncludesStdoutTail: """Error messages must include stdout tail, not only stderr. From d08331f14eefb72ffcb9e306c9794626700586b5 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:56:49 -0400 Subject: [PATCH 32/46] fix(skill-comply): harden home path redaction --- skills/skill-comply/scripts/runner.py | 53 +++++++---- skills/skill-comply/tests/test_runner.py | 109 +++++++++++++++++++---- 2 files changed, 128 insertions(+), 34 deletions(-) diff --git a/skills/skill-comply/scripts/runner.py b/skills/skill-comply/scripts/runner.py index fc052a579..4a852a57e 100644 --- a/skills/skill-comply/scripts/runner.py +++ b/skills/skill-comply/scripts/runner.py @@ -24,6 +24,7 @@ ALLOWED_SETUP_EXECUTABLES = frozenset({ # controlled by the cwd= keyword. Scenarios that include these in # setup_commands (a common shell-style convention) must be tolerated. SHELL_BUILTINS = frozenset({"cd", "pushd", "popd"}) +REPORT_VALUE_LIMIT = 5000 @dataclass(frozen=True) @@ -132,10 +133,40 @@ def _redact_home_path(text: str) -> str: itself, which lives under a tempdir but scenario setup_commands or an agent's own tool calls can still reference $HOME directly). """ - home = str(Path.home()) - if home and home != "/" and home in text: - return text.replace(home, "~") - return text + home = str(Path.home()).rstrip("/\\") + if not home or home == "/" or re.fullmatch(r"[A-Za-z]:", home): + return text + + parts = re.split(r"[\\/]+", home) + home_pattern = r"[\\/]".join(re.escape(part) for part in parts) + right_boundary = r"(?=$|[\\/]|[\s\"'`,;:)}\]])" + flags = re.IGNORECASE if re.match(r"^[A-Za-z]:[\\/]", home) else 0 + pattern = re.compile( + rf"(? object: + """Return a copy with home paths redacted from every string leaf.""" + if isinstance(value, str): + return _redact_home_path(value) + if isinstance(value, dict): + return {key: _redact_home_paths(item) for key, item in value.items()} + if isinstance(value, list): + return [_redact_home_paths(item) for item in value] + return value + + +def _serialize_report_value(value: object) -> str: + """Redact structured report data before encoding and truncating it.""" + redacted = _redact_home_paths(value) + if isinstance(redacted, (dict, list)): + serialized = json.dumps(redacted) + else: + serialized = str(redacted) + return serialized[:REPORT_VALUE_LIMIT] def _parse_stream_json(stdout: str) -> list[ObservationEvent]: @@ -163,14 +194,9 @@ def _parse_stream_json(stdout: str) -> list[ObservationEvent]: if block.get("type") == "tool_use": tool_use_id = block.get("id", "") tool_input = block.get("input", {}) - input_str = ( - json.dumps(tool_input)[:5000] - if isinstance(tool_input, dict) - else str(tool_input)[:5000] - ) pending[tool_use_id] = { "tool": block.get("name", "unknown"), - "input": _redact_home_path(input_str), + "input": _serialize_report_value(tool_input), "order": event_counter, } event_counter += 1 @@ -183,18 +209,13 @@ def _parse_stream_json(stdout: str) -> list[ObservationEvent]: if tool_use_id in pending: info = pending.pop(tool_use_id) output_content = block.get("content", "") - if isinstance(output_content, list): - output_str = json.dumps(output_content)[:5000] - else: - output_str = str(output_content)[:5000] - events.append(ObservationEvent( timestamp=f"T{info['order']:04d}", event="tool_complete", tool=info["tool"], session=msg.get("session_id", "unknown"), input=info["input"], - output=_redact_home_path(output_str), + output=_serialize_report_value(output_content), )) for _tool_use_id, info in pending.items(): diff --git a/skills/skill-comply/tests/test_runner.py b/skills/skill-comply/tests/test_runner.py index 2fef5a23e..a45270fc1 100644 --- a/skills/skill-comply/tests/test_runner.py +++ b/skills/skill-comply/tests/test_runner.py @@ -2,15 +2,13 @@ from __future__ import annotations +import json import subprocess from dataclasses import dataclass -from unittest.mock import MagicMock, patch - -import json from pathlib import Path +from unittest.mock import patch import pytest - from scripts.runner import _parse_stream_json, _setup_sandbox, run_scenario @@ -156,40 +154,115 @@ class TestParseStreamJsonRedactsHomePath: rather than persisting the raw path. """ - def _stream_json_for(self, tool_input: dict, output_text: str) -> str: + def _stream_json_for(self, tool_input: dict, output_content: object) -> str: return ( '{"type":"assistant","message":{"content":[{"type":"tool_use",' '"id":"tu1","name":"Read","input":' + json.dumps(tool_input) + "}]}}\n" '{"type":"user","session_id":"s1","message":{"content":[{"type":' - '"tool_result","tool_use_id":"tu1","content":' + json.dumps(output_text) + "}]}}\n" + '"tool_result","tool_use_id":"tu1","content":' + json.dumps(output_content) + "}]}}\n" ) - def test_input_home_path_redacted(self): - home = str(Path.home()) + @staticmethod + def _set_home(monkeypatch: pytest.MonkeyPatch, home: str) -> None: + monkeypatch.setattr(Path, "home", classmethod(lambda cls: Path(home))) + + def test_posix_input_string_leaves_and_embedded_paths_redacted( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + home = "/home/alice" + self._set_home(monkeypatch, home) stdout = self._stream_json_for( - {"file_path": f"{home}/notes/secrets.env"}, "irrelevant output" + { + "command": f"cat '{home}/notes/secrets.env' && echo home={home}, done", + "nested": {"paths": [f"{home}/one", f"{home}/two"]}, + }, + "irrelevant output", ) events = _parse_stream_json(stdout) + assert len(events) == 1 assert home not in events[0].input - assert "~/notes/secrets.env" in events[0].input + parsed_input = json.loads(events[0].input) + assert parsed_input["command"] == "cat '~/notes/secrets.env' && echo home=~, done" + assert parsed_input["nested"]["paths"] == ["~/one", "~/two"] - def test_output_home_path_redacted(self): - home = str(Path.home()) + def test_windows_home_with_unicode_and_backslashes_redacted( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + home = r"C:\Users\Zoë" + self._set_home(monkeypatch, home) stdout = self._stream_json_for( - {"file_path": "irrelevant"}, f"wrote to {home}/notes/secrets.env" + { + "paths": [ + home + r"\Documents\résumé.txt", + "C:/Users/Zoë/資料.txt", + ] + }, + "irrelevant output", ) events = _parse_stream_json(stdout) + assert len(events) == 1 - assert home not in events[0].output - assert "~/notes/secrets.env" in events[0].output + parsed_input = json.loads(events[0].input) + assert parsed_input["paths"] == [ + r"~\Documents\résumé.txt", + "~/資料.txt", + ] - def test_paths_outside_home_untouched(self): + def test_sibling_and_embedded_prefix_paths_untouched( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + home = "/home/alice" + self._set_home(monkeypatch, home) + outside_paths = [ + "/home/alice-old/report.txt", + "/home/alice2/report.txt", + "/tmp/home/alice/report.txt", + ] stdout = self._stream_json_for( - {"file_path": "/tmp/skill-comply-sandbox/t1/file.txt"}, "ok" + {"paths": outside_paths}, + [{"type": "text", "text": path} for path in outside_paths], ) events = _parse_stream_json(stdout) - assert "/tmp/skill-comply-sandbox/t1/file.txt" in events[0].input + + assert json.loads(events[0].input)["paths"] == outside_paths + assert [item["text"] for item in json.loads(events[0].output)] == outside_paths + + def test_list_output_redacts_nested_string_leaves( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + home = "/Users/reviewer" + self._set_home(monkeypatch, home) + output_content = [ + {"type": "text", "text": f"created {home}/résumé.txt"}, + {"type": "metadata", "paths": [home, f"{home}/資料.json"]}, + ] + stdout = self._stream_json_for({"file_path": "irrelevant"}, output_content) + events = _parse_stream_json(stdout) + + assert json.loads(events[0].output) == [ + {"type": "text", "text": "created ~/résumé.txt"}, + {"type": "metadata", "paths": ["~", "~/資料.json"]}, + ] + + def test_redacts_before_json_serialization_and_5000_character_truncation( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + home = "/home/alice" + self._set_home(monkeypatch, home) + boundary_value = "x" * 4977 + f" {home}/secret.txt" + "tail" * 20 + stdout = self._stream_json_for( + {"command": boundary_value}, + boundary_value, + ) + events = _parse_stream_json(stdout) + + assert len(events[0].input) == 5000 + assert "~/secret" in events[0].input + assert "/home/" not in events[0].input + assert len(events[0].output) == 5000 + assert "~/secret.txt" in events[0].output + assert "/home/" not in events[0].output class TestRunScenarioErrorIncludesStdoutTail: From 30c41a9bde3614d92fcc2ed5331d6198b6f613d6 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:42:59 -0400 Subject: [PATCH 33/46] fix: close truth and portability review gaps --- scripts/ci/validate-skills.js | 92 +++++++++++++------ scripts/lib/transcript-context.js | 11 ++- skills/skill-comply/pyproject.toml | 1 + skills/skill-comply/scripts/runner.py | 18 +++- skills/skill-comply/tests/test_runner.py | 22 +++++ skills/skill-stocktake/scripts/quick-diff.sh | 25 ++++- skills/skill-stocktake/scripts/scan.sh | 25 ++++- tests/ci/gateguard-env-documented.test.js | 16 +++- tests/ci/validators.test.js | 25 +++++ tests/hooks/suggest-compact.test.js | 5 +- tests/lib/transcript-context.test.js | 16 +++- .../scripts/skill-stocktake-discovery.test.js | 7 +- 12 files changed, 216 insertions(+), 47 deletions(-) diff --git a/scripts/ci/validate-skills.js b/scripts/ci/validate-skills.js index 1ae0e67ce..6e7c3a64b 100644 --- a/scripts/ci/validate-skills.js +++ b/scripts/ci/validate-skills.js @@ -68,9 +68,41 @@ function extractFrontmatter(content) { * @param {string[]} lines * @returns {{values: Record, descriptionIndicator: string|null}} */ +function stripUnquotedYamlComment(rawValue) { + let inSingleQuote = false; + let inDoubleQuote = false; + + for (let index = 0; index < rawValue.length; index++) { + const character = rawValue[index]; + + if (inDoubleQuote && character === '\\') { + index += 1; + continue; + } + if (!inDoubleQuote && character === "'") { + if (inSingleQuote && rawValue[index + 1] === "'") { + index += 1; + } else { + inSingleQuote = !inSingleQuote; + } + continue; + } + if (!inSingleQuote && character === '"') { + inDoubleQuote = !inDoubleQuote; + continue; + } + if (!inSingleQuote && !inDoubleQuote && character === '#' + && (index === 0 || /\s/.test(rawValue[index - 1]))) { + return rawValue.slice(0, index).trim(); + } + } + + return rawValue.trim(); +} + function inspectFrontmatter(lines) { const values = Object.create(null); - const syntaxErrors = []; + let syntaxErrors = []; let descriptionIndicator = null; let inBlockScalar = false; let blockScalarIndent = -1; @@ -92,13 +124,8 @@ function inspectFrontmatter(lines) { const key = match[1]; const rawValue = match[2]; - // Strip unquoted comments for value/indicator inspection. Handles both - // trailing comments (`foo: bar # note`) and comment-only values - // (`foo: # todo`) so the latter is treated as empty. - const valueNoComment = rawValue - .replace(/^\s*#.*$/, '') - .replace(/\s+#.*$/, '') - .trim(); + // Strip YAML comments only when # appears outside a quoted scalar. + const valueNoComment = stripUnquotedYamlComment(rawValue); values[key] = valueNoComment; const isQuoted = /^"(?:[^"\\]|\\.)*"$/.test(valueNoComment) || /^'(?:[^']|'')*'$/.test(valueNoComment); @@ -109,16 +136,19 @@ function inspectFrontmatter(lines) { // drops a value's quoting, or glues the next frontmatter key onto // the end of a value, this is exactly what shows up (see #2630). if (valueNoComment.includes(': ')) { - syntaxErrors.push( + syntaxErrors = [...syntaxErrors, `${key}: unquoted value contains ': ' — invalid YAML; ` + `quote the value or the next key was likely glued onto this line` - ); + ]; } // '@' and '`' are reserved YAML indicators and cannot start a // plain scalar (see #2630 — a reordering during translation moved // '@' into the first column of an unquoted description). if (/^[@`]/.test(valueNoComment)) { - syntaxErrors.push(`${key}: unquoted value starts with reserved character '${valueNoComment[0]}' — quote the value`); + syntaxErrors = [ + ...syntaxErrors, + `${key}: unquoted value starts with reserved character '${valueNoComment[0]}' — quote the value` + ]; } } @@ -246,30 +276,31 @@ function validateSkillFile(skillMd, label, reportFrontmatterFinding, opts = {}) function findDocsSkillFiles(docsDir) { if (!fs.existsSync(docsDir)) return []; - const files = []; - const locales = fs - .readdirSync(docsDir, { withFileTypes: true }) + const readDirectories = (directory, label) => { + try { + return fs.readdirSync(directory, { withFileTypes: true }); + } catch { + throw new Error(`unable to read ${label}`); + } + }; + + const locales = readDirectories(docsDir, 'docs directory') .filter(e => e.isDirectory() && !e.name.startsWith('.')) .map(e => e.name); - for (const locale of locales) { + return locales.flatMap(locale => { const localeSkillsDir = path.join(docsDir, locale, 'skills'); - if (!fs.existsSync(localeSkillsDir)) continue; + if (!fs.existsSync(localeSkillsDir)) return []; - const skillDirs = fs - .readdirSync(localeSkillsDir, { withFileTypes: true }) + const skillDirs = readDirectories(localeSkillsDir, `docs/${locale}/skills directory`) .filter(e => e.isDirectory() && !e.name.startsWith('.')) .map(e => e.name); - for (const skillDir of skillDirs) { - files.push({ - skillMd: path.join(localeSkillsDir, skillDir, 'SKILL.md'), - label: `docs/${locale}/skills/${skillDir}/SKILL.md` - }); - } - } - - return files; + return skillDirs.map(skillDir => ({ + skillMd: path.join(localeSkillsDir, skillDir, 'SKILL.md'), + label: `docs/${locale}/skills/${skillDir}/SKILL.md` + })); + }); } function validateSkills() { @@ -329,4 +360,9 @@ function validateSkills() { console.log(msg); } -validateSkills(); +try { + validateSkills(); +} catch (error) { + console.error(`ERROR: ${error.message}`); + process.exit(1); +} diff --git a/scripts/lib/transcript-context.js b/scripts/lib/transcript-context.js index d0a944330..853a35b2a 100644 --- a/scripts/lib/transcript-context.js +++ b/scripts/lib/transcript-context.js @@ -162,10 +162,11 @@ function readLatestContextTokens(transcriptPath, options = {}) { * positively detected or merely assumed. * * `inferred: false` means the size came from evidence — an explicit env - * override, the `[1m]` marker, a known large-window family, or an observed - * token count that already exceeds the standard window. `inferred: true` means - * every check fell through and the standard 200k default was assumed; the - * window may actually be larger and callers must not present it as fact. + * override, the `[1m]` marker, or a known large-window family. An observed + * token count above the standard window selects the safer large-window + * thresholds, but remains inferred because the true denominator could be an + * unmarked intermediate size such as 400k. Callers must not present inferred + * windows as fact. * * @returns {{ windowTokens: number, inferred: boolean }} */ @@ -193,7 +194,7 @@ function resolveContextWindow(tokens, model) { } if (Number.isFinite(tokens) && tokens > STANDARD_CONTEXT_WINDOW_TOKENS) { - return { windowTokens: LARGE_CONTEXT_WINDOW_TOKENS, inferred: false }; + return { windowTokens: LARGE_CONTEXT_WINDOW_TOKENS, inferred: true }; } return { windowTokens: STANDARD_CONTEXT_WINDOW_TOKENS, inferred: true }; diff --git a/skills/skill-comply/pyproject.toml b/skills/skill-comply/pyproject.toml index 323185cef..3584f8262 100644 --- a/skills/skill-comply/pyproject.toml +++ b/skills/skill-comply/pyproject.toml @@ -8,6 +8,7 @@ dependencies = ["pyyaml>=6.0"] [tool.pytest.ini_options] testpaths = ["tests"] pythonpath = ["."] +markers = ["unit: isolated tests without external services"] [dependency-groups] dev = [ diff --git a/skills/skill-comply/scripts/runner.py b/skills/skill-comply/scripts/runner.py index 4a852a57e..ffad8447f 100644 --- a/skills/skill-comply/scripts/runner.py +++ b/skills/skill-comply/scripts/runner.py @@ -149,11 +149,25 @@ def _redact_home_path(text: str) -> str: def _redact_home_paths(value: object) -> object: - """Return a copy with home paths redacted from every string leaf.""" + """Return a copy with home paths redacted from string keys and leaves. + + Redacted mapping keys receive a stable numeric suffix when two original + keys collapse to the same portable value. This preserves every observation + without leaking the original home path or silently dropping data. + """ if isinstance(value, str): return _redact_home_path(value) if isinstance(value, dict): - return {key: _redact_home_paths(item) for key, item in value.items()} + redacted: dict[object, object] = {} + for key, item in value.items(): + redacted_key = _redact_home_path(key) if isinstance(key, str) else key + candidate = redacted_key + suffix = 2 + while candidate in redacted: + candidate = f"{redacted_key}#{suffix}" + suffix += 1 + redacted[candidate] = _redact_home_paths(item) + return redacted if isinstance(value, list): return [_redact_home_paths(item) for item in value] return value diff --git a/skills/skill-comply/tests/test_runner.py b/skills/skill-comply/tests/test_runner.py index a45270fc1..f8141d184 100644 --- a/skills/skill-comply/tests/test_runner.py +++ b/skills/skill-comply/tests/test_runner.py @@ -144,6 +144,7 @@ class TestRunScenarioMaxTurnsTermination: run_scenario(scenario, model="haiku") +@pytest.mark.unit class TestParseStreamJsonRedactsHomePath: """Observations feed grade() and then a written report (results/.md) — a raw absolute path bakes the operator's username into every tool call @@ -209,6 +210,27 @@ class TestParseStreamJsonRedactsHomePath: "~/資料.txt", ] + def test_mapping_keys_are_redacted_without_silent_collision( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + home = r"C:\Users\Zoë" + self._set_home(monkeypatch, home) + stdout = self._stream_json_for( + { + home + r"\private.txt": "first", + r"c:\users\zoë\private.txt": "second", + }, + "irrelevant output", + ) + events = _parse_stream_json(stdout) + + parsed_input = json.loads(events[0].input) + assert home not in events[0].input + assert parsed_input == { + r"~\private.txt": "first", + r"~\private.txt#2": "second", + } + def test_sibling_and_embedded_prefix_paths_untouched( self, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/skills/skill-stocktake/scripts/quick-diff.sh b/skills/skill-stocktake/scripts/quick-diff.sh index f929b2432..418b02558 100755 --- a/skills/skill-stocktake/scripts/quick-diff.sh +++ b/skills/skill-stocktake/scripts/quick-diff.sh @@ -13,6 +13,27 @@ set -euo pipefail +sort_nul_file() { + local input_file="$1" + local sorted_file="${input_file}.sorted" + node -e ' + const fs = require("fs"); + const input = fs.readFileSync(0); + const records = []; + let start = 0; + for (let index = 0; index < input.length; index += 1) { + if (input[index] === 0) { + records.push(input.subarray(start, index + 1)); + start = index + 1; + } + } + if (start < input.length) records.push(input.subarray(start)); + records.sort(Buffer.compare); + process.stdout.write(Buffer.concat(records)); + ' <"$input_file" >"$sorted_file" + mv "$sorted_file" "$input_file" +} + RESULTS_JSON="${1:-}" CWD_SKILLS_DIR="${SKILL_STOCKTAKE_PROJECT_DIR:-${2:-$PWD/.claude/skills}}" GLOBAL_DIR="${SKILL_STOCKTAKE_GLOBAL_DIR:-$HOME/.claude/skills}" @@ -56,13 +77,13 @@ process_dir() { # Capture find's exit status and stderr instead of discarding them: with -L, # a broken symlink or unreadable directory makes find skip that entry AND # exit non-zero, which would otherwise silently under-count skills. - # NUL-delimited (-print0 / sort -z / read -d '') so a path containing a + # NUL-delimited (-print0 / sort_nul_file / read -d '') so a path containing a # literal newline can't desync record boundaries — paths here are untrusted. if ! find -L "$dir" -name "SKILL.md" -type f -print0 >"$find_out" 2>"$find_err"; then echo "Warning: find encountered errors while scanning $dir (broken symlinks or permission issues may cause skills to be missed):" >&2 cat "$find_err" >&2 fi - sort -z -o "$find_out" "$find_out" + sort_nul_file "$find_out" while IFS= read -r -d '' file; do local mtime dp is_new diff --git a/skills/skill-stocktake/scripts/scan.sh b/skills/skill-stocktake/scripts/scan.sh index 76f5523dc..e43509edc 100755 --- a/skills/skill-stocktake/scripts/scan.sh +++ b/skills/skill-stocktake/scripts/scan.sh @@ -13,6 +13,27 @@ set -euo pipefail +sort_nul_file() { + local input_file="$1" + local sorted_file="${input_file}.sorted" + node -e ' + const fs = require("fs"); + const input = fs.readFileSync(0); + const records = []; + let start = 0; + for (let index = 0; index < input.length; index += 1) { + if (input[index] === 0) { + records.push(input.subarray(start, index + 1)); + start = index + 1; + } + } + if (start < input.length) records.push(input.subarray(start)); + records.sort(Buffer.compare); + process.stdout.write(Buffer.concat(records)); + ' <"$input_file" >"$sorted_file" + mv "$sorted_file" "$input_file" +} + GLOBAL_DIR="${SKILL_STOCKTAKE_GLOBAL_DIR:-$HOME/.claude/skills}" CWD_SKILLS_DIR="${SKILL_STOCKTAKE_PROJECT_DIR:-${1:-$PWD/.claude/skills}}" # Path to JSONL file containing tool-use observations (optional; used for usage frequency counts). @@ -100,13 +121,13 @@ scan_dir_to_json() { # Capture find's exit status and stderr instead of discarding them: with -L, # a broken symlink or unreadable directory makes find skip that entry AND # exit non-zero, which would otherwise silently under-count skills. - # NUL-delimited (-print0 / sort -z / read -d '') so a path containing a + # NUL-delimited (-print0 / sort_nul_file / read -d '') so a path containing a # literal newline can't desync record boundaries — paths here are untrusted. if ! find -L "$dir" -name "SKILL.md" -type f -print0 >"$find_out" 2>"$find_err"; then echo "Warning: find encountered errors while scanning $dir (broken symlinks or permission issues may cause skills to be missed):" >&2 cat "$find_err" >&2 fi - sort -z -o "$find_out" "$find_out" + sort_nul_file "$find_out" while IFS= read -r -d '' file; do local name desc mtime u7 u30 dp diff --git a/tests/ci/gateguard-env-documented.test.js b/tests/ci/gateguard-env-documented.test.js index 6ee96754c..28400cbea 100644 --- a/tests/ci/gateguard-env-documented.test.js +++ b/tests/ci/gateguard-env-documented.test.js @@ -52,6 +52,15 @@ function test(name, fn) { const REGEX_CAN_FOLLOW = new Set([ '', '(', ',', '=', ':', '[', '!', '&', '|', '?', '{', '}', ';', '+', '-', '*', '%', '~', '^', '<', '>', ]); +const REGEX_CAN_FOLLOW_KEYWORD = new Set([ + 'await', 'case', 'delete', 'do', 'else', 'in', 'instanceof', 'new', 'of', + 'return', 'throw', 'typeof', 'void', 'yield', +]); + +function regexFollowsKeyword(source, slashIndex) { + const match = source.slice(0, slashIndex).match(/([A-Za-z_$][\w$]*)\s*$/); + return Boolean(match && REGEX_CAN_FOLLOW_KEYWORD.has(match[1])); +} /** * Blank out comments and literal text, preserving length and line breaks so @@ -102,7 +111,7 @@ function blankCommentsAndLiterals(source) { continue; } - if (ch === '/' && REGEX_CAN_FOLLOW.has(prev)) { + if (ch === '/' && (REGEX_CAN_FOLLOW.has(prev) || regexFollowsKeyword(source, i))) { emit(ch); i += 1; let inClass = false; while (i < source.length) { @@ -291,6 +300,11 @@ if (test('a regex literal containing a slash does not swallow the code after it' assert.deepStrictEqual([...readGateguardEnvNames(fixture)], ['GATEGUARD_AFTER_REGEX']); })) passed++; else failed++; +if (test('a regex literal after a statement keyword is ignored', () => { + const fixture = 'function matches() { return /process\\.env\\.GATEGUARD_IN_RETURN_REGEX/; }'; + assert.deepStrictEqual([...readGateguardEnvNames(fixture)], []); +})) passed++; else failed++; + if (test('the access guard rejects every form the parser cannot follow', () => { const cases = [ ['destructuring', 'const { GATEGUARD_HIDDEN } = process.env;'], diff --git a/tests/ci/validators.test.js b/tests/ci/validators.test.js index e6d950161..4bcb9452a 100644 --- a/tests/ci/validators.test.js +++ b/tests/ci/validators.test.js @@ -2856,6 +2856,31 @@ function runTests() { cleanupTestDir(testDir); })) passed++; else failed++; + if (test('preserves # inside a quoted frontmatter value', () => { + const testDir = createTestDir(); + const docsDir = path.join(testDir, 'docs-root'); + const skillDir = path.join(docsDir, 'ja-JP', 'skills', 'example'); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), + '---\nname: example\ndescription: "Fix: details #tag" # translation note\n---\n# Example'); + + const result = runSkillsValidator('/nonexistent/skills-dir', ['--strict'], {}, docsDir); + assert.strictEqual(result.code, 0, + `Quoted # content must remain valid, got stderr: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('reports an unreadable docs root deterministically', () => { + const testDir = createTestDir(); + const docsPath = path.join(testDir, 'docs-file'); + fs.writeFileSync(docsPath, 'not a directory'); + + const result = runSkillsValidator('/nonexistent/skills-dir', ['--strict'], {}, docsPath); + assert.strictEqual(result.code, 1, 'Should fail when the docs root cannot be read'); + assert.strictEqual(result.stderr.trim(), 'ERROR: unable to read docs directory'); + cleanupTestDir(testDir); + })) passed++; else failed++; + if (test('flags a docs mirror SKILL.md with no frontmatter block at all', () => { const testDir = createTestDir(); const docsDir = path.join(testDir, 'docs-root'); diff --git a/tests/hooks/suggest-compact.test.js b/tests/hooks/suggest-compact.test.js index 6036442d3..5b9d2324d 100644 --- a/tests/hooks/suggest-compact.test.js +++ b/tests/hooks/suggest-compact.test.js @@ -698,7 +698,10 @@ function runTests() { const ctx = createContextContext(); const transcript = writeTranscriptFixture(170000); try { - const result = runCompactWithInput({ session_id: ctx.sessionId, transcript_path: transcript }); + const result = runCompactWithInput( + { session_id: ctx.sessionId, transcript_path: transcript }, + { ECC_CONTEXT_WINDOW_TOKENS: '', CLAUDE_CODE_AUTO_COMPACT_WINDOW: '' }, + ); assert.strictEqual(result.code, 0, 'Should exit 0'); assert.ok(result.stdout.trim().length > 0, `Expected stdout payload. Got: "${result.stdout}"`); const parsed = JSON.parse(result.stdout); diff --git a/tests/lib/transcript-context.test.js b/tests/lib/transcript-context.test.js index f10f62c76..b5a3addf6 100644 --- a/tests/lib/transcript-context.test.js +++ b/tests/lib/transcript-context.test.js @@ -139,6 +139,10 @@ console.log('\nresolveContextWindowTokens:'); // Isolation: an env-set window override (either knob) otherwise leaks into the // default-window assertions below and fails them (#2290). +const originalContextWindowEnv = { + ECC_CONTEXT_WINDOW_TOKENS: process.env.ECC_CONTEXT_WINDOW_TOKENS, + CLAUDE_CODE_AUTO_COMPACT_WINDOW: process.env.CLAUDE_CODE_AUTO_COMPACT_WINDOW, +}; delete process.env.ECC_CONTEXT_WINDOW_TOKENS; delete process.env.CLAUDE_CODE_AUTO_COMPACT_WINDOW; @@ -222,9 +226,6 @@ test('treats an empty model id as standard window', () => { // ── isContextWindowInferred ── console.log('\nisContextWindowInferred:'); -delete process.env.ECC_CONTEXT_WINDOW_TOKENS; -delete process.env.CLAUDE_CODE_AUTO_COMPACT_WINDOW; - test('flags the assumed 200k default as inferred', () => { assert.strictEqual(isContextWindowInferred(187000, 'claude-opus-9'), true); }); @@ -246,10 +247,15 @@ test('a known large-window family is a detected window, not inferred', () => { assert.strictEqual(isContextWindowInferred(187000, 'claude-fable-5'), false); }); -test('tokens above the standard window make the size detected, not inferred', () => { - assert.strictEqual(isContextWindowInferred(220000, 'claude-opus-9'), false); +test('tokens above the standard window still leave the exact size inferred', () => { + assert.strictEqual(isContextWindowInferred(220000, 'claude-opus-9'), true); }); +for (const [name, value] of Object.entries(originalContextWindowEnv)) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; +} + // ── resolveContextThreshold ── console.log('\nresolveContextThreshold:'); diff --git a/tests/scripts/skill-stocktake-discovery.test.js b/tests/scripts/skill-stocktake-discovery.test.js index d1d36c0d5..498ed43fa 100644 --- a/tests/scripts/skill-stocktake-discovery.test.js +++ b/tests/scripts/skill-stocktake-discovery.test.js @@ -47,7 +47,9 @@ test('both scanners use canonical, error-visible, NUL-delimited discovery', () = for (const scriptPath of [scanScript, quickDiffScript]) { const source = fs.readFileSync(scriptPath, 'utf8'); assert.match(source, /find -L "\$dir" -name "SKILL\.md" -type f -print0/); - assert.match(source, /sort -z -o "\$find_out" "\$find_out"/); + assert.match(source, /sort_nul_file "\$find_out"/); + assert.match(source, /records\.sort\(Buffer\.compare\)/); + assert.doesNotMatch(source, /sort -z/, `${path.basename(scriptPath)} still requires GNU sort`); assert.match(source, /read -r -d '' file/); assert.doesNotMatch(source, /find [^\n]*2>\/dev\/null/, `${path.basename(scriptPath)} still hides find errors`); } @@ -103,6 +105,9 @@ if (process.platform === 'win32') { ); assert.ok(output.every(entry => entry.is_new === true)); }); + } catch (error) { + console.log(` ✗ fixture setup: ${error.message}`); + failed++; } finally { fs.rmSync(tempRoot, { recursive: true, force: true }); } From 6fa3efeef726ce8b57a961b4206a4c88a96734a5 Mon Sep 17 00:00:00 2001 From: Suliman Abdulrazzaq Date: Mon, 10 Aug 2026 20:51:00 +0300 Subject: [PATCH 34/46] fix(hooks): use valid wildcard matchers --- hooks/hooks.json | 32 ++++++++++++++++---------------- tests/hooks/hooks.test.js | 18 +++++++++++++++++- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/hooks/hooks.json b/hooks/hooks.json index 35d79fd5a..f1c82b515 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -36,7 +36,7 @@ "id": "pre:edit-write:suggest-compact" }, { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -73,7 +73,7 @@ "id": "pre:config-protection" }, { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -98,7 +98,7 @@ ], "PreCompact": [ { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -111,7 +111,7 @@ ], "SessionStart": [ { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -122,7 +122,7 @@ "id": "session:start" }, { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -135,7 +135,7 @@ ], "PostToolUse": [ { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -147,7 +147,7 @@ "id": "post:dispatcher:sync" }, { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -162,7 +162,7 @@ ], "PostToolUseFailure": [ { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -186,7 +186,7 @@ ], "Stop": [ { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -197,7 +197,7 @@ "id": "stop:plan-canvas-pending" }, { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -209,7 +209,7 @@ "id": "stop:format-typecheck" }, { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -220,7 +220,7 @@ "id": "stop:check-console-log" }, { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -233,7 +233,7 @@ "id": "stop:session-end" }, { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -246,7 +246,7 @@ "id": "stop:evaluate-session" }, { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -259,7 +259,7 @@ "id": "stop:cost-tracker" }, { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", @@ -274,7 +274,7 @@ ], "SessionEnd": [ { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index 746aa88f7..ce3411b15 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -2585,7 +2585,7 @@ async function runTests() { ['post:dispatcher:sync', 'post:dispatcher:async'], 'PostToolUse should have one sync and one async dispatcher' ); - assert.ok(postEntries.every(entry => entry.matcher === '*')); + assert.ok(postEntries.every(entry => entry.matcher === '.*')); const preCommand = Array.isArray(preBash[0].hooks[0].command) ? preBash[0].hooks[0].command.join(' ') : preBash[0].hooks[0].command; @@ -2599,6 +2599,22 @@ async function runTests() { passed++; else failed++; + if ( + test('all string hook matchers are valid regular expressions', () => { + const hooksPath = path.join(__dirname, '..', '..', 'hooks', 'hooks.json'); + const hooks = JSON.parse(fs.readFileSync(hooksPath, 'utf8')); + + for (const [eventName, hookArray] of Object.entries(hooks.hooks)) { + for (const entry of hookArray) { + if (typeof entry.matcher !== 'string') continue; + assert.doesNotThrow(() => new RegExp(entry.matcher), `${eventName}/${entry.id || 'hook'} should use a valid regex matcher`); + } + } + }) + ) + passed++; + else failed++; + if ( test('SessionEnd marker hook is async and cleanup-safe', () => { const hooksPath = path.join(__dirname, '..', '..', 'hooks', 'hooks.json'); From 962380c452b9e507c5e894b20bb6eda555346b91 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Fri, 28 Aug 2026 05:03:12 +0800 Subject: [PATCH 35/46] fix: ignore heredoc prose in GateGuard --- scripts/hooks/gateguard-fact-force.js | 169 ++++++++++++- tests/hooks/gateguard-fact-force.test.js | 291 +++++++++++++++++++++++ 2 files changed, 457 insertions(+), 3 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 3f7f9ed80..2cd852a93 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -151,6 +151,168 @@ function stripQuotedStrings(input) { return input.replace(/'(?:[^'\\]|\\.)*'/g, "''").replace(/"(?:[^"\\]|\\.)*"/g, '""'); } +/** + * Find simple heredoc redirections on one complete shell command line. + * Anything ambiguous is rejected so the caller can fail closed and run the + * destructive checks against the original input. Supported delimiters are + * shell identifiers, either unquoted or wholly single/double quoted. + * + * @param {string} line + * @returns {{ delimiter: string, quoted: boolean, stripTabs: boolean }[] | null} + */ +function findHeredocs(line) { + const heredocs = []; + let quote = null; + let escaped = false; + + for (let i = 0; i < line.length; i += 1) { + const ch = line[i]; + if (escaped) { + escaped = false; + continue; + } + if (ch === '\\') { + escaped = true; + continue; + } + if (quote) { + if (ch === quote) quote = null; + continue; + } + if (ch === '"' || ch === "'") { + quote = ch; + continue; + } + if ((ch === '$' && line[i + 1] === '(' && line[i + 2] === '(') || (ch === '(' && line[i + 1] === '(')) { + // Arithmetic syntax also uses `<<`. Treat the complete input + // conservatively instead of trying to parse nested arithmetic here. + return null; + } + if (ch === '$' && line[i + 1] === '[') return null; + if (ch === '#' && (i === 0 || /[\s;&|()]/.test(line[i - 1]))) { + break; + } + if (ch !== '<' || line[i + 1] !== '<' || line[i + 2] === '<') { + continue; + } + + // `<<` is also an operator inside arithmetic and [[ ... ]] expressions. + // A partial shell parser cannot distinguish every nested form safely. + const prefix = line.slice(0, i); + if (prefix.includes('((') || prefix.includes('[[')) return null; + + i += 2; + const stripTabs = line[i] === '-'; + if (stripTabs) i += 1; + while (i < line.length && /[ \t]/.test(line[i])) i += 1; + + let delimiter = ''; + let quoted = false; + const delimiterQuote = line[i] === '"' || line[i] === "'" ? line[i] : null; + if (delimiterQuote) { + quoted = true; + const endQuote = line.indexOf(delimiterQuote, i + 1); + if (endQuote < 0) return null; + delimiter = line.slice(i + 1, endQuote); + i = endQuote; + } else { + const match = line.slice(i).match(/^[A-Za-z_][A-Za-z0-9_]*/); + if (!match) return null; + delimiter = match[0]; + i += delimiter.length - 1; + } + + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(delimiter)) return null; + const next = line[i + 1]; + if (next && !/[\s;&|<>()]/.test(next)) return null; + heredocs.push({ delimiter, quoted, stripTabs }); + } + + return quote || escaped ? null : heredocs; +} + +/** + * Extract executable substitutions from an unquoted heredoc. Quote characters + * in its payload are literal and do not suppress expansion, so each unescaped + * `$(` or backtick is parsed from its own position rather than by feeding the + * complete payload through normal shell quote handling. + * + * @param {string[]} body + * @returns {string[]} + */ +function extractHeredocCommandSubstitutions(body) { + const text = body.join('\n'); + const substitutions = new Set(); + let escaped = false; + for (let i = 0; i < text.length; i += 1) { + const ch = text[i]; + if (escaped) { + escaped = false; + continue; + } + if (ch === '\\') { + escaped = true; + continue; + } + if (ch === '`' || (ch === '$' && text[i + 1] === '(')) { + for (const substitution of extractCommandSubstitutions(text.slice(i))) { + substitutions.add(substitution); + } + } + } + return [...substitutions]; +} + +/** + * Remove heredoc payload text before classifying the surrounding shell + * command. Prose in a heredoc is data, so matching it as a command produces + * false positives. Unquoted heredocs can still execute `$()` and backtick + * substitutions; retain the complete payload whenever either syntax appears. + * Quoted heredoc delimiters disable expansion, so their payload is fully inert. + * Ambiguous shell syntax returns the original input unchanged (fail closed). + * + * @param {string} input + * @returns {string} + */ +function stripHeredocBodies(input) { + const raw = String(input || ''); + const kept = []; + const pending = []; + + for (const line of raw.split(/\r?\n/)) { + if (pending.length > 0) { + const current = pending[0]; + // Bash removes backslash-newline pairs in an unquoted heredoc before + // comparing delimiters. Preserve the original input when physical lines + // can be joined into a terminator or executable expansion. + if (!current.quoted && /\\$/.test(line)) return raw; + const delimiterLine = current.stripTabs ? line.replace(/^\t+/, '') : line; + if (delimiterLine === current.delimiter) { + if (!current.quoted) { + kept.push(...extractHeredocCommandSubstitutions(current.body)); + } + pending.shift(); + } else { + current.body.push(line); + } + continue; + } + + kept.push(line); + const heredocs = findHeredocs(line); + if (heredocs === null) return raw; + pending.push(...heredocs.map(heredoc => ({ ...heredoc, body: [] }))); + } + + for (const current of pending) { + if (!current.quoted) { + kept.push(...extractHeredocCommandSubstitutions(current.body)); + } + } + + return kept.join('\n'); +} + /** * Promote subshell delimiters to top-level segment separators so the * destructive check applies inside `$(...)` and backtick subshells. @@ -672,7 +834,8 @@ function isDestructiveBash(command) { // after quoting AND subshell delimiters are normalized so phrases // inside `$(...)` or backticks are also caught. const raw = String(command || ''); - const flattened = explodeSubshells(stripQuotedStrings(raw)); + const executable = stripHeredocBodies(raw); + const flattened = explodeSubshells(stripQuotedStrings(executable)); if (DESTRUCTIVE_SQL_DD.test(flattened)) return true; // Operator-supplied additional destructive patterns. Same scope as the @@ -687,7 +850,7 @@ function isDestructiveBash(command) { // isDestructiveFindExec would turn `find . -exec 'rm' {} \;` into `find . -exec {} \;` // — the binary name disappears and the check returns false. Using raw body text avoids // that false-negative while also catching `&&`, `;`, `|`, and `||` compound forms. - const bodies = collectExecutableBodies(raw); + const bodies = collectExecutableBodies(executable); for (const body of bodies) { for (const rawSeg of body .split(/[;|&]+/) @@ -709,7 +872,7 @@ function isDestructiveBash(command) { // Quote-aware pass: closes the quoted-command-word, newline-separator, // quoted-find-exec, and sh/bash -c bypasses (GHSA-4v57-ph3x-gf55). - if (isDestructiveQuoteAware(raw)) return true; + if (isDestructiveQuoteAware(executable)) return true; return false; } diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 8912eb994..477fa28f8 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1477,6 +1477,297 @@ function runTests() { passed++; else failed++; + if ( + test('allows destructive SQL prose inside a quoted heredoc', () => { + expectAllow( + [ + "cat > migration-notes.md <<'EOF'", + 'This migration will DROP TABLE old_sessions after verification.', + 'EOF' + ].join('\n'), + 'quoted heredoc SQL prose' + ); + }) + ) + passed++; + else failed++; + + if ( + test('allows destructive prose and separators inside an unquoted heredoc', () => { + expectAllow( + [ + 'cat > migration-notes.md < { + expectAllow( + [ + 'cat > migration-notes.md <<-EOF', + '\tTRUNCATE old_sessions; rm -rf old-cache', + '\tEOF' + ].join('\n'), + 'tab-stripping heredoc prose' + ); + }) + ) + passed++; + else failed++; + + if ( + test('still denies destructive commands after a heredoc terminator', () => { + expectDestructiveDeny( + [ + "cat > migration-notes.md <<'EOF'", + 'DROP TABLE is documentation here.', + 'EOF', + 'rm -rf /tmp/real-target' + ].join('\n'), + 'command after heredoc terminator' + ); + }) + ) + passed++; + else failed++; + + if ( + test('still denies command substitutions inside an unquoted heredoc', () => { + expectDestructiveDeny( + [ + 'cat > output.txt < { + expectAllow( + [ + "cat > example.md <<'EOF'", + '$(rm -rf /tmp/example-only)', + 'EOF' + ].join('\n'), + 'quoted heredoc command-substitution prose' + ); + }) + ) + passed++; + else failed++; + + if ( + test('does not mistake an arithmetic shift for a heredoc', () => { + expectDestructiveDeny( + ['echo $((1 << 2))', 'rm -rf /tmp/real-target'].join('\n'), + 'command after arithmetic shift' + ); + }) + ) + passed++; + else failed++; + + if ( + test('does not mistake a named arithmetic shift operand for a heredoc', () => { + expectDestructiveDeny( + ['echo $((flags << WIDTH))', 'rm -rf /tmp/real-target'].join('\n'), + 'command after named arithmetic shift' + ); + }) + ) + passed++; + else failed++; + + if ( + test('fails closed on multiline arithmetic shift contexts', () => { + for (const arithmetic of [ + ['((', 'flags << WIDTH', '))'], + ['$((', 'flags << WIDTH', '))'], + ['$[', 'flags << WIDTH', ']'] + ]) { + expectDestructiveDeny( + [...arithmetic, 'rm -rf /tmp/real-target'].join('\n'), + 'command after multiline arithmetic shift' + ); + } + }) + ) + passed++; + else failed++; + + if ( + test('does not mistake a conditional string operator for a heredoc', () => { + expectDestructiveDeny( + ['[[ alpha << omega ]]', 'rm -rf /tmp/real-target'].join('\n'), + 'command after conditional shift-like operator' + ); + }) + ) + passed++; + else failed++; + + if ( + test('does not parse heredocs inside operator-adjacent comments', () => { + expectDestructiveDeny( + ['true;# < { + expectDestructiveDeny( + ['printf \'%s\' "literal', '< { + expectDestructiveDeny( + ["cat <<$'EOF'", 'documentation', 'EOF', 'rm -rf /tmp/real-target'].join('\n'), + 'command after ANSI-C heredoc' + ); + }) + ) + passed++; + else failed++; + + if ( + test('fails closed on escaped heredoc delimiter words', () => { + expectDestructiveDeny( + ['cat < { + expectDestructiveDeny( + ['cat < { + expectDestructiveDeny( + ['cat < { + expectDestructiveDeny( + ['cat < { + expectDestructiveDeny( + ['cat < { + expectAllow( + ['cat < { + for (const payload of [ + "'$(rm -rf /tmp/expanded-target)'", + '"$(rm -rf /tmp/expanded-target)"', + "'`rm -rf /tmp/expanded-target`'" + ]) { + expectDestructiveDeny( + ['cat < { + expectAllow( + ['cat < { + expectDestructiveDeny( + ['echo $((1 << 2))', 'rm -rf /tmp/shift-target'].join('\n'), + 'command after $((...)) arithmetic shift' + ); + expectDestructiveDeny( + ['echo $((x << 2))', 'rm -rf /tmp/shift-target'].join('\n'), + 'command after $((...)) identifier shift' + ); + expectDestructiveDeny( + ['(( 1 << 2 ))', 'rm -rf /tmp/shift-target'].join('\n'), + 'command after ((...)) arithmetic shift' + ); + expectDestructiveDeny( + ['echo $[x << 1]', 'rm -rf /tmp/shift-target'].join('\n'), + 'command after legacy $[...] arithmetic shift' + ); + }) + ) + passed++; + else failed++; + if ( test('allows git push --force-if-includes as a safety-checked variant', () => { expectAllow('git push --force-with-lease --force-if-includes origin main', 'git push --force-if-includes'); From e72191ba74085a440fd2bd5023e210a94d5da70f Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Fri, 28 Aug 2026 06:56:00 +0800 Subject: [PATCH 36/46] fix: harden heredoc command filtering --- scripts/hooks/gateguard-fact-force.js | 163 +-------------------- scripts/hooks/gateguard-heredoc.js | 172 +++++++++++++++++++++++ tests/hooks/gateguard-fact-force.test.js | 65 +++++++++ 3 files changed, 238 insertions(+), 162 deletions(-) create mode 100644 scripts/hooks/gateguard-heredoc.js diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 2cd852a93..203092d64 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -26,6 +26,7 @@ const crypto = require('crypto'); const fs = require('fs'); const path = require('path'); const { extractCommandSubstitutions, extractSubshellGroups, extractBraceGroups } = require('../lib/shell-substitution'); +const { stripHeredocBodies } = require('./gateguard-heredoc'); // Session state — scoped per session to avoid cross-session races. const STATE_DIR = process.env.GATEGUARD_STATE_DIR || path.join(process.env.HOME || process.env.USERPROFILE || '/tmp', '.gateguard'); @@ -151,168 +152,6 @@ function stripQuotedStrings(input) { return input.replace(/'(?:[^'\\]|\\.)*'/g, "''").replace(/"(?:[^"\\]|\\.)*"/g, '""'); } -/** - * Find simple heredoc redirections on one complete shell command line. - * Anything ambiguous is rejected so the caller can fail closed and run the - * destructive checks against the original input. Supported delimiters are - * shell identifiers, either unquoted or wholly single/double quoted. - * - * @param {string} line - * @returns {{ delimiter: string, quoted: boolean, stripTabs: boolean }[] | null} - */ -function findHeredocs(line) { - const heredocs = []; - let quote = null; - let escaped = false; - - for (let i = 0; i < line.length; i += 1) { - const ch = line[i]; - if (escaped) { - escaped = false; - continue; - } - if (ch === '\\') { - escaped = true; - continue; - } - if (quote) { - if (ch === quote) quote = null; - continue; - } - if (ch === '"' || ch === "'") { - quote = ch; - continue; - } - if ((ch === '$' && line[i + 1] === '(' && line[i + 2] === '(') || (ch === '(' && line[i + 1] === '(')) { - // Arithmetic syntax also uses `<<`. Treat the complete input - // conservatively instead of trying to parse nested arithmetic here. - return null; - } - if (ch === '$' && line[i + 1] === '[') return null; - if (ch === '#' && (i === 0 || /[\s;&|()]/.test(line[i - 1]))) { - break; - } - if (ch !== '<' || line[i + 1] !== '<' || line[i + 2] === '<') { - continue; - } - - // `<<` is also an operator inside arithmetic and [[ ... ]] expressions. - // A partial shell parser cannot distinguish every nested form safely. - const prefix = line.slice(0, i); - if (prefix.includes('((') || prefix.includes('[[')) return null; - - i += 2; - const stripTabs = line[i] === '-'; - if (stripTabs) i += 1; - while (i < line.length && /[ \t]/.test(line[i])) i += 1; - - let delimiter = ''; - let quoted = false; - const delimiterQuote = line[i] === '"' || line[i] === "'" ? line[i] : null; - if (delimiterQuote) { - quoted = true; - const endQuote = line.indexOf(delimiterQuote, i + 1); - if (endQuote < 0) return null; - delimiter = line.slice(i + 1, endQuote); - i = endQuote; - } else { - const match = line.slice(i).match(/^[A-Za-z_][A-Za-z0-9_]*/); - if (!match) return null; - delimiter = match[0]; - i += delimiter.length - 1; - } - - if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(delimiter)) return null; - const next = line[i + 1]; - if (next && !/[\s;&|<>()]/.test(next)) return null; - heredocs.push({ delimiter, quoted, stripTabs }); - } - - return quote || escaped ? null : heredocs; -} - -/** - * Extract executable substitutions from an unquoted heredoc. Quote characters - * in its payload are literal and do not suppress expansion, so each unescaped - * `$(` or backtick is parsed from its own position rather than by feeding the - * complete payload through normal shell quote handling. - * - * @param {string[]} body - * @returns {string[]} - */ -function extractHeredocCommandSubstitutions(body) { - const text = body.join('\n'); - const substitutions = new Set(); - let escaped = false; - for (let i = 0; i < text.length; i += 1) { - const ch = text[i]; - if (escaped) { - escaped = false; - continue; - } - if (ch === '\\') { - escaped = true; - continue; - } - if (ch === '`' || (ch === '$' && text[i + 1] === '(')) { - for (const substitution of extractCommandSubstitutions(text.slice(i))) { - substitutions.add(substitution); - } - } - } - return [...substitutions]; -} - -/** - * Remove heredoc payload text before classifying the surrounding shell - * command. Prose in a heredoc is data, so matching it as a command produces - * false positives. Unquoted heredocs can still execute `$()` and backtick - * substitutions; retain the complete payload whenever either syntax appears. - * Quoted heredoc delimiters disable expansion, so their payload is fully inert. - * Ambiguous shell syntax returns the original input unchanged (fail closed). - * - * @param {string} input - * @returns {string} - */ -function stripHeredocBodies(input) { - const raw = String(input || ''); - const kept = []; - const pending = []; - - for (const line of raw.split(/\r?\n/)) { - if (pending.length > 0) { - const current = pending[0]; - // Bash removes backslash-newline pairs in an unquoted heredoc before - // comparing delimiters. Preserve the original input when physical lines - // can be joined into a terminator or executable expansion. - if (!current.quoted && /\\$/.test(line)) return raw; - const delimiterLine = current.stripTabs ? line.replace(/^\t+/, '') : line; - if (delimiterLine === current.delimiter) { - if (!current.quoted) { - kept.push(...extractHeredocCommandSubstitutions(current.body)); - } - pending.shift(); - } else { - current.body.push(line); - } - continue; - } - - kept.push(line); - const heredocs = findHeredocs(line); - if (heredocs === null) return raw; - pending.push(...heredocs.map(heredoc => ({ ...heredoc, body: [] }))); - } - - for (const current of pending) { - if (!current.quoted) { - kept.push(...extractHeredocCommandSubstitutions(current.body)); - } - } - - return kept.join('\n'); -} - /** * Promote subshell delimiters to top-level segment separators so the * destructive check applies inside `$(...)` and backtick subshells. diff --git a/scripts/hooks/gateguard-heredoc.js b/scripts/hooks/gateguard-heredoc.js new file mode 100644 index 000000000..57cd6f459 --- /dev/null +++ b/scripts/hooks/gateguard-heredoc.js @@ -0,0 +1,172 @@ +'use strict'; + +const { extractCommandSubstitutions } = require('../lib/shell-substitution'); + +/** + * Recognize the deliberately narrow passive sink supported by this parser. + * Shell operators and substitutions make the payload's destination ambiguous, + * so every other form retains the original input for fail-closed checks. + * + * @param {string} line + * @returns {boolean} + */ +function isProvenPassiveHeredocLine(line) { + const trimmed = line.trim(); + return /^cat(?=\s|[<>])/.test(trimmed) && !/[;&|()`]/.test(trimmed); +} + +/** + * Parse a heredoc delimiter after a verified `<<` operator. + * + * @param {string} line + * @param {number} operatorIndex + * @returns {{ heredoc: { delimiter: string, quoted: boolean, stripTabs: boolean }, endIndex: number } | null} + */ +function parseHeredocDelimiter(line, operatorIndex) { + let endIndex = operatorIndex + 2; + const stripTabs = line[endIndex] === '-'; + if (stripTabs) endIndex += 1; + while (endIndex < line.length && /[ \t]/.test(line[endIndex])) endIndex += 1; + + let delimiter = ''; + let quoted = false; + const delimiterQuote = line[endIndex] === '"' || line[endIndex] === "'" ? line[endIndex] : null; + if (delimiterQuote) { + quoted = true; + const closingQuote = line.indexOf(delimiterQuote, endIndex + 1); + if (closingQuote < 0) return null; + delimiter = line.slice(endIndex + 1, closingQuote); + endIndex = closingQuote; + } else { + const match = line.slice(endIndex).match(/^[A-Za-z_][A-Za-z0-9_]*/); + if (!match) return null; + delimiter = match[0]; + endIndex += delimiter.length - 1; + } + + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(delimiter)) return null; + const next = line[endIndex + 1]; + if (next && !/[\s;&|<>()]/.test(next)) return null; + return { heredoc: { delimiter, quoted, stripTabs }, endIndex }; +} + +/** + * Find simple heredoc redirections on one complete shell command line. + * Anything ambiguous returns null so the caller can fail closed. + * + * @param {string} line + * @returns {{ delimiter: string, quoted: boolean, stripTabs: boolean }[] | null} + */ +function findHeredocs(line) { + const heredocs = []; + let quote = null; + let escaped = false; + for (let i = 0; i < line.length; i += 1) { + const ch = line[i]; + if (quote === "'") { + if (ch === "'") quote = null; + continue; + } + if (escaped) { + escaped = false; + continue; + } + if (ch === '\\') { + escaped = true; + continue; + } + if (quote === '"') { + if (ch === quote) quote = null; + continue; + } + if (ch === '"' || ch === "'") { + quote = ch; + continue; + } + if ((ch === '$' && line[i + 1] === '(' && line[i + 2] === '(') || (ch === '(' && line[i + 1] === '(')) return null; + if (ch === '$' && line[i + 1] === '[') return null; + if (ch === '#' && (i === 0 || /[\s;&|()]/.test(line[i - 1]))) break; + if (ch !== '<' || line[i + 1] !== '<') continue; + if (line[i + 2] === '<') return null; + const prefix = line.slice(0, i); + if (prefix.includes('((') || prefix.includes('[[')) return null; + const parsed = parseHeredocDelimiter(line, i); + if (!parsed) return null; + heredocs.push(parsed.heredoc); + i = parsed.endIndex; + } + return quote || escaped ? null : heredocs; +} + +/** + * Extract executable substitutions from an unquoted heredoc. Quote characters + * in its payload are literal and do not suppress expansion. + * + * @param {string[]} body + * @returns {string[]} + */ +function extractHeredocCommandSubstitutions(body) { + const text = body.join('\n'); + const substitutions = new Set(); + let escaped = false; + for (let i = 0; i < text.length; i += 1) { + const ch = text[i]; + if (escaped) { + escaped = false; + continue; + } + if (ch === '\\') { + escaped = true; + continue; + } + if (ch === '`' || (ch === '$' && text[i + 1] === '(')) { + for (const substitution of extractCommandSubstitutions(text.slice(i))) { + substitutions.add(substitution); + } + } + } + return [...substitutions]; +} + +/** + * Remove heredoc payload text before classifying the surrounding shell + * command. Prose in a heredoc is data, so matching it as a command produces + * false positives. Unquoted heredocs can still execute `$()` and backtick + * substitutions; retain only those substitution bodies for classification and + * drop the remaining payload text. Quoted heredoc payloads are fully inert. + * Ambiguous shell syntax returns the original input unchanged (fail closed). + * + * @param {string} input + * @returns {string} + */ +function stripHeredocBodies(input) { + const raw = String(input || ''); + const kept = []; + const pending = []; + let completedHeredoc = false; + for (const line of raw.split(/\r?\n/)) { + if (pending.length > 0) { + const current = pending[0]; + if (!current.quoted && /\\$/.test(line)) return raw; + const delimiterLine = current.stripTabs ? line.replace(/^\t+/, '') : line; + if (delimiterLine === current.delimiter) { + if (!current.quoted) kept.push(...extractHeredocCommandSubstitutions(current.body)); + pending.shift(); + if (pending.length === 0) completedHeredoc = true; + } else { + current.body.push(line); + } + continue; + } + if (completedHeredoc && line.trim()) return raw; + kept.push(line); + const heredocs = findHeredocs(line); + if (heredocs === null) return raw; + if (heredocs.length > 0 && !isProvenPassiveHeredocLine(line)) return raw; + pending.push(...heredocs.map(heredoc => ({ ...heredoc, body: [] }))); + } + if (pending.length > 0) return raw; + return kept.join('\n'); +} + +module.exports = { stripHeredocBodies }; diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 477fa28f8..b5b18cf8c 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1522,6 +1522,71 @@ function runTests() { passed++; else failed++; + if ( + test('handles multiple heredoc redirections in declaration order', () => { + expectAllow( + [ + "cat < { + for (const command of [ + ['bash < /tmp/review-script <<'EOF'", 'rm -rf /tmp/persisted-target', 'EOF', 'bash /tmp/review-script'].join('\n') + ]) { + expectDestructiveDeny(command, 'shell-executed heredoc payload'); + } + }) + ) + passed++; + else failed++; + + if ( + test('does not rescan a here-string as a heredoc', () => { + expectDestructiveDeny( + ['cat << { + expectDestructiveDeny( + ["echo 'a\\'X'< { + expectDestructiveDeny( + ['cat < { expectDestructiveDeny( From 9a3ee6864a5d037d44ecf85ae01b8a8ef3e92421 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Fri, 28 Aug 2026 08:43:32 +0800 Subject: [PATCH 37/46] refactor: keep heredoc parser state immutable --- scripts/hooks/gateguard-heredoc.js | 137 +++++++++++++++++++++-------- 1 file changed, 100 insertions(+), 37 deletions(-) diff --git a/scripts/hooks/gateguard-heredoc.js b/scripts/hooks/gateguard-heredoc.js index 57cd6f459..76de206f3 100644 --- a/scripts/hooks/gateguard-heredoc.js +++ b/scripts/hooks/gateguard-heredoc.js @@ -51,14 +51,13 @@ function parseHeredocDelimiter(line, operatorIndex) { } /** - * Find simple heredoc redirections on one complete shell command line. - * Anything ambiguous returns null so the caller can fail closed. + * Iterate over simple heredoc redirections on one complete shell command line. + * A null item marks ambiguous syntax so the caller can fail closed. * * @param {string} line - * @returns {{ delimiter: string, quoted: boolean, stripTabs: boolean }[] | null} + * @returns {Generator<{ delimiter: string, quoted: boolean, stripTabs: boolean } | null>} */ -function findHeredocs(line) { - const heredocs = []; +function* iterateHeredocs(line) { let quote = null; let escaped = false; for (let i = 0; i < line.length; i += 1) { @@ -83,31 +82,55 @@ function findHeredocs(line) { quote = ch; continue; } - if ((ch === '$' && line[i + 1] === '(' && line[i + 2] === '(') || (ch === '(' && line[i + 1] === '(')) return null; - if (ch === '$' && line[i + 1] === '[') return null; + if ((ch === '$' && line[i + 1] === '(' && line[i + 2] === '(') || (ch === '(' && line[i + 1] === '(')) { + yield null; + return; + } + if (ch === '$' && line[i + 1] === '[') { + yield null; + return; + } if (ch === '#' && (i === 0 || /[\s;&|()]/.test(line[i - 1]))) break; if (ch !== '<' || line[i + 1] !== '<') continue; - if (line[i + 2] === '<') return null; + if (line[i + 2] === '<') { + yield null; + return; + } const prefix = line.slice(0, i); - if (prefix.includes('((') || prefix.includes('[[')) return null; + if (prefix.includes('((') || prefix.includes('[[')) { + yield null; + return; + } const parsed = parseHeredocDelimiter(line, i); - if (!parsed) return null; - heredocs.push(parsed.heredoc); + if (!parsed) { + yield null; + return; + } + yield parsed.heredoc; i = parsed.endIndex; } - return quote || escaped ? null : heredocs; + if (quote || escaped) yield null; } /** - * Extract executable substitutions from an unquoted heredoc. Quote characters - * in its payload are literal and do not suppress expansion. + * Find simple heredoc redirections on one complete shell command line. + * Anything ambiguous returns null so the caller can fail closed. * - * @param {string[]} body - * @returns {string[]} + * @param {string} line + * @returns {{ delimiter: string, quoted: boolean, stripTabs: boolean }[] | null} */ -function extractHeredocCommandSubstitutions(body) { - const text = body.join('\n'); - const substitutions = new Set(); +function findHeredocs(line) { + const heredocs = [...iterateHeredocs(line)]; + return heredocs.includes(null) ? null : heredocs; +} + +/** + * Iterate over executable substitutions in an unquoted heredoc. + * + * @param {string} text + * @returns {Generator} + */ +function* iterateHeredocCommandSubstitutions(text) { let escaped = false; for (let i = 0; i < text.length; i += 1) { const ch = text[i]; @@ -120,12 +143,21 @@ function extractHeredocCommandSubstitutions(body) { continue; } if (ch === '`' || (ch === '$' && text[i + 1] === '(')) { - for (const substitution of extractCommandSubstitutions(text.slice(i))) { - substitutions.add(substitution); - } + yield* extractCommandSubstitutions(text.slice(i)); } } - return [...substitutions]; +} + +/** + * Extract executable substitutions from an unquoted heredoc. Quote characters + * in its payload are literal and do not suppress expansion. + * + * @param {string[]} body + * @returns {string[]} + */ +function extractHeredocCommandSubstitutions(body) { + const text = body.join('\n'); + return [...new Set(iterateHeredocCommandSubstitutions(text))]; } /** @@ -141,32 +173,63 @@ function extractHeredocCommandSubstitutions(body) { */ function stripHeredocBodies(input) { const raw = String(input || ''); - const kept = []; - const pending = []; + const lines = raw.split(/\r?\n/); + let pending = []; + let pendingIndex = 0; + let bodyStartIndex = -1; + let headerIndex = -1; + let trailingStartIndex = lines.length; + let substitutionText = ''; + let substitutionCount = 0; let completedHeredoc = false; - for (const line of raw.split(/\r?\n/)) { - if (pending.length > 0) { - const current = pending[0]; + for (let lineIndex = 0; lineIndex < lines.length; lineIndex += 1) { + const line = lines[lineIndex]; + if (pendingIndex < pending.length) { + const current = pending[pendingIndex]; if (!current.quoted && /\\$/.test(line)) return raw; const delimiterLine = current.stripTabs ? line.replace(/^\t+/, '') : line; if (delimiterLine === current.delimiter) { - if (!current.quoted) kept.push(...extractHeredocCommandSubstitutions(current.body)); - pending.shift(); - if (pending.length === 0) completedHeredoc = true; - } else { - current.body.push(line); + if (!current.quoted) { + for (const substitution of extractHeredocCommandSubstitutions(lines.slice(bodyStartIndex, lineIndex))) { + substitutionText = substitutionCount === 0 ? substitution : `${substitutionText}\n${substitution}`; + substitutionCount += 1; + } + } + pendingIndex += 1; + bodyStartIndex = lineIndex + 1; + if (pendingIndex === pending.length) { + completedHeredoc = true; + trailingStartIndex = lineIndex + 1; + } } continue; } if (completedHeredoc && line.trim()) return raw; - kept.push(line); + if (completedHeredoc) continue; const heredocs = findHeredocs(line); if (heredocs === null) return raw; if (heredocs.length > 0 && !isProvenPassiveHeredocLine(line)) return raw; - pending.push(...heredocs.map(heredoc => ({ ...heredoc, body: [] }))); + if (heredocs.length > 0) { + pending = heredocs; + pendingIndex = 0; + bodyStartIndex = lineIndex + 1; + headerIndex = lineIndex; + } } - if (pending.length > 0) return raw; - return kept.join('\n'); + if (pendingIndex < pending.length) return raw; + if (headerIndex < 0) return lines.join('\n'); + + const prefix = lines.slice(0, headerIndex + 1).join('\n'); + const trailingCount = lines.length - trailingStartIndex; + const trailing = lines.slice(trailingStartIndex).join('\n'); + let result = prefix; + let resultCount = headerIndex + 1; + if (substitutionCount > 0) { + result = resultCount === 0 ? substitutionText : `${result}\n${substitutionText}`; + resultCount += substitutionCount; + } + if (trailingCount > 0) result = resultCount === 0 ? trailing : `${result}\n${trailing}`; + return result; } module.exports = { stripHeredocBodies }; From 9768c075c313298364a3d81f67e60bea7d180fb4 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Fri, 28 Aug 2026 09:05:41 +0800 Subject: [PATCH 38/46] refactor: keep heredoc scanning linear --- scripts/hooks/gateguard-heredoc.js | 129 +++++++++---------- scripts/lib/shell-substitution.js | 181 +++++++++++---------------- tests/lib/shell-substitution.test.js | 12 +- 3 files changed, 143 insertions(+), 179 deletions(-) diff --git a/scripts/hooks/gateguard-heredoc.js b/scripts/hooks/gateguard-heredoc.js index 76de206f3..998a7bec1 100644 --- a/scripts/hooks/gateguard-heredoc.js +++ b/scripts/hooks/gateguard-heredoc.js @@ -124,30 +124,6 @@ function findHeredocs(line) { return heredocs.includes(null) ? null : heredocs; } -/** - * Iterate over executable substitutions in an unquoted heredoc. - * - * @param {string} text - * @returns {Generator} - */ -function* iterateHeredocCommandSubstitutions(text) { - let escaped = false; - for (let i = 0; i < text.length; i += 1) { - const ch = text[i]; - if (escaped) { - escaped = false; - continue; - } - if (ch === '\\') { - escaped = true; - continue; - } - if (ch === '`' || (ch === '$' && text[i + 1] === '(')) { - yield* extractCommandSubstitutions(text.slice(i)); - } - } -} - /** * Extract executable substitutions from an unquoted heredoc. Quote characters * in its payload are literal and do not suppress expansion. @@ -157,7 +133,58 @@ function* iterateHeredocCommandSubstitutions(text) { */ function extractHeredocCommandSubstitutions(body) { const text = body.join('\n'); - return [...new Set(iterateHeredocCommandSubstitutions(text))]; + return [...new Set(extractCommandSubstitutions(text, { literalOuterQuotes: true }))]; +} + +/** + * Consume one heredoc body and return its immutable parser result. + * + * @param {string[]} lines + * @param {number} startIndex + * @param {{ delimiter: string, quoted: boolean, stripTabs: boolean }} heredoc + * @returns {{ nextIndex: number, substitutions: string[] } | null} + */ +function consumeHeredocBody(lines, startIndex, heredoc) { + for (let lineIndex = startIndex; lineIndex < lines.length; lineIndex += 1) { + const line = lines[lineIndex]; + if (!heredoc.quoted && /\\$/.test(line)) return null; + const delimiterLine = heredoc.stripTabs ? line.replace(/^\t+/, '') : line; + if (delimiterLine !== heredoc.delimiter) continue; + const body = lines.slice(startIndex, lineIndex); + const substitutions = heredoc.quoted ? [] : extractHeredocCommandSubstitutions(body); + return { nextIndex: lineIndex + 1, substitutions }; + } + return null; +} + +/** + * @param {string[]} lines + * @param {number} startIndex + * @param {{ delimiter: string, quoted: boolean, stripTabs: boolean }[]} heredocs + * @returns {{ nextIndex: number, chunks: object | null } | null} + */ +function consumeHeredocBodies(lines, startIndex, heredocs) { + let state = { nextIndex: startIndex, chunks: null }; + for (const heredoc of heredocs) { + const consumed = consumeHeredocBody(lines, state.nextIndex, heredoc); + if (!consumed) return null; + state = { + nextIndex: consumed.nextIndex, + chunks: consumed.substitutions.length === 0 ? state.chunks : { substitutions: consumed.substitutions, previous: state.chunks } + }; + } + return state; +} + +/** @returns {Generator} */ +function* iterateSubstitutionChunks(chunks) { + let ordered = null; + for (let chunk = chunks; chunk; chunk = chunk.previous) { + ordered = { substitutions: chunk.substitutions, next: ordered }; + } + for (let chunk = ordered; chunk; chunk = chunk.next) { + yield* chunk.substitutions; + } } /** @@ -174,62 +201,26 @@ function extractHeredocCommandSubstitutions(body) { function stripHeredocBodies(input) { const raw = String(input || ''); const lines = raw.split(/\r?\n/); - let pending = []; - let pendingIndex = 0; - let bodyStartIndex = -1; let headerIndex = -1; - let trailingStartIndex = lines.length; - let substitutionText = ''; - let substitutionCount = 0; - let completedHeredoc = false; + let pending = []; for (let lineIndex = 0; lineIndex < lines.length; lineIndex += 1) { const line = lines[lineIndex]; - if (pendingIndex < pending.length) { - const current = pending[pendingIndex]; - if (!current.quoted && /\\$/.test(line)) return raw; - const delimiterLine = current.stripTabs ? line.replace(/^\t+/, '') : line; - if (delimiterLine === current.delimiter) { - if (!current.quoted) { - for (const substitution of extractHeredocCommandSubstitutions(lines.slice(bodyStartIndex, lineIndex))) { - substitutionText = substitutionCount === 0 ? substitution : `${substitutionText}\n${substitution}`; - substitutionCount += 1; - } - } - pendingIndex += 1; - bodyStartIndex = lineIndex + 1; - if (pendingIndex === pending.length) { - completedHeredoc = true; - trailingStartIndex = lineIndex + 1; - } - } - continue; - } - if (completedHeredoc && line.trim()) return raw; - if (completedHeredoc) continue; const heredocs = findHeredocs(line); if (heredocs === null) return raw; if (heredocs.length > 0 && !isProvenPassiveHeredocLine(line)) return raw; if (heredocs.length > 0) { pending = heredocs; - pendingIndex = 0; - bodyStartIndex = lineIndex + 1; headerIndex = lineIndex; + break; } } - if (pendingIndex < pending.length) return raw; if (headerIndex < 0) return lines.join('\n'); - - const prefix = lines.slice(0, headerIndex + 1).join('\n'); - const trailingCount = lines.length - trailingStartIndex; - const trailing = lines.slice(trailingStartIndex).join('\n'); - let result = prefix; - let resultCount = headerIndex + 1; - if (substitutionCount > 0) { - result = resultCount === 0 ? substitutionText : `${result}\n${substitutionText}`; - resultCount += substitutionCount; - } - if (trailingCount > 0) result = resultCount === 0 ? trailing : `${result}\n${trailing}`; - return result; + const consumed = consumeHeredocBodies(lines, headerIndex + 1, pending); + if (!consumed) return raw; + const trailing = lines.slice(consumed.nextIndex); + if (trailing.some(line => line.trim())) return raw; + const substitutions = iterateSubstitutionChunks(consumed.chunks); + return [...lines.slice(0, headerIndex + 1), ...substitutions, ...trailing].join('\n'); } module.exports = { stripHeredocBodies }; diff --git a/scripts/lib/shell-substitution.js b/scripts/lib/shell-substitution.js index 0251e74e2..a2241770d 100644 --- a/scripts/lib/shell-substitution.js +++ b/scripts/lib/shell-substitution.js @@ -1,126 +1,97 @@ 'use strict'; -/** - * Extract executable command-substitution bodies from a shell line. - * - * Single quotes are literal, so substitutions inside them are ignored; - * double quotes still permit substitutions, so those bodies are scanned - * before quoted text is stripped. Returns each substitution body plus - * any nested substitutions discovered recursively. - * - * Originally introduced in scripts/hooks/gateguard-fact-force.js - * (PR #1853 round 2). Extracted to a shared lib so other PreToolUse - * hooks that need the same "scan inside `$(...)` and backticks" - * behavior can reuse it without duplicating the parser. - * - * @param {string} input - * @returns {string[]} - */ -function extractCommandSubstitutions(input) { - const source = String(input || ''); - const substitutions = []; +/** @returns {{ body: string, endIndex: number }} */ +function readBacktickSubstitution(source, startIndex) { + let body = ''; + let endIndex = startIndex + 1; + while (endIndex < source.length) { + const inner = source[endIndex]; + if (inner === '\\') { + const escaped = source[endIndex + 1]; + body = escaped === undefined ? `${body}\\` : `${body}\\${escaped}`; + endIndex += escaped === undefined ? 1 : 2; + continue; + } + if (inner === '`') break; + body = `${body}${inner}`; + endIndex += 1; + } + return { body, endIndex }; +} + +/** @returns {{ body: string, endIndex: number }} */ +function readDollarSubstitution(source, startIndex) { + let body = ''; + let depth = 1; let inSingle = false; let inDouble = false; + let endIndex = startIndex + 2; + while (endIndex < source.length && depth > 0) { + const inner = source[endIndex]; + if (inner === '\\' && !inSingle) { + const escaped = source[endIndex + 1]; + body = escaped === undefined ? `${body}\\` : `${body}\\${escaped}`; + endIndex += escaped === undefined ? 1 : 2; + continue; + } + if (inner === "'" && !inDouble) inSingle = !inSingle; + else if (inner === '"' && !inSingle) inDouble = !inDouble; + else if (!inSingle && !inDouble && inner === '(') depth += 1; + else if (!inSingle && !inDouble && inner === ')') depth -= 1; + if (depth > 0) body = `${body}${inner}`; + endIndex += depth > 0 ? 1 : 0; + } + return { body, endIndex }; +} - for (let i = 0; i < source.length; i++) { +/** + * Iterate over command-substitution bodies, followed by nested bodies. + * Quote characters in an unquoted heredoc are literal only at the outer level; + * substitutions still use normal shell quote semantics internally. + * + * @param {string} input + * @param {{ literalOuterQuotes?: boolean }} [options] + * @returns {Generator} + */ +function* iterateCommandSubstitutions(input, options = {}) { + const source = String(input || ''); + const literalOuterQuotes = options.literalOuterQuotes === true; + let inSingle = false; + let inDouble = false; + for (let i = 0; i < source.length; i += 1) { const ch = source[i]; - const prev = source[i - 1]; - if (ch === '\\' && !inSingle) { i += 1; continue; } - - if (ch === "'" && !inDouble && prev !== '\\') { + if (!literalOuterQuotes && ch === "'" && !inDouble) { inSingle = !inSingle; continue; } - - if (ch === '"' && !inSingle && prev !== '\\') { + if (!literalOuterQuotes && ch === '"' && !inSingle) { inDouble = !inDouble; continue; } - - if (inSingle) { - continue; - } - - if (ch === '`') { - let body = ''; - i += 1; - while (i < source.length) { - const inner = source[i]; - if (inner === '\\') { - body += inner; - if (i + 1 < source.length) { - body += source[i + 1]; - i += 2; - } else { - // Trailing backslash at end of an unterminated span: advance past - // it so it is not appended a second time by the fallthrough below. - i += 1; - } - continue; - } - if (inner === '`') { - break; - } - body += inner; - i += 1; - } - if (body.trim()) { - substitutions.push(body); - substitutions.push(...extractCommandSubstitutions(body)); - } - continue; - } - - if (ch === '$' && source[i + 1] === '(') { - let depth = 1; - let body = ''; - let bodyInSingle = false; - let bodyInDouble = false; - i += 2; - while (i < source.length && depth > 0) { - const inner = source[i]; - const innerPrev = source[i - 1]; - if (inner === '\\' && !bodyInSingle) { - body += inner; - if (i + 1 < source.length) { - body += source[i + 1]; - i += 2; - } else { - // Trailing backslash at end of an unterminated span: advance past - // it so it is not appended a second time by the fallthrough below. - i += 1; - } - continue; - } - if (inner === "'" && !bodyInDouble && innerPrev !== '\\') { - bodyInSingle = !bodyInSingle; - } else if (inner === '"' && !bodyInSingle && innerPrev !== '\\') { - bodyInDouble = !bodyInDouble; - } else if (!bodyInSingle && !bodyInDouble) { - if (inner === '(') { - depth += 1; - } else if (inner === ')') { - depth -= 1; - if (depth === 0) { - break; - } - } - } - body += inner; - i += 1; - } - if (body.trim()) { - substitutions.push(body); - substitutions.push(...extractCommandSubstitutions(body)); - } - } + if (inSingle) continue; + const span = ch === '`' ? readBacktickSubstitution(source, i) : null; + const substitution = ch === '$' && source[i + 1] === '(' ? readDollarSubstitution(source, i) : span; + if (!substitution) continue; + i = substitution.endIndex; + if (!substitution.body.trim()) continue; + yield substitution.body; + yield* iterateCommandSubstitutions(substitution.body); } +} - return substitutions; +/** + * Extract executable command-substitution bodies from a shell line. + * + * @param {string} input + * @param {{ literalOuterQuotes?: boolean }} [options] + * @returns {string[]} + */ +function extractCommandSubstitutions(input, options = {}) { + return [...iterateCommandSubstitutions(input, options)]; } /** diff --git a/tests/lib/shell-substitution.test.js b/tests/lib/shell-substitution.test.js index 8b0be6cac..f64c90419 100644 --- a/tests/lib/shell-substitution.test.js +++ b/tests/lib/shell-substitution.test.js @@ -1,10 +1,6 @@ 'use strict'; const assert = require('assert'); -const { - extractCommandSubstitutions, - extractSubshellGroups, - extractBraceGroups, -} = require('../../scripts/lib/shell-substitution'); +const { extractCommandSubstitutions, extractSubshellGroups, extractBraceGroups } = require('../../scripts/lib/shell-substitution'); console.log('=== Testing shell-substitution.js ===\n'); @@ -66,6 +62,12 @@ test('double-quoted body extracted, single-quoted body ignored', () => { test('single quotes inside a $() body are preserved', () => { assert.deepStrictEqual(extractCommandSubstitutions("x=$(echo 'a b')"), ["echo 'a b'"]); }); +test('literal outer quotes do not suppress substitutions', () => { + assert.deepStrictEqual(extractCommandSubstitutions("'$(whoami)'", { literalOuterQuotes: true }), ['whoami']); +}); +test('literal outer quotes preserve shell quoting inside a substitution', () => { + assert.deepStrictEqual(extractCommandSubstitutions("'$(echo '$(ignored)')'", { literalOuterQuotes: true }), ["echo '$(ignored)'"]); +}); console.log('\nextractCommandSubstitutions - escaped substitutions:'); test('escaped \\$() is NOT extracted (literal dollar)', () => { From c40d0e4f7c4592af33a9dcca0462584cbbca561b Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Fri, 28 Aug 2026 09:19:12 +0800 Subject: [PATCH 39/46] fix: normalize heredoc line continuations --- scripts/hooks/gateguard-heredoc.js | 49 +++++++++++++++++++----- tests/hooks/gateguard-fact-force.test.js | 33 ++++++++++++++++ 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/scripts/hooks/gateguard-heredoc.js b/scripts/hooks/gateguard-heredoc.js index 998a7bec1..d29b58fe9 100644 --- a/scripts/hooks/gateguard-heredoc.js +++ b/scripts/hooks/gateguard-heredoc.js @@ -124,6 +124,35 @@ function findHeredocs(line) { return heredocs.includes(null) ? null : heredocs; } +/** @returns {boolean} */ +function hasLineContinuation(line) { + const trailing = line.match(/\\+$/); + return Boolean(trailing && trailing[0].length % 2 === 1); +} + +/** @returns {string} */ +function normalizeUnquotedHeredocLines(lines, stripTabs = false) { + const logical = lines + .map((line, index) => { + if (index === lines.length - 1) return line; + return hasLineContinuation(line) ? line.slice(0, -1) : `${line}\n`; + }) + .join(''); + return stripTabs ? logical.replace(/^\t+/, '') : logical; +} + +/** @returns {{ text: string, nextIndex: number }} */ +function readHeredocLine(lines, startIndex, quoted, stripTabs) { + if (quoted) { + const text = stripTabs ? lines[startIndex].replace(/^\t+/, '') : lines[startIndex]; + return { text, nextIndex: startIndex + 1 }; + } + let endIndex = startIndex; + while (endIndex < lines.length - 1 && hasLineContinuation(lines[endIndex])) endIndex += 1; + const text = normalizeUnquotedHeredocLines(lines.slice(startIndex, endIndex + 1), stripTabs); + return { text, nextIndex: endIndex + 1 }; +} + /** * Extract executable substitutions from an unquoted heredoc. Quote characters * in its payload are literal and do not suppress expansion. @@ -131,8 +160,8 @@ function findHeredocs(line) { * @param {string[]} body * @returns {string[]} */ -function extractHeredocCommandSubstitutions(body) { - const text = body.join('\n'); +function extractHeredocCommandSubstitutions(body, stripTabs) { + const text = normalizeUnquotedHeredocLines(body, stripTabs); return [...new Set(extractCommandSubstitutions(text, { literalOuterQuotes: true }))]; } @@ -145,14 +174,16 @@ function extractHeredocCommandSubstitutions(body) { * @returns {{ nextIndex: number, substitutions: string[] } | null} */ function consumeHeredocBody(lines, startIndex, heredoc) { - for (let lineIndex = startIndex; lineIndex < lines.length; lineIndex += 1) { - const line = lines[lineIndex]; - if (!heredoc.quoted && /\\$/.test(line)) return null; - const delimiterLine = heredoc.stripTabs ? line.replace(/^\t+/, '') : line; - if (delimiterLine !== heredoc.delimiter) continue; + let lineIndex = startIndex; + while (lineIndex < lines.length) { + const logical = readHeredocLine(lines, lineIndex, heredoc.quoted, heredoc.stripTabs); + if (logical.text !== heredoc.delimiter) { + lineIndex = logical.nextIndex; + continue; + } const body = lines.slice(startIndex, lineIndex); - const substitutions = heredoc.quoted ? [] : extractHeredocCommandSubstitutions(body); - return { nextIndex: lineIndex + 1, substitutions }; + const substitutions = heredoc.quoted ? [] : extractHeredocCommandSubstitutions(body, heredoc.stripTabs); + return { nextIndex: logical.nextIndex, substitutions }; } return null; } diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index b5b18cf8c..4c738c92a 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1760,6 +1760,39 @@ function runTests() { passed++; else failed++; + if ( + test('denies split command names after heredoc line continuation', () => { + expectDestructiveDeny( + ['cat < { + expectDestructiveDeny( + ['cat <<-EOF', '\t$(rm\\', '\t-rf /tmp/expanded-target)', 'EOF'].join('\n'), + 'split option in tab-stripped unquoted heredoc substitution' + ); + }) + ) + passed++; + else failed++; + + if ( + test('preserves internal tabs after tab-stripped heredoc continuations', () => { + expectAllow( + ['cat <<-EOF', '\t$(r\\', '\tm -rf /tmp/expanded-target)', 'EOF'].join('\n'), + 'internal tab after tab-stripped heredoc continuation' + ); + }) + ) + passed++; + else failed++; + if ( test('fails closed on line-continued unquoted heredoc terminators', () => { expectDestructiveDeny( From a4d72b2271a7045220cb8ed36f54c2a2c0f30054 Mon Sep 17 00:00:00 2001 From: wellkilo Date: Fri, 28 Aug 2026 15:40:33 +0800 Subject: [PATCH 40/46] fix(gateguard): surface graduated recovery hints Change-Id: I6ade0a2a54a26bd5721c62edf7efa462e8043a08 Co-authored-by: TRAE CLI --- scripts/hooks/gateguard-fact-force.js | 34 +++++++++++++++++++----- tests/hooks/gateguard-fact-force.test.js | 15 +++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js index 203092d64..bf91eb78a 100644 --- a/scripts/hooks/gateguard-fact-force.js +++ b/scripts/hooks/gateguard-fact-force.js @@ -42,6 +42,10 @@ const MAX_SESSION_KEYS = 50; const ROUTINE_BASH_SESSION_KEY = '__bash_session__'; const EDIT_WRITE_HOOK_ID = 'pre:edit-write:gateguard-fact-force'; const BASH_HOOK_ID = 'pre:bash:gateguard-fact-force'; +const EDIT_WRITE_NARROW_RECOVERY_HINT = + 'Narrow recovery: add a matching path glob to `GATEGUARD_EXEMPT_GLOBS` to skip first-touch Edit/Write checks without disabling destructive Bash checks.'; +const ROUTINE_BASH_NARROW_RECOVERY_HINT = + 'Narrow recovery: set `GATEGUARD_BASH_ROUTINE_DISABLED=1`; destructive Bash checks remain active.'; const ECC_DISABLE_VALUES = new Set(['0', 'false', 'off', 'disabled', 'disable']); const ECC_ENABLE_VALUES = new Set(['1', 'true', 'on', 'enabled', 'enable', 'yes']); @@ -1097,7 +1101,7 @@ function condensedGateMsg(action, filePath, ordinal) { return ( `[Fact-Forcing Gate] (denial #${ordinal} this session) First ${action} of ${safe}: ` + "briefly state importers/callers, affected API, data schemas if any, and the user's verbatim instruction, then retry. " + - '(ECC_GATEGUARD=off disables this gate.)' + '(Use GATEGUARD_EXEMPT_GLOBS for path-scoped exemptions; ECC_GATEGUARD=off disables this gate.)' ); } @@ -1128,9 +1132,15 @@ function routineBashMsg() { ].join('\n'); } -function withRecoveryHint(message, hookIds = [EDIT_WRITE_HOOK_ID]) { +function withRecoveryHint(message, hookIds = [EDIT_WRITE_HOOK_ID], narrowRecoveryHint = '') { const disableTargets = hookIds.map(hookId => `\`${hookId}\``).join(' or '); - return [message, '', `Recovery: if GateGuard is blocking setup or repair work, run this session with \`ECC_GATEGUARD=off\` or add ${disableTargets} to \`ECC_DISABLED_HOOKS\`.`].join('\n'); + const recoveryLines = narrowRecoveryHint ? [narrowRecoveryHint, ''] : []; + return [ + message, + '', + ...recoveryLines, + `Recovery: if GateGuard is blocking setup or repair work, run this session with \`ECC_GATEGUARD=off\` or add ${disableTargets} to \`ECC_DISABLED_HOOKS\`.` + ].join('\n'); } function isSubagentInvocation(data) { @@ -1148,12 +1158,15 @@ function isSubagentInvocation(data) { function denyResult(reason, options = {}) { const includeRecoveryHint = options.includeRecoveryHint !== false; const hookIds = Array.isArray(options.hookIds) && options.hookIds.length > 0 ? options.hookIds : [EDIT_WRITE_HOOK_ID]; + const narrowRecoveryHint = typeof options.narrowRecoveryHint === 'string' ? options.narrowRecoveryHint : ''; return { stdout: JSON.stringify({ hookSpecificOutput: { hookEventName: 'PreToolUse', permissionDecision: 'deny', - permissionDecisionReason: includeRecoveryHint ? withRecoveryHint(reason, hookIds) : reason + permissionDecisionReason: includeRecoveryHint + ? withRecoveryHint(reason, hookIds, narrowRecoveryHint) + : reason } }), exitCode: 0 @@ -1210,7 +1223,9 @@ function run(rawInput) { const action = toolName === 'Edit' ? 'edit' : 'creation'; return denyResult(condensedGateMsg(action, filePath, denials), { includeRecoveryHint: false }); } - return denyResult(toolName === 'Edit' ? editGateMsg(filePath) : writeGateMsg(filePath)); + return denyResult(toolName === 'Edit' ? editGateMsg(filePath) : writeGateMsg(filePath), { + narrowRecoveryHint: EDIT_WRITE_NARROW_RECOVERY_HINT + }); } return rawInput; // allow @@ -1232,7 +1247,9 @@ function run(rawInput) { if (denials > getFullDenialBudget()) { return denyResult(condensedGateMsg('edit', filePath, denials), { includeRecoveryHint: false }); } - return denyResult(editGateMsg(filePath)); + return denyResult(editGateMsg(filePath), { + narrowRecoveryHint: EDIT_WRITE_NARROW_RECOVERY_HINT + }); } } return rawInput; // allow @@ -1268,7 +1285,10 @@ function run(rawInput) { if (!markChecked(ROUTINE_BASH_SESSION_KEY)) { return allowWithStateWarning(); } - return denyResult(routineBashMsg(), { hookIds: [BASH_HOOK_ID] }); + return denyResult(routineBashMsg(), { + hookIds: [BASH_HOOK_ID], + narrowRecoveryHint: ROUTINE_BASH_NARROW_RECOVERY_HINT + }); } return rawInput; // allow diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 4c738c92a..5023dac5c 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -145,6 +145,8 @@ function runTests() { assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('Fact-Forcing Gate')); assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('import/require')); assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('/src/app.js')); + assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('GATEGUARD_EXEMPT_GLOBS'), 'Edit denial should show the path-scoped exemption control'); + assert.ok(!output.hookSpecificOutput.permissionDecisionReason.includes('GATEGUARD_BASH_ROUTINE_DISABLED'), 'Edit denial should not suggest the routine Bash control'); }) ) passed++; @@ -538,6 +540,8 @@ function runTests() { assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny'); assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('ECC_GATEGUARD=off'), 'denial reason should show the direct recovery env toggle'); assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('ECC_DISABLED_HOOKS'), 'denial reason should mention the existing hook-id disable control'); + assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('GATEGUARD_EXEMPT_GLOBS'), 'Edit/Write denial should show the path-scoped exemption control'); + assert.ok(!output.hookSpecificOutput.permissionDecisionReason.includes('GATEGUARD_BASH_ROUTINE_DISABLED'), 'Edit/Write denial should not suggest the routine Bash control'); }) ) passed++; @@ -558,6 +562,9 @@ function runTests() { assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny'); assert.ok(reason.includes('pre:bash:gateguard-fact-force'), 'routine Bash denial should show the Bash hook ID'); assert.ok(!reason.includes('pre:edit-write:gateguard-fact-force'), 'routine Bash denial should not show the Edit/Write hook ID as the targeted disable'); + assert.ok(reason.includes('GATEGUARD_BASH_ROUTINE_DISABLED=1'), 'routine Bash denial should show the narrow routine-gate control'); + assert.ok(reason.includes('destructive Bash checks remain active'), 'routine Bash denial should preserve the destructive-check safety boundary'); + assert.ok(!reason.includes('GATEGUARD_EXEMPT_GLOBS'), 'routine Bash denial should not suggest the Edit/Write path control'); }) ) passed++; @@ -577,6 +584,9 @@ function runTests() { assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny'); assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('Destructive command detected')); assert.ok(!output.hookSpecificOutput.permissionDecisionReason.includes('ECC_GATEGUARD=off'), 'destructive gate should not advertise disabling GateGuard'); + assert.ok(!output.hookSpecificOutput.permissionDecisionReason.includes('ECC_DISABLED_HOOKS'), 'destructive gate should not advertise disabling its hook'); + assert.ok(!output.hookSpecificOutput.permissionDecisionReason.includes('GATEGUARD_BASH_ROUTINE_DISABLED'), 'destructive gate should not advertise the routine-only bypass'); + assert.ok(!output.hookSpecificOutput.permissionDecisionReason.includes('GATEGUARD_EXEMPT_GLOBS'), 'destructive gate should not advertise the Edit/Write path exemption'); }) ) passed++; @@ -602,6 +612,7 @@ function runTests() { assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny'); assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('Fact-Forcing Gate')); assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('/src/multi-a.js')); + assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('GATEGUARD_EXEMPT_GLOBS'), 'MultiEdit denial should show the path-scoped exemption control'); }) ) passed++; @@ -2556,6 +2567,7 @@ function runTests() { assert.ok(!reason.includes('present these facts'), 'no repeated four-fact block'); assert.ok(!reason.includes('\n'), 'condensed message is a single line'); assert.ok(reason.includes('ECC_GATEGUARD=off'), 'condensed message keeps a recovery hint'); + assert.ok(reason.includes('GATEGUARD_EXEMPT_GLOBS'), 'condensed Edit denial keeps the path-scoped recovery hint'); }) ) passed++; @@ -2571,6 +2583,8 @@ function runTests() { const secondReason = second.hookSpecificOutput.permissionDecisionReason; assert.ok(firstReason.includes('denial #6'), `expected ordinal 6, got: ${firstReason}`); assert.ok(secondReason.includes('denial #7'), `expected ordinal 7, got: ${secondReason}`); + assert.ok(firstReason.includes('GATEGUARD_EXEMPT_GLOBS'), 'condensed Write denial keeps the path-scoped recovery hint'); + assert.ok(!firstReason.includes('GATEGUARD_BASH_ROUTINE_DISABLED'), 'condensed Write denial should not suggest the routine Bash control'); assert.notStrictEqual(firstReason, secondReason, 'successive denials must differ so they cannot compound verbatim'); }) ) @@ -2635,6 +2649,7 @@ function runTests() { assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny'); assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('denial #5')); assert.ok(!output.hookSpecificOutput.permissionDecisionReason.includes('present these facts')); + assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('GATEGUARD_EXEMPT_GLOBS'), 'condensed MultiEdit denial keeps the path-scoped recovery hint'); }) ) passed++; From fab534f9247ebe0565dc01ebf64f5fed19d890d9 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:15:16 -0400 Subject: [PATCH 41/46] test(hooks): keep matcher mirrors in sync --- hooks/codex-hooks.json | 2 +- tests/hooks/posttooluse-dispatcher.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hooks/codex-hooks.json b/hooks/codex-hooks.json index efcdcee91..551f7a4b4 100644 --- a/hooks/codex-hooks.json +++ b/hooks/codex-hooks.json @@ -3,7 +3,7 @@ "hooks": { "SessionStart": [ { - "matcher": "*", + "matcher": ".*", "hooks": [ { "type": "command", diff --git a/tests/hooks/posttooluse-dispatcher.test.js b/tests/hooks/posttooluse-dispatcher.test.js index 0900117d4..0ce83581e 100644 --- a/tests/hooks/posttooluse-dispatcher.test.js +++ b/tests/hooks/posttooluse-dispatcher.test.js @@ -82,7 +82,7 @@ function runTests() { entries.map(entry => entry.id), ['post:dispatcher:sync', 'post:dispatcher:async'] ); - assert.ok(entries.every(entry => entry.matcher === '*')); + assert.ok(entries.every(entry => entry.matcher === '.*')); assert.strictEqual(entries[0].hooks[0].async, undefined); assert.strictEqual(entries[1].hooks[0].async, true); assert.ok(entries[0].hooks[0].command.includes('posttooluse-dispatcher.js')); From 224da03d01ecae5187e0cb5458f0d85bc6fc4869 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sat, 29 Aug 2026 14:45:22 -0400 Subject: [PATCH 42/46] fix(gateguard): match heredoc tab-strip order --- scripts/hooks/gateguard-heredoc.js | 7 ++++--- tests/hooks/gateguard-fact-force.test.js | 12 ++++++------ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/scripts/hooks/gateguard-heredoc.js b/scripts/hooks/gateguard-heredoc.js index d29b58fe9..31e41b29b 100644 --- a/scripts/hooks/gateguard-heredoc.js +++ b/scripts/hooks/gateguard-heredoc.js @@ -134,11 +134,12 @@ function hasLineContinuation(line) { function normalizeUnquotedHeredocLines(lines, stripTabs = false) { const logical = lines .map((line, index) => { - if (index === lines.length - 1) return line; - return hasLineContinuation(line) ? line.slice(0, -1) : `${line}\n`; + const normalized = stripTabs ? line.replace(/^\t+/, '') : line; + if (index === lines.length - 1) return normalized; + return hasLineContinuation(normalized) ? normalized.slice(0, -1) : `${normalized}\n`; }) .join(''); - return stripTabs ? logical.replace(/^\t+/, '') : logical; + return logical; } /** @returns {{ text: string, nextIndex: number }} */ diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js index 5023dac5c..54a19c0e0 100644 --- a/tests/hooks/gateguard-fact-force.test.js +++ b/tests/hooks/gateguard-fact-force.test.js @@ -1783,10 +1783,10 @@ function runTests() { else failed++; if ( - test('denies split options after tab-stripped heredoc line continuation', () => { - expectDestructiveDeny( + test('allows a joined command when tab stripping removes the option separator', () => { + expectAllow( ['cat <<-EOF', '\t$(rm\\', '\t-rf /tmp/expanded-target)', 'EOF'].join('\n'), - 'split option in tab-stripped unquoted heredoc substitution' + 'tab stripping joins rm and -rf into a harmless command name' ); }) ) @@ -1794,10 +1794,10 @@ function runTests() { else failed++; if ( - test('preserves internal tabs after tab-stripped heredoc continuations', () => { - expectAllow( + test('denies split command names after tab-stripped heredoc continuations', () => { + expectDestructiveDeny( ['cat <<-EOF', '\t$(r\\', '\tm -rf /tmp/expanded-target)', 'EOF'].join('\n'), - 'internal tab after tab-stripped heredoc continuation' + 'split command name in tab-stripped unquoted heredoc substitution' ); }) ) From 2f895a1823833069799ce3b67f898771640f0aaa Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 27 Jul 2026 04:55:25 +0000 Subject: [PATCH 43/46] chore(deps): bump actions/setup-python from 6.2.0 to 7.0.0 Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6.2.0 to 7.0.0. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](https://github.com/actions/setup-python/compare/a309ff8b426b58ec0e2a45f0f869d46889d02405...5fda3b95a4ea91299a34e894583c3862153e4b97) --- updated-dependencies: - dependency-name: actions/setup-python dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index af0926402..98b1a7d73 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -251,7 +251,7 @@ jobs: persist-credentials: false - name: Setup Python - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: '3.11' From 703163275d32630ea74bb23accc85eb641469f6f Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sat, 29 Aug 2026 15:11:38 -0400 Subject: [PATCH 44/46] test: honor per-invocation Bash overrides --- tests/scripts/codex-hooks.test.js | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 353c64efe..928d2a121 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -56,13 +56,14 @@ function runBash( scriptPath, { args = [], env = {}, cwd = repoRoot, input = undefined, preservePath = true } = {}, ) { - const bash = resolveBashExecutable(); + const effectiveEnv = { + ...(preservePath ? process.env : {}), + ...env, + }; + const bash = resolveBashExecutable(effectiveEnv); return spawnSync(bash, [scriptPath, ...args], { cwd, - env: { - ...(preservePath ? process.env : {}), - ...env, - }, + env: effectiveEnv, encoding: 'utf8', input, stdio: ['pipe', 'pipe', 'pipe'], @@ -148,6 +149,16 @@ if ( passed++; else failed++; +if ( + test('shell test runner honors a per-invocation BASH_PATH override', () => { + const missingBash = path.join(os.tmpdir(), 'ecc-missing-bash-executable'); + const result = runBash(prePushHook, { env: { BASH_PATH: missingBash } }); + assert.strictEqual(result.error?.code, 'ENOENT'); + }) +) + passed++; +else failed++; + function runHermeticPrePush({ failScript = null, includeCorepack = true, From 1bdda4bdacca65d53ff7c4e1cb268f60e3fe669c Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sat, 29 Aug 2026 15:36:59 -0400 Subject: [PATCH 45/46] fix: close validator and path edge cases --- scripts/ci/validate-skills.js | 23 +++++++++++++-- skills/skill-stocktake/scripts/quick-diff.sh | 9 ++---- skills/skill-stocktake/scripts/scan.sh | 19 ++++++++---- tests/ci/validators.test.js | 28 ++++++++++++++++++ tests/scripts/codex-hooks.test.js | 11 +++++-- .../scripts/skill-stocktake-discovery.test.js | 29 ++++++++++++++++++- 6 files changed, 101 insertions(+), 18 deletions(-) diff --git a/scripts/ci/validate-skills.js b/scripts/ci/validate-skills.js index 6e7c3a64b..47334687f 100644 --- a/scripts/ci/validate-skills.js +++ b/scripts/ci/validate-skills.js @@ -26,6 +26,7 @@ const fs = require('fs'); const path = require('path'); +const yaml = require('js-yaml'); const SKILLS_DIR = path.join(__dirname, '../../skills'); const DOCS_DIR = path.join(__dirname, '../../docs'); @@ -101,7 +102,7 @@ function stripUnquotedYamlComment(rawValue) { } function inspectFrontmatter(lines) { - const values = Object.create(null); + let values = Object.create(null); let syntaxErrors = []; let descriptionIndicator = null; let inBlockScalar = false; @@ -126,7 +127,7 @@ function inspectFrontmatter(lines) { const rawValue = match[2]; // Strip YAML comments only when # appears outside a quoted scalar. const valueNoComment = stripUnquotedYamlComment(rawValue); - values[key] = valueNoComment; + values = Object.assign(Object.create(null), values, { [key]: valueNoComment }); const isQuoted = /^"(?:[^"\\]|\\.)*"$/.test(valueNoComment) || /^'(?:[^']|'')*'$/.test(valueNoComment); @@ -164,6 +165,24 @@ function inspectFrontmatter(lines) { } } + try { + const parsed = yaml.load(lines.join('\n')); + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + syntaxErrors = [...syntaxErrors, 'must be a top-level YAML mapping']; + } else { + for (const key of ['name', 'description']) { + if (!Object.prototype.hasOwnProperty.call(parsed, key)) continue; + if (typeof parsed[key] !== 'string') { + syntaxErrors = [...syntaxErrors, `${key}: value must be a string`]; + continue; + } + values = Object.assign(Object.create(null), values, { [key]: parsed[key] }); + } + } + } catch (error) { + syntaxErrors = [...syntaxErrors, `invalid YAML: ${error.reason || error.message}`]; + } + return { values, descriptionIndicator, syntaxErrors }; } diff --git a/skills/skill-stocktake/scripts/quick-diff.sh b/skills/skill-stocktake/scripts/quick-diff.sh index 418b02558..b22d42e11 100755 --- a/skills/skill-stocktake/scripts/quick-diff.sh +++ b/skills/skill-stocktake/scripts/quick-diff.sh @@ -58,9 +58,6 @@ if [[ ! "$evaluated_at" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2 exit 1 fi -# Pre-extract known paths from results.json once (O(1) lookup per file instead of O(n*m)) -known_paths=$(jq -r '.skills[].path' "$RESULTS_JSON" 2>/dev/null) - tmpdir=$(mktemp -d) # Use a function to avoid embedding $tmpdir in a quoted string (prevents injection # if TMPDIR were crafted to contain shell metacharacters). @@ -90,9 +87,9 @@ process_dir() { mtime=$(date -u -r "$file" +%Y-%m-%dT%H:%M:%SZ) dp="${file/#$HOME/~}" - # Check if this file is known to results.json (exact whole-line match to - # avoid substring false-positives, e.g. "python-patterns" matching "python-patterns-v2"). - if echo "$known_paths" | grep -qxF "$dp"; then + # Keep path comparison structured so literal newlines remain part of one + # JSON string instead of becoming ambiguous line-delimited records. + if jq -e --arg path "$dp" '.skills | any(.path == $path)' "$RESULTS_JSON" >/dev/null 2>&1; then is_new="false" # Known file: only emit if mtime changed (ISO 8601 string comparison is safe) [[ "$mtime" > "$evaluated_at" ]] || continue diff --git a/skills/skill-stocktake/scripts/scan.sh b/skills/skill-stocktake/scripts/scan.sh index e43509edc..ea2e65574 100755 --- a/skills/skill-stocktake/scripts/scan.sh +++ b/skills/skill-stocktake/scripts/scan.sh @@ -134,12 +134,19 @@ scan_dir_to_json() { name=$(extract_field "$file" "name") desc=$(extract_field "$file" "description") mtime=$(date -u -r "$file" +%Y-%m-%dT%H:%M:%SZ) - # Use awk exact field match to avoid substring false-positives from grep -F. - # uniq -c output format: " N /path/to/file" — path is always field 2. - u7=$(echo "$obs_7d_counts" | awk -v f="$file" '$2 == f {print $1}' | head -1) - u7="${u7:-0}" - u30=$(echo "$obs_30d_counts" | awk -v f="$file" '$2 == f {print $1}' | head -1) - u30="${u30:-0}" + if [[ "$file" == *$'\n'* ]]; then + # The aggregated fast path is line-delimited. Preserve unusual paths by + # falling back to the structured JSON matcher for this record. + u7=$(count_obs "$file" "$c7") + u30=$(count_obs "$file" "$c30") + else + # Use awk exact field match to avoid substring false-positives from grep -F. + # uniq -c output format: " N /path/to/file" — path is always field 2. + u7=$(echo "$obs_7d_counts" | awk -v f="$file" '$2 == f {print $1}' | head -1) + u7="${u7:-0}" + u30=$(echo "$obs_30d_counts" | awk -v f="$file" '$2 == f {print $1}' | head -1) + u30="${u30:-0}" + fi dp="${file/#$HOME/~}" jq -n \ diff --git a/tests/ci/validators.test.js b/tests/ci/validators.test.js index 4bcb9452a..afac4a469 100644 --- a/tests/ci/validators.test.js +++ b/tests/ci/validators.test.js @@ -2870,6 +2870,34 @@ function runTests() { cleanupTestDir(testDir); })) passed++; else failed++; + if (test('rejects malformed quoted skill frontmatter', () => { + const testDir = createTestDir(); + const skillDir = path.join(testDir, 'malformed-quote'); + fs.mkdirSync(skillDir); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), + '---\nname: malformed-quote\ndescription: "unterminated\n---\n# Example'); + + const result = runSkillsValidator(testDir, ['--strict']); + assert.strictEqual(result.code, 1, 'Strict validation must reject malformed YAML'); + assert.ok(result.stderr.includes('invalid YAML'), + `Should report the YAML parse failure, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + + if (test('rejects an empty folded skill description', () => { + const testDir = createTestDir(); + const skillDir = path.join(testDir, 'empty-folded-description'); + fs.mkdirSync(skillDir); + fs.writeFileSync(path.join(skillDir, 'SKILL.md'), + '---\nname: empty-folded-description\ndescription: >\n---\n# Example'); + + const result = runSkillsValidator(testDir, ['--strict']); + assert.strictEqual(result.code, 1, 'Strict validation must reject an empty folded scalar'); + assert.ok(result.stderr.includes("'description' is empty"), + `Should report the empty parsed description, got: ${result.stderr}`); + cleanupTestDir(testDir); + })) passed++; else failed++; + if (test('reports an unreadable docs root deterministically', () => { const testDir = createTestDir(); const docsPath = path.join(testDir, 'docs-file'); diff --git a/tests/scripts/codex-hooks.test.js b/tests/scripts/codex-hooks.test.js index 928d2a121..0dfe1d2f9 100644 --- a/tests/scripts/codex-hooks.test.js +++ b/tests/scripts/codex-hooks.test.js @@ -151,9 +151,14 @@ else failed++; if ( test('shell test runner honors a per-invocation BASH_PATH override', () => { - const missingBash = path.join(os.tmpdir(), 'ecc-missing-bash-executable'); - const result = runBash(prePushHook, { env: { BASH_PATH: missingBash } }); - assert.strictEqual(result.error?.code, 'ENOENT'); + const tempDir = createTempDir('ecc-missing-bash-'); + try { + const missingBash = path.join(tempDir, 'bash'); + const result = runBash(prePushHook, { env: { BASH_PATH: missingBash } }); + assert.strictEqual(result.error?.code, 'ENOENT'); + } finally { + cleanup(tempDir); + } }) ) passed++; diff --git a/tests/scripts/skill-stocktake-discovery.test.js b/tests/scripts/skill-stocktake-discovery.test.js index 498ed43fa..106d041ef 100644 --- a/tests/scripts/skill-stocktake-discovery.test.js +++ b/tests/scripts/skill-stocktake-discovery.test.js @@ -65,6 +65,7 @@ if (process.platform === 'win32') { const linkedTarget = path.join(tempRoot, 'shared', 'linked-skill'); const newlineSkill = path.join(projectSkills, 'newline\nskill'); const resultsPath = path.join(tempRoot, 'results.json'); + const observationsPath = path.join(tempRoot, 'observations.jsonl'); writeSkill(directSkill, 'direct-skill'); writeSkill(linkedTarget, 'linked-skill'); @@ -76,11 +77,19 @@ if (process.platform === 'win32') { resultsPath, JSON.stringify({ evaluated_at: '2099-01-01T00:00:00Z', skills: [] }), ); + fs.writeFileSync( + observationsPath, + `${JSON.stringify({ + tool: 'Read', + path: path.join(newlineSkill, 'SKILL.md'), + timestamp: new Date().toISOString(), + })}\n`, + ); const env = { SKILL_STOCKTAKE_GLOBAL_DIR: path.join(tempRoot, 'missing-global'), SKILL_STOCKTAKE_PROJECT_DIR: projectSkills, - SKILL_STOCKTAKE_OBSERVATIONS: path.join(tempRoot, 'missing-observations.jsonl'), + SKILL_STOCKTAKE_OBSERVATIONS: observationsPath, }; test('scan follows symlinked skills and ignores nested Markdown assets', () => { @@ -92,6 +101,9 @@ if (process.platform === 'win32') { output.skills.map(skill => skill.name).sort(), ['direct-skill', 'linked-skill', 'newline-skill'], ); + const newlineEntry = output.skills.find(skill => skill.name === 'newline-skill'); + assert.strictEqual(newlineEntry.use_7d, 1); + assert.strictEqual(newlineEntry.use_30d, 1); }); test('quick diff keeps newline-containing skill paths as one record', () => { @@ -105,6 +117,21 @@ if (process.platform === 'win32') { ); assert.ok(output.every(entry => entry.is_new === true)); }); + + test('quick diff recognizes a cached newline-containing path', () => { + fs.writeFileSync( + resultsPath, + JSON.stringify({ + evaluated_at: '2099-01-01T00:00:00Z', + skills: [{ path: path.join(newlineSkill, 'SKILL.md') }], + }), + ); + const result = runBash(quickDiffScript, [resultsPath], env); + assert.strictEqual(result.status, 0, result.stderr); + const output = JSON.parse(result.stdout); + assert.strictEqual(output.length, 2); + assert.ok(output.every(entry => !entry.path.includes('newline\nskill/SKILL.md'))); + }); } catch (error) { console.log(` ✗ fixture setup: ${error.message}`); failed++; From 299544e6801e6938281985f74df9a25e12905c65 Mon Sep 17 00:00:00 2001 From: haelyra <49814733+haelyra@users.noreply.github.com> Date: Sat, 29 Aug 2026 16:07:00 -0400 Subject: [PATCH 46/46] fix: count observations for whitespace paths --- skills/skill-stocktake/scripts/scan.sh | 2 +- tests/scripts/skill-stocktake-discovery.test.js | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/skills/skill-stocktake/scripts/scan.sh b/skills/skill-stocktake/scripts/scan.sh index ea2e65574..02c9c3dab 100755 --- a/skills/skill-stocktake/scripts/scan.sh +++ b/skills/skill-stocktake/scripts/scan.sh @@ -134,7 +134,7 @@ scan_dir_to_json() { name=$(extract_field "$file" "name") desc=$(extract_field "$file" "description") mtime=$(date -u -r "$file" +%Y-%m-%dT%H:%M:%SZ) - if [[ "$file" == *$'\n'* ]]; then + if [[ "$file" == *[[:space:]]* ]]; then # The aggregated fast path is line-delimited. Preserve unusual paths by # falling back to the structured JSON matcher for this record. u7=$(count_obs "$file" "$c7") diff --git a/tests/scripts/skill-stocktake-discovery.test.js b/tests/scripts/skill-stocktake-discovery.test.js index 106d041ef..92c13c6df 100644 --- a/tests/scripts/skill-stocktake-discovery.test.js +++ b/tests/scripts/skill-stocktake-discovery.test.js @@ -61,7 +61,7 @@ if (process.platform === 'win32') { const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-skill-stocktake-')); try { const projectSkills = path.join(tempRoot, 'project', '.claude', 'skills'); - const directSkill = path.join(projectSkills, 'direct-skill'); + const directSkill = path.join(projectSkills, 'direct skill'); const linkedTarget = path.join(tempRoot, 'shared', 'linked-skill'); const newlineSkill = path.join(projectSkills, 'newline\nskill'); const resultsPath = path.join(tempRoot, 'results.json'); @@ -83,6 +83,10 @@ if (process.platform === 'win32') { tool: 'Read', path: path.join(newlineSkill, 'SKILL.md'), timestamp: new Date().toISOString(), + })}\n${JSON.stringify({ + tool: 'Read', + path: path.join(directSkill, 'SKILL.md'), + timestamp: new Date().toISOString(), })}\n`, ); @@ -104,6 +108,9 @@ if (process.platform === 'win32') { const newlineEntry = output.skills.find(skill => skill.name === 'newline-skill'); assert.strictEqual(newlineEntry.use_7d, 1); assert.strictEqual(newlineEntry.use_30d, 1); + const spaceEntry = output.skills.find(skill => skill.name === 'direct-skill'); + assert.strictEqual(spaceEntry.use_7d, 1); + assert.strictEqual(spaceEntry.use_30d, 1); }); test('quick diff keeps newline-containing skill paths as one record', () => {