From 8ec7cd990b6d040f61cfb40d4b306ff4de831fc2 Mon Sep 17 00:00:00 2001 From: Anthony Sayge Date: Thu, 16 Jul 2026 07:22:27 -0500 Subject: [PATCH] Bugfix- update allow lanes. --- .claude/hooks/enforce-lanes.js | 53 ++++++---- plugin/.claude-plugin/plugin.json | 2 +- plugin/CHANGELOG.md | 31 ++++++ plugin/hooks/enforce-lanes.js | 53 ++++++---- plugin/hooks/enforce-lanes.test.js | 152 +++++++++++++++++++++++++++++ 5 files changed, 250 insertions(+), 41 deletions(-) create mode 100644 plugin/CHANGELOG.md create mode 100644 plugin/hooks/enforce-lanes.test.js diff --git a/.claude/hooks/enforce-lanes.js b/.claude/hooks/enforce-lanes.js index dcc4644..24dd01f 100644 --- a/.claude/hooks/enforce-lanes.js +++ b/.claude/hooks/enforce-lanes.js @@ -158,6 +158,24 @@ function stripVersion(token) { return at === -1 ? token : token.slice(0, at); } +// Deny messages shared by the install and npx paths. +function zeroDepDeny(agent) { + return { + deny: + `Blocked for ${agent}: package managers are disabled — this project has no ` + + 'approved dependencies (zero-dependency by default). Raise the package you need ' + + 'under OPEN QUESTIONS so the product owner can add it to .claude/lanes.json ' + + 'dependencies.allow. Do not retry.', + }; +} +function installerLaneDeny(agent, deps) { + return { + deny: + `Blocked for ${agent}: installing dependencies is the ${deps.installers.join('/')} ` + + "lane's responsibility, not yours. Raise it under OPEN QUESTIONS. Do not retry.", + }; +} + // Decide whether a Bash command that touches a package manager is allowed under // the project's dependency policy. Returns: // null -> no package manager involved; caller proceeds normally @@ -178,25 +196,16 @@ function resolveDependencyDecision(cmd, agent, deps) { if (!PACKAGE_MANAGERS.has(mgr)) continue; sawPackageManager = true; - if (!active) { - return { - deny: - `Blocked for ${agent}: package managers are disabled — this project has no ` + - 'approved dependencies (zero-dependency by default). Raise the package you need ' + - 'under OPEN QUESTIONS so the product owner can add it to .claude/lanes.json ' + - 'dependencies.allow. Do not retry.', - }; - } - if (!isInstaller(agent, deps)) { - return { - deny: - `Blocked for ${agent}: installing dependencies is the ${deps.installers.join('/')} ` + - "lane's responsibility, not yours. Raise it under OPEN QUESTIONS. Do not retry.", - }; - } - - // npx runs (and may fetch) a package — gate its target like an install. + // Classify the command FIRST — only *installs* (and `npx`, which fetches/ + // runs a package) are gated by the active/installer/allowlist policy. + // Non-install runs (`npm run`, `npm test`, `npm ls`, `pnpm exec`, …) are + // plain script execution and are allowed for ANY lane, regardless of + // whether dependencies are active. Gating them here was the bug that stopped + // frontend/QA lanes from building or testing frontend-only projects. if (mgr === 'npx') { + // npx runs (and may fetch) a package — gate it like an install. + if (!active) return zeroDepDeny(agent); + if (!isInstaller(agent, deps)) return installerLaneDeny(agent, deps); const target = tokens.slice(1).find((t) => !t.startsWith('-')); if (target && !deps.allow.includes(stripVersion(target))) { return { @@ -213,7 +222,11 @@ function resolveDependencyDecision(cmd, agent, deps) { const installSubs = INSTALL_SUBCMDS[mgr] || new Set(); // Bare `yarn` (no subcommand) installs from the manifest. const isInstall = sub ? installSubs.has(sub) : mgr === 'yarn'; - if (!isInstall) continue; // `npm run`, `npm test`, `npm ls`, … — allowed when active. + if (!isInstall) continue; // `npm run`, `npm test`, `npm ls`, … — allowed for any lane. + + // From here down the command installs packages — apply the full policy. + if (!active) return zeroDepDeny(agent); + if (!isInstaller(agent, deps)) return installerLaneDeny(agent, deps); const rest = tokens.slice(2); if (rest.includes('-r') || rest.includes('--requirement') || rest.includes('-e')) { @@ -314,7 +327,7 @@ function run(raw) { if (!agent) return; // orchestrator Bash is not policed here const cmd = String(args.command || ''); const depDecision = resolveDependencyDecision(cmd, agent, deps); - if (depDecision && depDecision.deny) deny(depDecision.reason); + if (depDecision && depDecision.deny) deny(depDecision.deny); // A permitted package-manager command still falls through to the other // Bash heuristics below (e.g. it must not also `git push`). for (const { re, why } of cfg.bashDeny) { diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 356b63d..a47b8f9 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "agentic-dev", - "version": "0.1.3", + "version": "0.1.4", "description": "Agentic dev process: 5 role agents (ux/frontend/backend/qa/docs), the /feature pipeline, and mechanical lane enforcement — all native Claude Code subagents, no framework.", "author": { "name": "tmkab121" }, "homepage": "https://github.com/tmkab121/agentic-dev", diff --git a/plugin/CHANGELOG.md b/plugin/CHANGELOG.md new file mode 100644 index 0000000..4e275a8 --- /dev/null +++ b/plugin/CHANGELOG.md @@ -0,0 +1,31 @@ +# Changelog + +All notable changes to the `agentic-dev` plugin are recorded here. + +## 0.1.5 + +### Fixed — lane-enforcement dependency policy (`hooks/enforce-lanes.js`) + +- **Non-install package-manager commands are no longer gated by the installer + lane.** `resolveDependencyDecision` classified the command *after* it had + already applied the active/installer gate, so `npm run build`, `npm run dev`, + `npm test`, `npm ls`, etc. were denied for any lane not on + `dependencies.installers` (default: only `backend-developer`). In a + frontend-only project the frontend and QA lanes could not build, serve, or + test at all. The installer/active gate now guards only commands that actually + install (`npm install`/`ci`/`add`, bare `yarn`, `pnpm add`, …) and the `npx` + fetch/run path; non-install script runs are allowed for any lane. Every + existing install and allowlist restriction is preserved. +- **Deny reasons now reach the agent.** The `run()` caller read + `depDecision.reason` while `resolveDependencyDecision` returns + `{ deny: }`, so `deny(undefined)` emitted a blank + `permissionDecisionReason` and blocked agents saw a generic denial with no + guidance. The caller now passes `depDecision.deny`, so the steering text + (raise it under OPEN QUESTIONS, the installer lane name, the offending package + names) reaches the agent. +- Zero-dependency projects: the zero-dep block now also applies only to installs + and `npx`, so a non-installer running `npm test` / `npm ls` is allowed rather + than denied with a misleading "package managers are disabled" message. +- Added `hooks/enforce-lanes.test.js` covering non-install allow, installer-lane + deny, allowlisted install allow, non-allowlisted install deny, npx gating, and + the deny-reason propagation. diff --git a/plugin/hooks/enforce-lanes.js b/plugin/hooks/enforce-lanes.js index dcc4644..24dd01f 100644 --- a/plugin/hooks/enforce-lanes.js +++ b/plugin/hooks/enforce-lanes.js @@ -158,6 +158,24 @@ function stripVersion(token) { return at === -1 ? token : token.slice(0, at); } +// Deny messages shared by the install and npx paths. +function zeroDepDeny(agent) { + return { + deny: + `Blocked for ${agent}: package managers are disabled — this project has no ` + + 'approved dependencies (zero-dependency by default). Raise the package you need ' + + 'under OPEN QUESTIONS so the product owner can add it to .claude/lanes.json ' + + 'dependencies.allow. Do not retry.', + }; +} +function installerLaneDeny(agent, deps) { + return { + deny: + `Blocked for ${agent}: installing dependencies is the ${deps.installers.join('/')} ` + + "lane's responsibility, not yours. Raise it under OPEN QUESTIONS. Do not retry.", + }; +} + // Decide whether a Bash command that touches a package manager is allowed under // the project's dependency policy. Returns: // null -> no package manager involved; caller proceeds normally @@ -178,25 +196,16 @@ function resolveDependencyDecision(cmd, agent, deps) { if (!PACKAGE_MANAGERS.has(mgr)) continue; sawPackageManager = true; - if (!active) { - return { - deny: - `Blocked for ${agent}: package managers are disabled — this project has no ` + - 'approved dependencies (zero-dependency by default). Raise the package you need ' + - 'under OPEN QUESTIONS so the product owner can add it to .claude/lanes.json ' + - 'dependencies.allow. Do not retry.', - }; - } - if (!isInstaller(agent, deps)) { - return { - deny: - `Blocked for ${agent}: installing dependencies is the ${deps.installers.join('/')} ` + - "lane's responsibility, not yours. Raise it under OPEN QUESTIONS. Do not retry.", - }; - } - - // npx runs (and may fetch) a package — gate its target like an install. + // Classify the command FIRST — only *installs* (and `npx`, which fetches/ + // runs a package) are gated by the active/installer/allowlist policy. + // Non-install runs (`npm run`, `npm test`, `npm ls`, `pnpm exec`, …) are + // plain script execution and are allowed for ANY lane, regardless of + // whether dependencies are active. Gating them here was the bug that stopped + // frontend/QA lanes from building or testing frontend-only projects. if (mgr === 'npx') { + // npx runs (and may fetch) a package — gate it like an install. + if (!active) return zeroDepDeny(agent); + if (!isInstaller(agent, deps)) return installerLaneDeny(agent, deps); const target = tokens.slice(1).find((t) => !t.startsWith('-')); if (target && !deps.allow.includes(stripVersion(target))) { return { @@ -213,7 +222,11 @@ function resolveDependencyDecision(cmd, agent, deps) { const installSubs = INSTALL_SUBCMDS[mgr] || new Set(); // Bare `yarn` (no subcommand) installs from the manifest. const isInstall = sub ? installSubs.has(sub) : mgr === 'yarn'; - if (!isInstall) continue; // `npm run`, `npm test`, `npm ls`, … — allowed when active. + if (!isInstall) continue; // `npm run`, `npm test`, `npm ls`, … — allowed for any lane. + + // From here down the command installs packages — apply the full policy. + if (!active) return zeroDepDeny(agent); + if (!isInstaller(agent, deps)) return installerLaneDeny(agent, deps); const rest = tokens.slice(2); if (rest.includes('-r') || rest.includes('--requirement') || rest.includes('-e')) { @@ -314,7 +327,7 @@ function run(raw) { if (!agent) return; // orchestrator Bash is not policed here const cmd = String(args.command || ''); const depDecision = resolveDependencyDecision(cmd, agent, deps); - if (depDecision && depDecision.deny) deny(depDecision.reason); + if (depDecision && depDecision.deny) deny(depDecision.deny); // A permitted package-manager command still falls through to the other // Bash heuristics below (e.g. it must not also `git push`). for (const { re, why } of cfg.bashDeny) { diff --git a/plugin/hooks/enforce-lanes.test.js b/plugin/hooks/enforce-lanes.test.js new file mode 100644 index 0000000..f634323 --- /dev/null +++ b/plugin/hooks/enforce-lanes.test.js @@ -0,0 +1,152 @@ +'use strict'; + +// Tests for the dependency-policy path of enforce-lanes.js. Each case feeds a +// simulated PreToolUse payload on stdin (the hook's own contract) and inspects +// the JSON deny response, if any. Run: node --test plugin/hooks/enforce-lanes.test.js +// +// Regression coverage for two fixed bugs: +// 1. non-install package-manager commands were gated by the installer lane +// 2. the deny reason was dropped, so agents saw a blank denial + +const { test } = require('node:test'); +const assert = require('node:assert'); +const { spawnSync } = require('node:child_process'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const HOOK = path.join(__dirname, 'enforce-lanes.js'); + +// A project dir with an ACTIVE allowlist (vite, left-pad approved; backend is +// the installer lane). +function activeProject() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'lanes-active-')); + fs.mkdirSync(path.join(dir, '.claude')); + fs.writeFileSync( + path.join(dir, '.claude', 'lanes.json'), + JSON.stringify({ + dependencies: { allow: ['vite', 'left-pad'], installers: ['backend-developer'] }, + }), + ); + return dir; +} + +// A zero-dependency project dir (empty allowlist). +function zeroDepProject() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'lanes-zerodep-')); + fs.mkdirSync(path.join(dir, '.claude')); + fs.writeFileSync( + path.join(dir, '.claude', 'lanes.json'), + JSON.stringify({ dependencies: { allow: [], installers: ['backend-developer'] } }), + ); + return dir; +} + +// Run the hook against a Bash command as `agent`, in project `dir`. Returns the +// parsed hook output ({} when the hook allows / stays silent). +function runHook(dir, agent, command) { + const payload = JSON.stringify({ + tool_name: 'Bash', + agent_type: agent, + tool_input: { command }, + }); + const res = spawnSync(process.execPath, [HOOK], { + input: payload, + env: { ...process.env, CLAUDE_PROJECT_DIR: dir }, + encoding: 'utf8', + }); + assert.strictEqual(res.status, 0, 'hook must always exit 0 (fail-open posture)'); + const out = res.stdout.trim(); + return out ? JSON.parse(out) : {}; +} + +function isDeny(out) { + return out.hookSpecificOutput && out.hookSpecificOutput.permissionDecision === 'deny'; +} +function reasonOf(out) { + return out.hookSpecificOutput && out.hookSpecificOutput.permissionDecisionReason; +} + +test('non-installer running a non-install command is allowed (Bug 1)', () => { + const dir = activeProject(); + try { + for (const cmd of ['npm run build', 'npm run dev', 'npm test', 'npm ls']) { + const out = runHook(dir, 'frontend-developer', cmd); + assert.ok(!isDeny(out), `expected "${cmd}" to be allowed, got deny: ${reasonOf(out)}`); + } + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('non-installer running an install is denied WITH a reason (Bug 2)', () => { + const dir = activeProject(); + try { + const out = runHook(dir, 'frontend-developer', 'npm install left-pad'); + assert.ok(isDeny(out), 'expected a deny for a non-installer install'); + assert.ok(reasonOf(out) && reasonOf(out).length > 0, 'deny reason must be non-empty'); + assert.match(reasonOf(out), /backend-developer/, 'reason names the installer lane'); + assert.match(reasonOf(out), /OPEN QUESTIONS/, 'reason steers to OPEN QUESTIONS'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('installer installing an allowlisted package is allowed', () => { + const dir = activeProject(); + try { + const out = runHook(dir, 'backend-developer', 'npm install left-pad'); + assert.ok(!isDeny(out), `expected allow, got deny: ${reasonOf(out)}`); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('installer installing a non-allowlisted package is denied WITH a reason', () => { + const dir = activeProject(); + try { + const out = runHook(dir, 'backend-developer', 'npm install express'); + assert.ok(isDeny(out), 'expected a deny for a non-allowlisted install'); + assert.match(reasonOf(out), /express/, 'reason names the offending package'); + assert.match(reasonOf(out), /allowlist/, 'reason mentions the allowlist'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('npx is still gated by the installer lane WITH a reason', () => { + const dir = activeProject(); + try { + const out = runHook(dir, 'frontend-developer', 'npx vite'); + assert.ok(isDeny(out), 'expected npx to be gated for a non-installer'); + assert.ok(reasonOf(out) && reasonOf(out).length > 0, 'deny reason must be non-empty'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('npx of a non-allowlisted target is denied for the installer WITH a reason', () => { + const dir = activeProject(); + try { + const out = runHook(dir, 'backend-developer', 'npx cowsay hi'); + assert.ok(isDeny(out), 'expected npx of a non-allowlisted target to be denied'); + assert.match(reasonOf(out), /cowsay/, 'reason names the npx target'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('zero-dep project allows a non-install run but denies an install WITH a reason', () => { + const dir = zeroDepProject(); + try { + const run = runHook(dir, 'frontend-developer', 'npm test'); + assert.ok(!isDeny(run), `expected "npm test" allowed in zero-dep, got: ${reasonOf(run)}`); + + const install = runHook(dir, 'frontend-developer', 'npm install left-pad'); + assert.ok(isDeny(install), 'expected an install to be denied in a zero-dep project'); + assert.ok(reasonOf(install) && reasonOf(install).length > 0, 'deny reason must be non-empty'); + assert.match(reasonOf(install), /OPEN QUESTIONS/, 'reason steers to OPEN QUESTIONS'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +});