diff --git a/.changeset/review-current-permissions.md b/.changeset/review-current-permissions.md new file mode 100644 index 0000000..6da9bdb --- /dev/null +++ b/.changeset/review-current-permissions.md @@ -0,0 +1,5 @@ +--- +'@tanstack/intent': minor +--- + +Add `intent install --review` to revisit existing skill permissions, inspect current access, and confirm additions, removals, and individual exclusions. Preserve undiscovered rules and inherited permissions unless explicitly changed. Keep default configured installs guidance-only, reuse one discovery scan throughout review, and compile permission rules once per selection pass. diff --git a/docs/cli/intent-install.md b/docs/cli/intent-install.md index 97dba3c..91fcc85 100644 --- a/docs/cli/intent-install.md +++ b/docs/cli/intent-install.md @@ -11,6 +11,10 @@ npx @tanstack/intent@latest install [--map] [--dry-run] [--print-prompt] [--glob ## Options +### Permission review + +- `--review`: review current skill permissions interactively, then update guidance + ### Guidance output - `--map`: write explicit task-to-skill mappings instead of lightweight loading guidance @@ -27,7 +31,7 @@ npx @tanstack/intent@latest install [--map] [--dry-run] [--print-prompt] [--glob ### Default install -If `intent.skills` is already configured, including through workspace inheritance, `install` only updates guidance. It does not prompt or change `package.json`. +If `intent.skills` is already configured, including through workspace inheritance, `install` only updates guidance. It does not prompt or change `package.json`. Run `intent install --review` to change permissions. Otherwise, first-run setup requires an interactive terminal. Non-TTY execution fails before discovery or writes. Node.js 20.12.0 or newer is required. @@ -66,6 +70,27 @@ After permissions are saved, Intent updates an existing managed guidance block i - **Decline or cancel a prompt:** writes neither permissions nor guidance. - **`--dry-run`:** performs discovery and selection, previews permissions and guidance, and writes neither file. + +### Review existing permissions + +```bash +npx @tanstack/intent@latest install --review +``` + +Review starts from the current `intent.skills` rules. Continue with them, add packages/scopes/individual skills, remove explicit rules, or review individual skills within enabled packages. Existing rules stay intact unless you change them, including rules for packages or skills that are **not discovered**. Removing a rule requires unchecking it; Intent never removes it automatically. + +**Inspect access and descriptions** shows whether each current candidate is permitted by a matching rule or blocked by the allowlist or `intent.exclude`. Searchable lists show at most six options at a time; descriptions appear on request. Package and scope rules continue to cover future matching skills. Adding a skill already covered by an existing rule does not add a redundant permission. + +Unchecking a skill covered by a broader rule adds an exclusion. Existing exclusions stay in effect and cannot be removed through this picker; use [`intent exclude`](./intent-exclude) from the directory containing the exclusion to remove one. + +The confirmation previews the destination, additions, removals, and new exclusions. Choose **Show exact proposed configuration** in the review menu for complete arrays. Canceling writes neither permissions nor guidance. `--review --dry-run` walks through review and prints the preview without saving either file. + +In a workspace, inherited permissions are the starting selection. If you change them, confirmation creates an override in the nearest owning `package.json`; it does not edit the ancestor. Continuing unchanged preserves inheritance. Inherited exclusions still apply. If a policy manifest changes during review, the command stops and asks you to retry. + +Review requires a terminal and cannot be combined with `--map`, `--print-prompt`, `--global`, or `--global-only`. With no effective policy, `--review` opens first-run setup. Plain `install` retains its guidance-only behavior for configured projects. + +Review scans local candidates once and reuses that result throughout the prompts and completion counts. It compares current permissions with proposed edits. It does **not** detect newly discovered skills relative to an earlier run, content changes, hashes, or delivery drift. Permissions and guidance results are reported separately; a guidance failure after saving does not undo confirmed permissions. + ### Mapping mode - Scans packages and writes compact `id`, `run`, and `for` mappings only when `--map` is passed. diff --git a/docs/concepts/trust-model.md b/docs/concepts/trust-model.md index f50b058..f4e771f 100644 --- a/docs/concepts/trust-model.md +++ b/docs/concepts/trust-model.md @@ -48,6 +48,12 @@ When no effective policy exists, `intent install` follows this flow: The completion summary reports skills available under the saved policy. It does not prove that an agent loaded or applied them. See [Default install](../cli/intent-install#default-install) for picker controls and permission choices. +## Revisiting permissions + +Run `intent install --review` to revisit current permissions. Existing decisions are retained until you confirm changes. The review can add permission rules, remove selected rules, and add individual exclusions under broader rules. Existing exclusions continue to win. + +A review inside a workspace starts with inherited permissions. Confirmed edits create a local override; an unchanged review preserves inheritance. This reviews permission configuration, not whether skill content has changed. See [Review existing permissions](../cli/intent-install#review-existing-permissions). + ## Static discovery Intent reads package data as files. It never imports, requires, or executes the code of a discovered package to find or load a skill. Adding a package to your dependency tree cannot run that package's code through Intent. diff --git a/packages/intent/src/cli.ts b/packages/intent/src/cli.ts index 20630ad..d3c3432 100644 --- a/packages/intent/src/cli.ts +++ b/packages/intent/src/cli.ts @@ -126,8 +126,9 @@ function createCli(runtime: InstallCommandRuntime = {}): CAC { 'Create or update skill loading guidance in an agent config file', ) .usage( - 'install [--map] [--dry-run] [--print-prompt] [--global] [--global-only] [--no-notices]', + 'install [--review] [--map] [--dry-run] [--print-prompt] [--global] [--global-only] [--no-notices]', ) + .option('--review', 'Review and change skill permissions interactively') .option('--map', 'Write explicit skill-to-task mappings') .option('--dry-run', 'Print the generated block without writing') .option( @@ -138,6 +139,7 @@ function createCli(runtime: InstallCommandRuntime = {}): CAC { .option('--global-only', 'Install mappings from global packages only') .option('--no-notices', 'Suppress non-critical notices on stderr') .example('install') + .example('install --review') .example('install --map') .example('install --dry-run') .example('install --print-prompt') diff --git a/packages/intent/src/commands/install/command.ts b/packages/intent/src/commands/install/command.ts index cad4def..37f3526 100644 --- a/packages/intent/src/commands/install/command.ts +++ b/packages/intent/src/commands/install/command.ts @@ -131,6 +131,7 @@ tanstackIntent: export interface InstallCommandOptions extends GlobalScanFlags { dryRun?: boolean map?: boolean + review?: boolean printPrompt?: boolean } @@ -211,6 +212,19 @@ export async function runInstallCommand( scanIntentsOrFail: (coreOptions?: IntentCoreOptions) => Promise, runtime: InstallCommandRuntime = {}, ): Promise { + if ( + options.review && + (options.map || options.printPrompt || options.global || options.globalOnly) + ) { + fail( + '--review cannot be combined with --map, --print-prompt, --global, or --global-only.', + ) + } + if (options.review && !(runtime.isTTY ?? process.stdin.isTTY === true)) { + fail( + 'Permission review requires an interactive terminal. Run `intent install --review` in a terminal.', + ) + } if (options.printPrompt) { console.log(INSTALL_PROMPT) return @@ -225,7 +239,7 @@ export async function runInstallCommand( ReturnType > | null = null - if (policy.mode === 'absent') { + if (policy.mode === 'absent' || options.review) { const isTTY = runtime.isTTY ?? process.stdin.isTTY === true if (!isTTY) { fail( @@ -236,6 +250,7 @@ export async function runInstallCommand( try { permissions = await setupInitialPermissions({ dryRun: options.dryRun, + review: options.review, root: process.cwd(), runtime: { prompts: runtime.permissionPrompts ?? createPermissionPrompts(), @@ -273,7 +288,8 @@ export async function runInstallCommand( return } - const available = permissions ? await scanIntentsOrFail() : null + const available = + permissions && !permissions.available ? await scanIntentsOrFail() : null try { const result = writeIntentSkillsBlock({ ...generated, @@ -305,26 +321,25 @@ export async function runInstallCommand( } else { printWriteResult(result) } + if (!permissions) + console.log('To change permissions, run intent install --review.') printPlacementTip(result.targetPath) - if (available && permissions) { - const packages = available.packages.filter( - (pkg) => pkg.skills.length > 0, - ) - const skillCount = packages.reduce( - (count, pkg) => count + pkg.skills.length, - 0, - ) + if (permissions && (available || permissions.available)) { + const packages = + available?.packages.filter((pkg) => pkg.skills.length > 0) ?? [] + const packageCount = permissions.available?.packages ?? packages.length + const skillCount = + permissions.available?.skills ?? + packages.reduce((count, pkg) => count + pkg.skills.length, 0) console.log( - `Available: ${skillCount} ${skillCount === 1 ? 'skill' : 'skills'} from ${packages.length} ${packages.length === 1 ? 'package' : 'packages'}.`, + `Available: ${skillCount} ${skillCount === 1 ? 'skill' : 'skills'} from ${packageCount} ${packageCount === 1 ? 'package' : 'packages'}.`, ) if (skillCount > 0) { console.log( `Next: ${formatIntentCommand(detectIntentCommandPackageManager(), 'list')}`, ) } else { - console.log( - `To enable skills, edit intent.skills in ${formatTargetPath(permissions.packageJsonPath)} and run intent install again.`, - ) + console.log('To change permissions, run intent install --review.') } } return diff --git a/packages/intent/src/commands/install/permission-prompts.ts b/packages/intent/src/commands/install/permission-prompts.ts index 8df020a..2f7642e 100644 --- a/packages/intent/src/commands/install/permission-prompts.ts +++ b/packages/intent/src/commands/install/permission-prompts.ts @@ -7,6 +7,9 @@ import { isCancel, select, } from '@clack/prompts' +import { compileExcludePatterns, isSkillExcluded } from '../../core/excludes.js' +import { parseSkillSources } from '../../core/skill-sources.js' +import { compileSkillSourcePolicy } from '../../core/source-policy.js' import { selectedPermissionSkills } from './permissions.js' import type { PermissionPackage, PermissionPrompts } from './permissions.js' @@ -48,7 +51,7 @@ export function createPermissionPrompts( return true } - return { + const prompts: PermissionPrompts = { selectPermissions: async (packages) => { const availablePackages = packages.filter((pkg) => pkg.skills.some((skill) => !skill.excluded), @@ -169,14 +172,198 @@ export function createPermissionPrompts( } } }, + editPermissions: async (packages, initial) => { + let selection = { + skills: [...initial.skills], + exclude: [...initial.exclude], + } + const allSkills = packages.flatMap((pkg) => pkg.skills) + for (;;) { + const action = await runtime.select({ + ...picker, + message: 'Review current skill permissions', + options: [ + { value: 'continue', label: 'Continue to confirmation' }, + { value: 'add', label: 'Add packages, scopes, or skills' }, + { value: 'remove', label: 'Remove permission rules' }, + { value: 'skills', label: 'Review individual skills' }, + { value: 'inspect', label: 'Inspect access and descriptions' }, + { value: 'preview', label: 'Show exact proposed configuration' }, + ], + }) + if (canceled(action)) return null + if (action === 'continue') return selection + if (action === 'add') { + if (!allSkills.some((skill) => !skill.excluded)) { + console.log( + 'No selectable skills discovered. Install a package with skills or review intent.exclude, then retry.', + ) + continue + } + const added = await prompts.selectPermissions(packages) + if (added === null) return null + // Preserve existing rules verbatim, including rules not discovered today. + const existing = new Set(selection.skills) + const policy = compileSkillSourcePolicy( + parseSkillSources(selection.skills), + ) + const additions = added.filter((rule) => { + if (existing.has(rule)) return false + const parsed = parseSkillSources([rule]) + if (parsed.mode !== 'explicit') + return ( + parsed.mode === 'allow-all' && !selection.skills.includes('*') + ) + const source = parsed.sources[0]! + if (source.kind === 'git' || 'pattern' in source) + return !selection.skills.includes('*') + // A collection of exact skills does not grant future package skills. + return ( + !policy.matchers.some( + (matcher) => + matcher.matchesPackage(source.id, source.kind) && + (!('skill' in matcher.source) || + matcher.source.skill === undefined || + matcher.source.skill === source.skill), + ) && !selection.skills.includes('*') + ) + }) + selection = { + ...selection, + skills: [...selection.skills, ...additions], + } + } else if (action === 'remove') { + if (selection.skills.length === 0) { + console.log( + 'No permission rules to remove. Add packages or skills to enable them.', + ) + continue + } + const policy = compileSkillSourcePolicy( + parseSkillSources(selection.skills.filter((rule) => rule !== '*')), + ) + const kept = await runtime.autocompleteMultiselect({ + ...picker, + message: 'Keep permission rules — uncheck to remove', + options: selection.skills.map((rule) => { + const matcher = policy.matchers.find( + (entry) => entry.source.raw === rule, + ) + const skillName = + matcher && 'skill' in matcher.source + ? matcher.source.skill + : undefined + const discovered = + rule === '*' || + packages.some((pkg) => { + const kind = pkg.id.startsWith('workspace:') + ? 'workspace' + : 'npm' + const name = pkg.id.replace(/^workspace:/, '') + return ( + matcher?.matchesPackage(name, kind) && + (skillName === undefined || + pkg.skills.some((skill) => skill.name === skillName)) + ) + }) + return { + value: rule, + label: rule, + hint: discovered + ? undefined + : 'Not discovered; kept unless you remove it', + } + }), + initialValues: selection.skills, + required: false, + }) + if (canceled(kept)) return null + const retained = new Set(kept) + selection = { + ...selection, + skills: selection.skills.filter((rule) => retained.has(rule)), + } + } else if (action === 'skills') { + const reviewed = await prompts.reviewPermissions(packages, selection) + if (reviewed === null) return null + selection = reviewed + } else if (action === 'preview') { + console.log( + `intent.skills: ${JSON.stringify(selection.skills, null, 2)}`, + ) + console.log( + `Add to intent.exclude: ${JSON.stringify(selection.exclude, null, 2)}`, + ) + console.log( + 'Existing exclusions remain in effect. Skill content changes are not tracked.', + ) + } else { + const config = parseSkillSources(selection.skills) + const policy = compileSkillSourcePolicy(config) + const excludes = compileExcludePatterns(selection.exclude) + const reasons = new Map() + for (const pkg of packages) { + const kind = pkg.id.startsWith('workspace:') ? 'workspace' : 'npm' + const name = pkg.id.replace(/^workspace:/, '') + for (const skill of pkg.skills) { + const matcher = policy.matchers.find( + (entry) => + entry.matchesPackage(name, kind) && + (!('skill' in entry.source) || + entry.source.skill === undefined || + entry.source.skill === skill.name), + ) + reasons.set( + skill.id, + skill.excluded || isSkillExcluded(name, skill.name, excludes) + ? 'Blocked by intent.exclude' + : config.mode === 'allow-all' + ? 'Permitted by * (all sources)' + : config.mode === 'empty' + ? 'Blocked by intent.skills: []' + : matcher + ? `Permitted by ${matcher.source.raw}` + : 'Blocked: no matching intent.skills rule', + ) + } + } + for (;;) { + const skill = await runtime.autocomplete< + PermissionPackage['skills'][number] | 'back' + >({ + ...searchablePicker, + message: 'Inspect current access — type to filter', + options: [ + { value: 'back', label: 'Back to permission review' }, + ...allSkills.map((entry) => ({ + value: entry, + label: entry.id, + hint: reasons.get(entry.id), + })), + ], + }) + if (canceled(skill)) return null + if (skill === 'back') break + console.log(`${skill.id} — ${reasons.get(skill.id)}`) + console.log(stripVTControlCharacters(skill.description)) + } + } + } + }, reviewPermissions: async (packages, selection) => { - const reviewable = packages.filter( - (pkg) => - selectedPermissionSkills([pkg], { - skills: selection.skills, - exclude: [], - }).length > 0, + const reviewableIds = new Set( + selectedPermissionSkills(packages, { + skills: selection.skills, + exclude: [], + }).map((skill) => skill.id.slice(0, skill.id.indexOf('#'))), ) + const reviewable = packages.filter((pkg) => reviewableIds.has(pkg.id)) + if (reviewable.length === 0) { + console.log( + 'No enabled packages have selectable skills. Add packages or skills first; existing exclusions still apply.', + ) + return selection + } const packageIds = await runtime.autocompleteMultiselect({ ...picker, message: 'Review individual skills — choose packages to review', @@ -190,8 +377,9 @@ export function createPermissionPrompts( const selected = new Set( selectedPermissionSkills(packages, selection).map((skill) => skill.id), ) + const reviewedPackages = new Set(packageIds) for (const pkg of reviewable.filter((pkg) => - packageIds.includes(pkg.id), + reviewedPackages.has(pkg.id), )) { const skills = await runtime.autocompleteMultiselect({ ...picker, @@ -218,15 +406,51 @@ export function createPermissionPrompts( (skill) => skill.id, ), ) + const reviewedIds = new Set( + packages + .filter((pkg) => reviewedPackages.has(pkg.id)) + .flatMap((pkg) => + pkg.skills + .filter((skill) => !skill.excluded) + .map((skill) => skill.id), + ), + ) + const retainedIds = new Set() + const retainedRules = selection.skills.filter((rule) => { + if (!rule.includes('#')) return true + const config = parseSkillSources([rule]) + if (config.mode !== 'explicit') return true + const source = config.sources[0]! + if (source.kind === 'git' || 'pattern' in source) return true + const id = `${source.kind === 'workspace' ? 'workspace:' : ''}${source.id}#${source.skill}` + // Keep undiscovered and excluded rules, and preserve unchanged raw entries. + if (reviewedIds.has(id) && !selected.has(id)) return false + retainedIds.add(id) + return true + }) + const reviewedExcludes = new Set( + [...reviewedIds].map((id) => id.replace(/^workspace:/, '')), + ) return { - skills: [...broad, ...[...selected].filter((id) => !covered.has(id))], + skills: [ + ...retainedRules, + ...[...selected].filter( + (id) => + reviewedIds.has(id) && !covered.has(id) && !retainedIds.has(id), + ), + ], // Exclusions are package-name based for both npm and workspace sources. - exclude: [...covered] - .filter((id) => !selected.has(id)) - .map((id) => id.replace(/^workspace:/, '')), + exclude: [ + ...new Set([ + ...selection.exclude.filter((id) => !reviewedExcludes.has(id)), + ...[...covered] + .filter((id) => !selected.has(id)) + .map((id) => id.replace(/^workspace:/, '')), + ]), + ], } }, - confirmWrite: async (denyAll) => { + confirmWrite: async (denyAll, review = false) => { const result = await runtime.select({ ...picker, message: denyAll @@ -238,11 +462,20 @@ export function createPermissionPrompts( value: 'save', label: denyAll ? 'Disable all skills' - : 'Continue with all selected skills', + : review + ? 'Save permissions' + : 'Continue with all selected skills', }, - ...(denyAll + ...(denyAll && !review ? [] - : [{ value: 'review', label: 'Review individual skills' }]), + : [ + { + value: 'review', + label: review + ? 'Review permissions' + : 'Review individual skills', + }, + ]), { value: 'cancel', label: 'Cancel' }, ], }) @@ -250,4 +483,5 @@ export function createPermissionPrompts( return result === 'review' ? 'review' : result === 'save' }, } + return prompts } diff --git a/packages/intent/src/commands/install/permissions.ts b/packages/intent/src/commands/install/permissions.ts index 65a2b1b..4637aff 100644 --- a/packages/intent/src/commands/install/permissions.ts +++ b/packages/intent/src/commands/install/permissions.ts @@ -1,13 +1,20 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { readPackageJson } from '../../core/package-json.js' import { compileExcludePatterns, + getConfigDirs, getEffectiveExcludePatterns, isPackageExcluded, isSkillExcluded, } from '../../core/excludes.js' import { parseSkillSources } from '../../core/skill-sources.js' -import { isSourcePermitted } from '../../core/source-policy.js' +import { + compileSkillSourcePolicy, + isSourcePermitted, +} from '../../core/source-policy.js' import { resolveProjectContext } from '../../core/project-context.js' -// First-run permission setup must show unpoliced candidates for explicit review. +// Permission setup and review must show unpoliced candidates for human selection. // eslint-disable-next-line no-restricted-imports import { scanForIntents } from '../../discovery/scanner.js' import { @@ -45,7 +52,14 @@ export interface PermissionPrompts { packages: Array, selection: PermissionSelection, ) => Promise - confirmWrite: (denyAll: boolean) => Promise + editPermissions: ( + packages: Array, + selection: PermissionSelection, + ) => Promise + confirmWrite: ( + denyAll: boolean, + review?: boolean, + ) => Promise } export interface PermissionSetupRuntime { @@ -56,7 +70,11 @@ export interface PermissionSetupRuntime { export type PermissionSetupResult = | { status: 'canceled' } | { status: 'unavailable' } - | { packageJsonPath: string; status: 'unchanged' | 'updated' } + | { + packageJsonPath: string + status: 'unchanged' | 'updated' + available?: { skills: number; packages: number } + } function selectorForPackage(pkg: IntentPackage): string { return pkg.kind === 'workspace' ? `workspace:${pkg.name}` : pkg.name @@ -89,7 +107,7 @@ export function selectedPermissionSkills( packages: Array, selection: PermissionSelection, ): Array { - const config = parseSkillSources(selection.skills) + const policy = compileSkillSourcePolicy(parseSkillSources(selection.skills)) const excludes = compileExcludePatterns(selection.exclude) return packages.flatMap((pkg) => { const kind = pkg.id.startsWith('workspace:') ? 'workspace' : 'npm' @@ -98,7 +116,7 @@ export function selectedPermissionSkills( return pkg.skills.filter( (skill) => !skill.excluded && - isSourcePermitted(config, name, kind, skill.name) && + policy.permits(name, kind, skill.name) && !isSkillExcluded(name, skill.name, excludes), ) }) @@ -127,10 +145,12 @@ function normalizePermissions(selected: Array): Array { export async function setupInitialPermissions({ dryRun = false, + review = false, root, runtime, }: { dryRun?: boolean + review?: boolean root: string runtime: PermissionSetupRuntime }): Promise { @@ -141,6 +161,40 @@ export async function setupInitialPermissions({ ) } + const originalManifest = readFileSync(context.targetPackageJsonPath, 'utf8') + const policyManifests = review + ? getConfigDirs(root, context).map((dir) => ({ + dir, + manifest: readPackageJson(dir), + })) + : [] + const assertPolicyUnchanged = () => { + if ( + policyManifests.some( + ({ dir, manifest }) => + JSON.stringify(readPackageJson(dir)) !== JSON.stringify(manifest), + ) + ) { + throw new Error( + 'Project policy changed during permission review. Run intent install --review again.', + ) + } + } + let initial: Array | undefined + let inheritedFrom: string | undefined + if (review) { + for (const { dir, manifest } of policyManifests) { + const intent = manifest?.intent as Record | undefined + if (intent?.skills === undefined || intent.skills === null) continue + parseSkillSources(intent.skills) + initial = intent.skills as Array + if (join(dir, 'package.json') !== context.targetPackageJsonPath) { + inheritedFrom = join(dir, 'package.json') + } + break + } + } + const scan = ( runtime.scan ?? ((cwd) => scanForIntents(cwd, { scope: 'local' })) )(root) @@ -159,7 +213,7 @@ export async function setupInitialPermissions({ console.log( `Found ${discoveredSkills.length} skill${discoveredSkills.length === 1 ? '' : 's'} in ${packages.length} package${packages.length === 1 ? '' : 's'}.`, ) - if (availableSkillCount === 0) { + if (availableSkillCount === 0 && initial === undefined) { console.log( discoveredSkills.length === 0 ? 'No intent-enabled skills found. Install a package that ships skills, then run intent install again.' @@ -175,16 +229,41 @@ export async function setupInitialPermissions({ ) } console.log('Skills can change when dependencies update.') - const selected = await runtime.prompts.selectPermissions(candidates) - if (selected === null) return { status: 'canceled' } - let selection: PermissionSelection = { skills: selected, exclude: [] } + if (inheritedFrom) { + console.log( + `Current permissions inherited from ${inheritedFrom}. Confirming changes creates a local override in ${context.targetPackageJsonPath}; inherited exclusions still apply.`, + ) + } + let selection: PermissionSelection + if (initial !== undefined) { + const edited = await runtime.prompts.editPermissions(candidates, { + skills: [...initial], + exclude: [], + }) + if (edited === null) return { status: 'canceled' } + selection = edited + } else { + const selected = await runtime.prompts.selectPermissions(candidates) + if (selected === null) return { status: 'canceled' } + selection = { skills: selected, exclude: [] } + } for (;;) { - const skills = normalizePermissions(selection.skills) + const skills = + initial === undefined + ? normalizePermissions(selection.skills) + : selection.skills + parseSkillSources(skills) const update = preparePackageSkillsUpdate( context.targetPackageJsonPath, skills, selection.exclude, ) + assertPolicyUnchanged() + if (update.source !== originalManifest) { + throw new Error( + 'Project policy changed during permission review. Run intent install --review again.', + ) + } const enabled = selectedPermissionSkills(candidates, selection) const packageCount = new Set(enabled.map((skill) => skill.id.split('#')[0])) .size @@ -213,8 +292,20 @@ export async function setupInitialPermissions({ `${label}: ${JSON.stringify(values.slice(0, 6))}${values.length > 6 ? ` (+${values.length - 6} more)` : ''}`, ) } - if (skills.length === 1 && skills[0] === '*') - printNotices([ALLOW_ALL_NOTICE]) + if (initial !== undefined) { + const before = new Set(initial) + const after = new Set(skills) + for (const [label, entries] of [ + ['Add permissions', skills.filter((rule) => !before.has(rule))], + ['Remove permissions', initial.filter((rule) => !after.has(rule))], + ] as const) { + if (entries.length > 0) + console.log( + `${label}: ${JSON.stringify(entries.slice(0, 6))}${entries.length > 6 ? ` (+${entries.length - 6} more; choose Show exact proposed configuration in review)` : ''}`, + ) + } + } + if (skills.includes('*')) printNotices([ALLOW_ALL_NOTICE]) if (dryRun) { return { @@ -222,12 +313,15 @@ export async function setupInitialPermissions({ status: 'unchanged', } } - const confirmation = await runtime.prompts.confirmWrite(skills.length === 0) + const confirmation = await (initial === undefined + ? runtime.prompts.confirmWrite(skills.length === 0) + : runtime.prompts.confirmWrite(skills.length === 0, true)) if (confirmation === 'review') { - const reviewed = await runtime.prompts.reviewPermissions( - candidates, - selection, - ) + const reviewed = await ( + initial === undefined + ? runtime.prompts.reviewPermissions + : runtime.prompts.editPermissions + )(candidates, selection) if (reviewed === null) return { status: 'canceled' } selection = reviewed continue @@ -236,9 +330,19 @@ export async function setupInitialPermissions({ console.log('Permissions: canceled.') return { status: 'canceled' } } + assertPolicyUnchanged() + const unchanged = + initial !== undefined && + JSON.stringify(skills) === JSON.stringify(initial) && + selection.exclude.length === 0 return { packageJsonPath: context.targetPackageJsonPath, - status: writePreparedPackageSkillsUpdate(update), + status: unchanged + ? 'unchanged' + : writePreparedPackageSkillsUpdate(update), + ...(review + ? { available: { skills: enabled.length, packages: packageCount } } + : {}), } } } diff --git a/packages/intent/src/core/source-policy.ts b/packages/intent/src/core/source-policy.ts index 48cf284..409ad06 100644 --- a/packages/intent/src/core/source-policy.ts +++ b/packages/intent/src/core/source-policy.ts @@ -79,7 +79,7 @@ function compileSkillSourceMatcher( } } -function compileSkillSourcePolicy(config: SkillSourcesConfig): { +export function compileSkillSourcePolicy(config: SkillSourcesConfig): { matchers: Array permits: ( packageName: string, diff --git a/packages/intent/tests/cli.test.ts b/packages/intent/tests/cli.test.ts index a2f6c5f..f08d02f 100644 --- a/packages/intent/tests/cli.test.ts +++ b/packages/intent/tests/cli.test.ts @@ -11,6 +11,7 @@ import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import { fileURLToPath, pathToFileURL } from 'node:url' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import * as discovery from '../src/discovery/scanner.js' import { INSTALL_PROMPT } from '../src/commands/install/command.js' import { isMainModule, main } from '../src/cli.js' import type { PermissionPrompts } from '../src/commands/install/permissions.js' @@ -78,6 +79,7 @@ function permissionPrompts({ reviewPermissions: vi.fn((_groups, selection) => Promise.resolve(selection), ), + editPermissions: vi.fn((_groups, selection) => Promise.resolve(selection)), confirmWrite: vi.fn(async () => confirmWrite), } } @@ -419,6 +421,111 @@ describe('cli commands', () => { expect(readFileSync(agentsPath, 'utf8')).toBe(content) }) + it('reviews existing permissions without replaying first-run selection', async () => { + const root = mkdtempSync(join(realTmpdir, 'intent-cli-repeat-review-')) + tempDirs.push(root) + const packageJsonPath = join(root, 'package.json') + writeJson(packageJsonPath, { + name: 'app', + intent: { skills: ['missing#keep', '@tanstack/query'] }, + }) + writeInstalledIntentPackage(root, { + name: '@tanstack/query', + version: '5.0.0', + skillName: 'fetching', + description: 'Query data fetching patterns', + }) + process.chdir(root) + const scanSpy = vi.spyOn(discovery, 'scanForIntents') + const prompts: PermissionPrompts = { + ...permissionPrompts(), + editPermissions: vi.fn(async (_packages, current) => ({ + ...current, + skills: [...current.skills, 'other'], + })), + } + expect( + await main(['install', '--review'], { + isTTY: true, + permissionPrompts: prompts, + }), + ).toBe(0) + expect(scanSpy).toHaveBeenCalledOnce() + scanSpy.mockRestore() + expect(prompts.selectPermissions).not.toHaveBeenCalled() + expect(prompts.editPermissions).toHaveBeenCalledOnce() + expect( + JSON.parse(readFileSync(packageJsonPath, 'utf8')).intent.skills, + ).toEqual(['missing#keep', '@tanstack/query', 'other']) + expect(logSpy.mock.calls.flat().join('\n')).toContain( + 'Permissions: updated', + ) + expect(readFileSync(join(root, 'AGENTS.md'), 'utf8')).toContain( + '## Skill Loading', + ) + }) + + it.each(['--map', '--print-prompt', '--global', '--global-only'])( + 'rejects --review with %s before prompts or writes', + async (flag) => { + const prompts = permissionPrompts() + expect( + await main(['install', '--review', flag], { + isTTY: true, + permissionPrompts: prompts, + }), + ).toBe(1) + expect(prompts.editPermissions).not.toHaveBeenCalled() + expect(errorSpy.mock.calls.flat().join('\n')).toContain( + '--review cannot be combined', + ) + }, + ) + + it('rejects non-TTY review without discovery or writes', async () => { + const scanSpy = vi.spyOn(discovery, 'scanForIntents') + try { + expect(await main(['install', '--review'], { isTTY: false })).toBe(1) + expect(scanSpy).not.toHaveBeenCalled() + expect(errorSpy.mock.calls.flat().join('\n')).toContain( + 'Permission review requires an interactive terminal', + ) + } finally { + scanSpy.mockRestore() + } + }) + + it.each(['cancel', 'decline', 'dry-run'])( + 'leaves repeat-install policy and guidance untouched on %s', + async (action) => { + const root = mkdtempSync(join(realTmpdir, 'intent-repeat-unchanged-')) + tempDirs.push(root) + const policyPath = join(root, 'package.json') + const source = '{"name":"app","intent":{"skills":["missing"]}}\n' + writeFileSync(policyPath, source) + writeFileSync(join(root, 'AGENTS.md'), 'Existing guidance\n') + process.chdir(root) + const prompts = permissionPrompts({ confirmWrite: action !== 'decline' }) + vi.mocked(prompts.editPermissions).mockResolvedValue( + action === 'cancel' ? null : { skills: [], exclude: [] }, + ) + const args = [ + 'install', + '--review', + ...(action === 'dry-run' ? ['--dry-run'] : []), + ] + expect( + await main(args, { isTTY: true, permissionPrompts: prompts }), + ).toBe(0) + expect(readFileSync(policyPath, 'utf8')).toBe(source) + expect(readFileSync(join(root, 'AGENTS.md'), 'utf8')).toBe( + 'Existing guidance\n', + ) + if (action !== 'decline') + expect(prompts.confirmWrite).not.toHaveBeenCalled() + }, + ) + it('fails without writes when intent.skills is absent in non-TTY use', async () => { const root = mkdtempSync(join(realTmpdir, 'intent-cli-install-non-tty-')) tempDirs.push(root) @@ -569,7 +676,9 @@ describe('cli commands', () => { expect(prompts.confirmWrite).toHaveBeenCalledWith(true) const output = logSpy.mock.calls.flat().join('\n') expect(output).toContain('Available: 0 skills from 0 packages.') - expect(output).toContain('To enable skills, edit intent.skills in') + expect(output).toContain( + 'To change permissions, run intent install --review.', + ) expect(output).not.toContain('Next:') }) diff --git a/packages/intent/tests/permission-prompts.test.ts b/packages/intent/tests/permission-prompts.test.ts index 598add3..d75d53b 100644 --- a/packages/intent/tests/permission-prompts.test.ts +++ b/packages/intent/tests/permission-prompts.test.ts @@ -508,3 +508,201 @@ describe('skill enablement', () => { } }) }) + +describe('repeat permission review', () => { + it('keeps all existing rules verbatim when continuing', async () => { + const api = runtime() + api.select.mockResolvedValue('continue') + const selection = { skills: ['missing#old', '*', 'pkg#core'], exclude: [] } + await expect( + createPermissionPrompts(api).editPermissions([pkg], selection), + ).resolves.toEqual(selection) + expect(api.autocompleteMultiselect).not.toHaveBeenCalled() + }) + + it('adds only uncovered permissions and retains rules not discovered', async () => { + const api = runtime() + api.select + .mockResolvedValueOnce('add') + .mockResolvedValueOnce('skills') + .mockResolvedValueOnce('continue') + api.autocompleteMultiselect.mockResolvedValueOnce([ + 'pkg#core', + 'workspace:pkg#local', + ]) + await expect( + createPermissionPrompts(api).editPermissions([pkg, workspace], { + skills: ['missing#old', 'pkg'], + exclude: [], + }), + ).resolves.toEqual({ + skills: ['missing#old', 'pkg', 'workspace:pkg#local'], + exclude: [], + }) + }) + + it('does not mistake exact permissions for permission to future package skills', async () => { + const api = runtime() + api.select + .mockResolvedValueOnce('add') + .mockResolvedValueOnce('packages') + .mockResolvedValueOnce('continue') + api.autocompleteMultiselect.mockResolvedValueOnce(['pkg']) + await expect( + createPermissionPrompts(api).editPermissions([pkg], { + skills: ['pkg#core'], + exclude: [], + }), + ).resolves.toEqual({ skills: ['pkg#core', 'pkg'], exclude: [] }) + }) + + it('shows missing exact skills as not discovered and removes only unchecked rules', async () => { + const api = runtime() + api.select.mockResolvedValueOnce('remove').mockResolvedValueOnce('continue') + api.autocompleteMultiselect.mockResolvedValueOnce([ + 'pkg#missing', + 'workspace:pkg', + ]) + await expect( + createPermissionPrompts(api).editPermissions([pkg, workspace], { + skills: ['pkg#missing', '*', 'workspace:pkg'], + exclude: [], + }), + ).resolves.toEqual({ + skills: ['pkg#missing', 'workspace:pkg'], + exclude: [], + }) + const options = api.autocompleteMultiselect.mock.calls[0]![0] + expect(options.initialValues).toEqual(['pkg#missing', '*', 'workspace:pkg']) + expect(options.options[0].hint).toBe( + 'Not discovered; kept unless you remove it', + ) + expect(options.options[2].hint).toBeUndefined() + }) + + it('preserves undiscovered, excluded, and unreviewed exact rules during skill review', async () => { + const api = runtime() + api.select.mockResolvedValueOnce('skills').mockResolvedValueOnce('continue') + api.autocompleteMultiselect + .mockResolvedValueOnce(['pkg']) + .mockResolvedValueOnce(['pkg#core']) + await expect( + createPermissionPrompts(api).editPermissions([pkg, workspace], { + skills: ['pkg#missing', 'pkg#private', 'workspace:pkg#local', 'pkg'], + exclude: [], + }), + ).resolves.toEqual({ + skills: ['pkg#missing', 'pkg#private', 'workspace:pkg#local', 'pkg'], + exclude: ['pkg#other'], + }) + }) + + it('keeps raw exact entries and order when skill review makes no changes', async () => { + const api = runtime() + api.autocompleteMultiselect + .mockResolvedValueOnce(['pkg']) + .mockResolvedValueOnce(['pkg#core']) + const initial = { + skills: [' pkg#core ', 'missing#old', 'workspace:pkg#local'], + exclude: ['gone#keep'], + } + await expect( + createPermissionPrompts(api).reviewPermissions([pkg, workspace], initial), + ).resolves.toEqual(initial) + }) + + it('explains exclusions, package rules, source kinds, and deny-all on demand', async () => { + const output = vi.spyOn(console, 'log').mockImplementation(() => {}) + try { + for (const [skills, expected] of [ + [ + ['pkg'], + [ + 'Permitted by pkg', + 'Blocked by intent.exclude', + 'Blocked: no matching intent.skills rule', + ], + ], + [ + ['*'], + [ + 'Permitted by * (all sources)', + 'Blocked by intent.exclude', + 'Permitted by * (all sources)', + ], + ], + [ + [], + [ + 'Blocked by intent.skills: []', + 'Blocked by intent.exclude', + 'Blocked by intent.skills: []', + ], + ], + ] as const) { + const api = runtime() + api.select + .mockResolvedValueOnce('inspect') + .mockResolvedValueOnce('continue') + api.autocomplete.mockResolvedValueOnce('back') + await createPermissionPrompts(api).editPermissions([pkg, workspace], { + skills: [...skills], + exclude: [], + }) + const options = api.autocomplete.mock.calls[0]![0].options + expect(options[1].hint).toBe(expected[0]) + expect(options[3].hint).toBe(expected[1]) + expect(options[4].hint).toBe(expected[2]) + expect(api.autocomplete.mock.calls[0]![0].maxItems).toBe(6) + } + expect(output).not.toHaveBeenCalled() + } finally { + output.mockRestore() + } + }) + + it('cancels pending edits without mutating the initial selection', async () => { + const api = runtime() + api.select + .mockResolvedValueOnce('remove') + .mockResolvedValueOnce(Symbol('cancel')) + api.autocompleteMultiselect.mockResolvedValueOnce([]) + const initial = { skills: ['pkg'], exclude: ['gone#keep'] } + await expect( + createPermissionPrompts(api).editPermissions([pkg], initial), + ).resolves.toBeNull() + expect(initial).toEqual({ skills: ['pkg'], exclude: ['gone#keep'] }) + }) + + it('supports keyboard review without replaying the package picker', async () => { + vi.stubEnv('TERM', 'xterm-256color') + const input = new PassThrough() + const output = Object.assign(new PassThrough(), { columns: 90, rows: 24 }) + let rendered = '' + output.on('data', (data) => { + rendered += data.toString() + }) + const api: ClackPermissionRuntime = { + ...clack, + select: (options) => { + const result = clack.select({ ...options, input, output }) + process.nextTick(() => input.write('\r')) + return result + }, + } + try { + const initial = { skills: ['pkg', 'missing#keep'], exclude: [] } + await expect( + createPermissionPrompts(api).editPermissions([pkg], initial), + ).resolves.toEqual(initial) + expect(stripVTControlCharacters(rendered)).toContain( + 'Review current skill permissions', + ) + expect(stripVTControlCharacters(rendered)).not.toContain('Core guidance') + } finally { + vi.unstubAllEnvs() + input.destroy() + output.destroy() + } + }) +}) diff --git a/packages/intent/tests/permissions.test.ts b/packages/intent/tests/permissions.test.ts index 1180e96..0c36c10 100644 --- a/packages/intent/tests/permissions.test.ts +++ b/packages/intent/tests/permissions.test.ts @@ -88,6 +88,7 @@ function prompts({ reviewPermissions: vi.fn((_groups, selection) => Promise.resolve(selection), ), + editPermissions: vi.fn((_groups, selection) => Promise.resolve(selection)), confirmWrite: vi.fn(async () => confirmWrite), } return result @@ -549,3 +550,157 @@ describe('interactive permission selection', () => { expect(configuredSkills(packageJsonPath)).toBeUndefined() }) }) + +describe('existing permission setup', () => { + it.each( + [[], ['*', 'missing'], ['missing#old', '@scope/*', 'workspace:pkg']].map( + (skills) => ({ skills }), + ), + )('preserves unchanged rules and formatting for %j', async ({ skills }) => { + const root = mkdtempSync(join(tmpdir(), 'intent-review-policy-')) + tempDirs.push(root) + const packageJsonPath = join(root, 'package.json') + const source = JSON.stringify({ name: 'app', intent: { skills } }) + '\n' + writeFileSync(packageJsonPath, source) + const discover = vi.fn(() => scan([])) + const result = await setupInitialPermissions({ + root, + review: true, + runtime: { prompts: prompts(), scan: discover }, + }) + expect(result).toEqual({ + packageJsonPath, + status: 'unchanged', + available: { skills: 0, packages: 0 }, + }) + expect(readFileSync(packageJsonPath, 'utf8')).toBe(source) + expect(discover).toHaveBeenCalledOnce() + }) + + it('retains inherited permissions until an explicitly confirmed local override', async () => { + const root = mkdtempSync(join(tmpdir(), 'intent-review-workspace-')) + tempDirs.push(root) + const child = join(root, 'packages', 'app') + mkdirSync(child, { recursive: true }) + const parentPath = join(root, 'package.json') + const parent = JSON.stringify({ + name: 'repo', + workspaces: ['packages/*'], + intent: { skills: ['pkg'], exclude: ['pkg#private'] }, + }) + writeFileSync(parentPath, parent) + const childPath = join(child, 'package.json') + const childSource = '{"name":"app"}\n' + writeFileSync(childPath, childSource) + const permissionPrompts = prompts() + const runtime = { + prompts: permissionPrompts, + scan: () => scan([packageCandidate('pkg', 'npm', ['core', 'private'])]), + } + const unchanged = await setupInitialPermissions({ + root: child, + review: true, + runtime, + }) + expect(unchanged.status).toBe('unchanged') + expect(readFileSync(childPath, 'utf8')).toBe(childSource) + expect(permissionPrompts.editPermissions).toHaveBeenCalledWith( + expect.any(Array), + { skills: ['pkg'], exclude: [] }, + ) + vi.mocked(permissionPrompts.editPermissions).mockResolvedValue({ + skills: ['*'], + exclude: [], + }) + const changed = await setupInitialPermissions({ + root: child, + review: true, + runtime, + }) + expect(changed).toMatchObject({ + status: 'updated', + available: { skills: 1, packages: 1 }, + }) + expect(JSON.parse(readFileSync(childPath, 'utf8')).intent.skills).toEqual([ + '*', + ]) + expect(readFileSync(parentPath, 'utf8')).toBe(parent) + }) + + it.each(['edit', 'confirm'])( + 'rejects local policy changed during %s without overwriting it', + async (stage) => { + const root = mkdtempSync(join(tmpdir(), 'intent-review-race-')) + tempDirs.push(root) + const packageJsonPath = join(root, 'package.json') + writeFileSync( + packageJsonPath, + '{"name":"app","intent":{"skills":["pkg"]}}', + ) + const replacement = + '{"name":"app","intent":{"skills":[],"exclude":["pkg"]}}' + const permissionPrompts = prompts() + vi.mocked(permissionPrompts.editPermissions).mockImplementation( + async () => { + if (stage === 'edit') writeFileSync(packageJsonPath, replacement) + return { skills: ['*'], exclude: [] } + }, + ) + vi.mocked(permissionPrompts.confirmWrite).mockImplementation(async () => { + if (stage === 'confirm') writeFileSync(packageJsonPath, replacement) + return true + }) + await expect( + setupInitialPermissions({ + root, + review: true, + runtime: { prompts: permissionPrompts, scan: () => scan([]) }, + }), + ).rejects.toThrow('policy changed during permission review') + expect(readFileSync(packageJsonPath, 'utf8')).toBe(replacement) + }, + ) + + it('rejects an inherited exclusion change during confirmation', async () => { + const root = mkdtempSync(join(tmpdir(), 'intent-review-exclusion-race-')) + tempDirs.push(root) + const child = join(root, 'packages', 'app') + mkdirSync(child, { recursive: true }) + const parentPath = join(root, 'package.json') + writeFileSync( + parentPath, + JSON.stringify({ + name: 'repo', + workspaces: ['packages/*'], + intent: { exclude: [] }, + }), + ) + const childPath = join(child, 'package.json') + const source = '{"name":"app","intent":{"skills":["pkg"]}}' + writeFileSync(childPath, source) + const permissionPrompts = prompts() + vi.mocked(permissionPrompts.editPermissions).mockResolvedValue({ + skills: ['*'], + exclude: [], + }) + vi.mocked(permissionPrompts.confirmWrite).mockImplementation(async () => { + writeFileSync( + parentPath, + JSON.stringify({ + name: 'repo', + workspaces: ['packages/*'], + intent: { exclude: ['pkg'] }, + }), + ) + return true + }) + await expect( + setupInitialPermissions({ + root: child, + review: true, + runtime: { prompts: permissionPrompts, scan: () => scan([]) }, + }), + ).rejects.toThrow('policy changed during permission review') + expect(readFileSync(childPath, 'utf8')).toBe(source) + }) +}) diff --git a/packages/intent/tests/review-cost.test.ts b/packages/intent/tests/review-cost.test.ts new file mode 100644 index 0000000..44cb540 --- /dev/null +++ b/packages/intent/tests/review-cost.test.ts @@ -0,0 +1,28 @@ +import { expect, it, vi } from 'vitest' +import * as excludes from '../src/core/excludes.js' +import { selectedPermissionSkills } from '../src/commands/install/permissions.js' + +it('compiles permission rules once per selection pass', () => { + const packages = Array.from({ length: 100 }, (_, i) => ({ + id: `@scope${i}/pkg`, + version: '1.0.0', + skills: Array.from({ length: 10 }, (_, j) => ({ + id: `@scope${i}/pkg#skill${j}`, + name: `skill${j}`, + description: 'Guidance', + excluded: false, + })), + })) + const selection = { + skills: Array.from({ length: 100 }, (_, i) => `@scope${i}/*`), + exclude: [], + } + const compile = vi.spyOn(excludes, 'compileWildcardPattern') + try { + expect(selectedPermissionSkills(packages, selection)).toHaveLength(1000) + + expect(compile.mock.calls.length).toBeLessThanOrEqual(100) + } finally { + compile.mockRestore() + } +})