Add Intent hook enforcement for supported agents#174
Open
LadyBluenotes wants to merge 13 commits into
Open
Conversation
…very evals - Introduce 'hooked-intent' condition in conditions.ts - Implement live-router-hooked-intent task in live-tasks.ts - Create gate.mjs and hook-core.mts for intent hooks management - Add hook-io.mjs for event handling and observation logging - Implement prepare-copilot-home.ts for managing copilot home directory - Update run-copilot-task.ts to support gate state management - Enhance setup-intent-condition.ts to handle hooked-intent - Add tests for hooked-intent functionality in intent-hooks.eval.ts - Update vitest configuration for live concurrency settings
…ocumentation - Added `--hooks` option to `intent install` for lifecycle hook installation. - Updated documentation to reflect new hook options and usage. - Introduced hook adapters for Claude, Codex, and Copilot with project/user scope support. - Implemented tests to verify supported scopes in the adapter registry.
…rns and add skill categorization tests
…ated install command and enhance type definitions
Contributor
📝 WalkthroughWalkthroughThis PR adds a new ChangesIntent CLI, hooks, schema, and eval updates
Sequence Diagram(s)sequenceDiagram
participant agent as AI Agent (e.g. Copilot)
participant hook as gate.mjs / hook runner
participant state as JSONL state file
participant policy as gateDecision()
participant agent2 as Agent continues
agent->>hook: PreToolUse event (stdin JSON)
hook->>state: readObservations()
state-->>hook: prior observations[]
hook->>policy: gateDecision({toolName, hasLoaded})
alt hasLoaded is false AND toolName is edit tool
policy-->>hook: deny + GATE_DENY_REASON
hook->>agent: stdout JSON permissionDecision: deny
else intent list or load command
hook->>state: appendObservation({action, skillUse})
hook->>agent: no output (allow)
else other tool or already loaded
policy-->>hook: allow
hook->>agent2: no output (allow)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit d5c9525
☁️ Nx Cloud last updated this comment at |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@evals/intent-discovery/harness/intent-hooks/hook-core.mjs`:
- Around line 1-3: The INTENT_COMMAND_PATTERN regex has two issues: the leading
`\s` alternative allows false positives by matching plain text mentions like
"echo intent load" as valid commands, and the pattern is missing the `||`
operator from the command-chain alternatives, creating inconsistency with the
production parser. Remove or restrict the `\s` alternative to only match at
proper command boundaries (start of line or after actual command delimiters like
&&, ;, |) and add `||` to the list of command separators in the initial
alternation group to match the production parser in
packages/intent/src/hooks/policy.ts.
In `@packages/intent/src/commands/install/command.ts`:
- Around line 112-114: The verification checklist comment at lines 112-114 in
the install command file promises to validate conditions (checking
`id`/`run`/`for` presence, validating `id` format as `<package>#<skill>`, and
rejecting local file paths) that are not actually enforced by the
`verifyIntentSkillsBlockFile` function in guidance.ts. Update the checklist
comment to accurately reflect only what the verifier currently validates (field
presence/types and `id` parseability), or alternatively enhance the
`verifyIntentSkillsBlockFile` verification function to add the missing
validations for ensuring `run` references match the parsed `id` and rejecting
any path-like values in mapping fields.
In `@packages/intent/src/commands/install/guidance.ts`:
- Around line 4-7: The import statements at the top of the file are not in the
correct order according to the import/order lint rule. Move the value import
statement containing formatSkillUse and parseSkillUse from `../../skills/use.js`
to appear before the type-only import statement containing ScanResult and
SkillEntry from `../../shared/types.js`. Value imports should be ordered before
type-only imports to comply with the linting configuration.
In `@packages/intent/src/commands/load.ts`:
- Around line 1-9: The imports at the top of the load.ts file are not ordered
according to the configured import/order ESLint rule. Reorganize the imports by
grouping type imports separately from regular imports, placing all regular value
imports first (including the imports from '../shared/cli-error.js',
'./support.js', and '../core/index.js'), then group all type imports together at
the end (the type imports from './support.js' and '../core/index.js'). This
ensures compliance with the import ordering convention used in the project.
In `@packages/intent/src/commands/validate.ts`:
- Around line 8-11: The import statement for printWarnings from ./support.js is
positioned incorrectly according to the import/order lint rule. Move the import
statement for printWarnings from ./support.js to appear after the import
statement for findWorkspacePackages from ../setup/workspace-patterns.js, so that
relative imports to parent directories come before relative imports to the
current directory.
In `@packages/intent/src/core/intent-core.ts`:
- Around line 2-16: The import ordering at the top of intent-core.ts violates
the configured rule where parent directory imports (../...) should be placed
before current directory imports (./...). Currently, the imports from
../discovery/fs-cache.js, ../skills/resolver.js, and ../skills/use.js are
scattered throughout the block after ./excludes.js and other ./ imports.
Reorganize the import statements by moving all parent directory imports (those
starting with ../) to the top of the import block, followed by all current
directory imports (those starting with ./), while preserving the internal
ordering within each group.
In `@packages/intent/src/discovery/scanner.ts`:
- Around line 9-24: The imports in the scanner.ts file violate the import/order
linting rule. Reorganize the imports so that the type imports (import type
statements for IntentFsCache and ReadFs) are placed at the end of the import
block, after all regular imports. Additionally, ensure that the regular imports
are ordered correctly with relative imports from the current directory
(./index.js, ./fs-cache.js, ./package-manager.js) appearing before parent
directory imports (../setup/workspace-patterns.js and ../shared/utils.js). This
will comply with the configured linting rules for import ordering.
In `@packages/intent/src/hooks/install.ts`:
- Around line 380-384: The isIntentHook function uses overly broad substring
matching on the entire serialized object, which can incorrectly identify
non-Intent hooks if they happen to contain 'intent-' and '-gate.mjs' anywhere in
their fields. Narrow the detection logic to check only the specific hook
identifier field (such as the hook name property) rather than the entire
serialized object, ensuring that only actual Intent hooks matching the expected
naming pattern are identified and preventing false positives that could lead to
deletion of unrelated hook entries during reinstall.
In `@packages/intent/src/setup/project-setup.ts`:
- Line 14: The import statement for resolveProjectContext from
'../core/project-context.js' at line 14 violates the ESLint import/order rule,
which enforces a specific ordering for imports. Review all import statements in
the project-setup.ts file and reorder the resolveProjectContext import to comply
with the enforced import order rules, which typically require relative parent
directory imports (../) to be grouped together and ordered correctly relative to
other imports like external packages and internal modules.
- Around line 175-217: The code currently treats URL-valued intent.docs (like
https://...) as file paths and appends /** to create invalid glob patterns.
Before constructing the docsPath and WATCH_PATHS, add a check to detect if the
docs value is a URL by checking if it starts with http:// or https://. For URL
values, skip the glob path transformation (the .replace(/\/$/, '') + '/**'
pattern) and handle them appropriately, so that remote URLs are not converted
into invalid file-watch globs like https://.../**. This check should be applied
to both the docsPath assignment and the WATCH_PATHS string construction in the
single-package mode branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ebc56361-5c29-4645-9d59-69fd8c3af984
📒 Files selected for processing (96)
.changeset/hot-bottles-float.mddocs/cli/intent-hooks.mddocs/cli/intent-install.mddocs/config.jsondocs/getting-started/quick-start-consumers.mddocs/overview.mdevals/intent-discovery/README.mdevals/intent-discovery/condition-setup.eval.tsevals/intent-discovery/corpus/conditions.tsevals/intent-discovery/corpus/live-tasks.tsevals/intent-discovery/harness/intent-hooks/gate.mjsevals/intent-discovery/harness/intent-hooks/hook-core.d.mtsevals/intent-discovery/harness/intent-hooks/hook-core.mjsevals/intent-discovery/harness/intent-hooks/hook-io.mjsevals/intent-discovery/harness/prepare-copilot-home.tsevals/intent-discovery/harness/run-copilot-task.tsevals/intent-discovery/harness/setup-intent-condition.tsevals/intent-discovery/intent-hooks.eval.tsevals/intent-discovery/live-copilot-harness.eval.tsevals/intent-discovery/vitest.evals.config.tsknip.jsonpackages/intent/README.mdpackages/intent/src/cli.tspackages/intent/src/commands/exclude.tspackages/intent/src/commands/hooks/command.tspackages/intent/src/commands/install/command.tspackages/intent/src/commands/install/guidance.tspackages/intent/src/commands/list.tspackages/intent/src/commands/load.tspackages/intent/src/commands/meta.tspackages/intent/src/commands/setup/edit-package-json.tspackages/intent/src/commands/setup/github-actions.tspackages/intent/src/commands/stale.tspackages/intent/src/commands/support.tspackages/intent/src/commands/validate.tspackages/intent/src/core.tspackages/intent/src/core/index.tspackages/intent/src/core/intent-core.tspackages/intent/src/core/load-resolution.tspackages/intent/src/core/markdown.tspackages/intent/src/core/project-context.tspackages/intent/src/core/source-policy.tspackages/intent/src/core/types.tspackages/intent/src/discovery/fs-cache.tspackages/intent/src/discovery/package-manager.tspackages/intent/src/discovery/register.tspackages/intent/src/discovery/scanner.tspackages/intent/src/discovery/walk.tspackages/intent/src/hooks/adapters.tspackages/intent/src/hooks/agents/claude.tspackages/intent/src/hooks/agents/codex.tspackages/intent/src/hooks/agents/copilot.tspackages/intent/src/hooks/install.tspackages/intent/src/hooks/policy.tspackages/intent/src/hooks/types.tspackages/intent/src/index.tspackages/intent/src/setup.tspackages/intent/src/setup/index.tspackages/intent/src/setup/project-setup.tspackages/intent/src/setup/workspace-patterns.tspackages/intent/src/shared/cli-error.tspackages/intent/src/shared/cli-output.tspackages/intent/src/shared/command-runner.tspackages/intent/src/shared/display.tspackages/intent/src/shared/types.tspackages/intent/src/shared/utils.tspackages/intent/src/skills/categories.tspackages/intent/src/skills/paths.tspackages/intent/src/skills/resolver.tspackages/intent/src/skills/use.tspackages/intent/src/staleness/artifact-coverage.tspackages/intent/src/staleness/check.tspackages/intent/src/staleness/index.tspackages/intent/src/staleness/workflow-review.tspackages/intent/tests/artifact-coverage.test.tspackages/intent/tests/cli.test.tspackages/intent/tests/core.test.tspackages/intent/tests/fs-cache.test.tspackages/intent/tests/hooks-install.test.tspackages/intent/tests/hooks.test.tspackages/intent/tests/install-writer.test.tspackages/intent/tests/integration/source-policy-surfaces.test.tspackages/intent/tests/parse-frontmatter.test.tspackages/intent/tests/read-scalar-field.test.tspackages/intent/tests/resolver.test.tspackages/intent/tests/scanner.test.tspackages/intent/tests/setup.test.tspackages/intent/tests/skill-categories.test.tspackages/intent/tests/skill-paths.test.tspackages/intent/tests/skill-use.test.tspackages/intent/tests/skills.test.tspackages/intent/tests/source-policy.test.tspackages/intent/tests/stale-command.test.tspackages/intent/tests/staleness.test.tspackages/intent/tests/workflow-review.test.tspackages/intent/tests/workspace-patterns.test.ts
Comment on lines
+1
to
+3
| const INTENT_COMMAND_PATTERN = | ||
| /(?:^|\s|&&|;|\|)\s*((?:bunx\s+@tanstack\/intent(?:@latest)?)|(?:pnpm\s+exec\s+intent)|(?:pnpm\s+dlx\s+@tanstack\/intent(?:@latest)?)|(?:npx\s+@tanstack\/intent(?:@latest)?)|(?:yarn\s+dlx\s+@tanstack\/intent(?:@latest)?)|(?:intent))\s+(list|load)(?:\s+([^\s|;&]+))?/i | ||
|
|
Contributor
There was a problem hiding this comment.
Regex in Line 2 allows false positives and diverges from the production parser.
The \s alternative makes plain text mentions parse as commands (e.g. echo intent load ...), which can incorrectly flip hasLoaded to true. It also omits ||, so command-chain coverage is inconsistent with packages/intent/src/hooks/policy.ts Line 10.
Suggested fix
-const INTENT_COMMAND_PATTERN =
- /(?:^|\s|&&|;|\|)\s*((?:bunx\s+@tanstack\/intent(?:`@latest`)?)|(?:pnpm\s+exec\s+intent)|(?:pnpm\s+dlx\s+@tanstack\/intent(?:`@latest`)?)|(?:npx\s+@tanstack\/intent(?:`@latest`)?)|(?:yarn\s+dlx\s+@tanstack\/intent(?:`@latest`)?)|(?:intent))\s+(list|load)(?:\s+([^\s|;&]+))?/i
+const INTENT_COMMAND_PATTERN =
+ /(?:^|&&|\|\||;|\|)\s*((?:bunx\s+@tanstack\/intent(?:`@latest`)?)|(?:pnpm\s+exec\s+intent)|(?:pnpm\s+dlx\s+@tanstack\/intent(?:`@latest`)?)|(?:npx\s+@tanstack\/intent(?:`@latest`)?)|(?:yarn\s+dlx\s+@tanstack\/intent(?:`@latest`)?)|(?:intent))\s+(list|load)(?:\s+([^\s|;&]+))?/i📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const INTENT_COMMAND_PATTERN = | |
| /(?:^|\s|&&|;|\|)\s*((?:bunx\s+@tanstack\/intent(?:@latest)?)|(?:pnpm\s+exec\s+intent)|(?:pnpm\s+dlx\s+@tanstack\/intent(?:@latest)?)|(?:npx\s+@tanstack\/intent(?:@latest)?)|(?:yarn\s+dlx\s+@tanstack\/intent(?:@latest)?)|(?:intent))\s+(list|load)(?:\s+([^\s|;&]+))?/i | |
| const INTENT_COMMAND_PATTERN = | |
| /(?:^|&&|\|\||;|\|)\s*((?:bunx\s+@tanstack\/intent(?:`@latest`)?)|(?:pnpm\s+exec\s+intent)|(?:pnpm\s+dlx\s+@tanstack\/intent(?:`@latest`)?)|(?:npx\s+@tanstack\/intent(?:`@latest`)?)|(?:yarn\s+dlx\s+@tanstack\/intent(?:`@latest`)?)|(?:intent))\s+(list|load)(?:\s+([^\s|;&]+))?/i |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@evals/intent-discovery/harness/intent-hooks/hook-core.mjs` around lines 1 -
3, The INTENT_COMMAND_PATTERN regex has two issues: the leading `\s` alternative
allows false positives by matching plain text mentions like "echo intent load"
as valid commands, and the pattern is missing the `||` operator from the
command-chain alternatives, creating inconsistency with the production parser.
Remove or restrict the `\s` alternative to only match at proper command
boundaries (start of line or after actual command delimiters like &&, ;, |) and
add `||` to the list of command separators in the initial alternation group to
match the production parser in packages/intent/src/hooks/policy.ts.
Comment on lines
+112
to
+114
| - Confirm every mapping has \`id\`, \`run\`, and \`for\` | ||
| - Confirm every \`id\` parses as \`<package>#<skill>\` | ||
| - Confirm no mapping includes local file paths |
Contributor
There was a problem hiding this comment.
Verification checklist over-promises checks that are not actually enforced.
Lines 112-114 now require validating id/run/for and no local paths, but the verifier path used by runInstallCommand (packages/intent/src/commands/install/guidance.ts, verifyIntentSkillsBlockFile) currently only checks field presence/types and id parseability. It does not enforce that run actually loads that same id, nor reject path-like values in mapping fields. This can report successful install verification for semantically broken mappings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intent/src/commands/install/command.ts` around lines 112 - 114, The
verification checklist comment at lines 112-114 in the install command file
promises to validate conditions (checking `id`/`run`/`for` presence, validating
`id` format as `<package>#<skill>`, and rejecting local file paths) that are not
actually enforced by the `verifyIntentSkillsBlockFile` function in guidance.ts.
Update the checklist comment to accurately reflect only what the verifier
currently validates (field presence/types and `id` parseability), or
alternatively enhance the `verifyIntentSkillsBlockFile` verification function to
add the missing validations for ensuring `run` references match the parsed `id`
and rejecting any path-like values in mapping fields.
Comment on lines
+4
to
+7
| import { formatIntentCommand } from '../../shared/command-runner.js' | ||
| import { isGeneratedMappingSkill } from '../../skills/categories.js' | ||
| import type { ScanResult, SkillEntry } from '../../shared/types.js' | ||
| import { formatSkillUse, parseSkillUse } from '../../skills/use.js' |
Contributor
There was a problem hiding this comment.
Fix import ordering to satisfy lint.
The value import from ../../skills/use.js should be placed before the type-only import from ../../shared/types.js to comply with the configured import/order rule.
Suggested diff
import { formatIntentCommand } from '../../shared/command-runner.js'
import { isGeneratedMappingSkill } from '../../skills/categories.js'
-import type { ScanResult, SkillEntry } from '../../shared/types.js'
import { formatSkillUse, parseSkillUse } from '../../skills/use.js'
+import type { ScanResult, SkillEntry } from '../../shared/types.js'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { formatIntentCommand } from '../../shared/command-runner.js' | |
| import { isGeneratedMappingSkill } from '../../skills/categories.js' | |
| import type { ScanResult, SkillEntry } from '../../shared/types.js' | |
| import { formatSkillUse, parseSkillUse } from '../../skills/use.js' | |
| import { formatIntentCommand } from '../../shared/command-runner.js' | |
| import { isGeneratedMappingSkill } from '../../skills/categories.js' | |
| import { formatSkillUse, parseSkillUse } from '../../skills/use.js' | |
| import type { ScanResult, SkillEntry } from '../../shared/types.js' |
🧰 Tools
🪛 ESLint
[error] 7-7: ../../skills/use.js import should occur before type import of ../../shared/types.js
(import/order)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intent/src/commands/install/guidance.ts` around lines 4 - 7, The
import statements at the top of the file are not in the correct order according
to the import/order lint rule. Move the value import statement containing
formatSkillUse and parseSkillUse from `../../skills/use.js` to appear before the
type-only import statement containing ScanResult and SkillEntry from
`../../shared/types.js`. Value imports should be ordered before type-only
imports to comply with the linting configuration.
Source: Linters/SAST tools
Comment on lines
+1
to
+9
| import { fail } from '../shared/cli-error.js' | ||
| import { coreOptionsFromGlobalFlags, printDebugInfo } from './support.js' | ||
| import { | ||
| IntentCoreError, | ||
| loadIntentSkill, | ||
| resolveIntentSkill, | ||
| } from '../core.js' | ||
| import type { GlobalScanFlags } from '../cli-support.js' | ||
| import type { LoadedIntentSkill, ResolvedIntentSkill } from '../core.js' | ||
| } from '../core/index.js' | ||
| import type { GlobalScanFlags } from './support.js' | ||
| import type { LoadedIntentSkill, ResolvedIntentSkill } from '../core/index.js' |
Contributor
There was a problem hiding this comment.
Fix import ordering to satisfy import/order.
This block violates the configured import-order rule and can fail CI.
Suggested reorder
import { fail } from '../shared/cli-error.js'
-import { coreOptionsFromGlobalFlags, printDebugInfo } from './support.js'
import {
IntentCoreError,
loadIntentSkill,
resolveIntentSkill,
} from '../core/index.js'
+import { coreOptionsFromGlobalFlags, printDebugInfo } from './support.js'
import type { GlobalScanFlags } from './support.js'
import type { LoadedIntentSkill, ResolvedIntentSkill } from '../core/index.js'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { fail } from '../shared/cli-error.js' | |
| import { coreOptionsFromGlobalFlags, printDebugInfo } from './support.js' | |
| import { | |
| IntentCoreError, | |
| loadIntentSkill, | |
| resolveIntentSkill, | |
| } from '../core.js' | |
| import type { GlobalScanFlags } from '../cli-support.js' | |
| import type { LoadedIntentSkill, ResolvedIntentSkill } from '../core.js' | |
| } from '../core/index.js' | |
| import type { GlobalScanFlags } from './support.js' | |
| import type { LoadedIntentSkill, ResolvedIntentSkill } from '../core/index.js' | |
| import { fail } from '../shared/cli-error.js' | |
| import { | |
| IntentCoreError, | |
| loadIntentSkill, | |
| resolveIntentSkill, | |
| } from '../core/index.js' | |
| import { coreOptionsFromGlobalFlags, printDebugInfo } from './support.js' | |
| import type { GlobalScanFlags } from './support.js' | |
| import type { LoadedIntentSkill, ResolvedIntentSkill } from '../core/index.js' |
🧰 Tools
🪛 ESLint
[error] 3-7: ../core/index.js import should occur before import of ./support.js
(import/order)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intent/src/commands/load.ts` around lines 1 - 9, The imports at the
top of the load.ts file are not ordered according to the configured import/order
ESLint rule. Reorganize the imports by grouping type imports separately from
regular imports, placing all regular value imports first (including the imports
from '../shared/cli-error.js', './support.js', and '../core/index.js'), then
group all type imports together at the end (the type imports from './support.js'
and '../core/index.js'). This ensures compliance with the import ordering
convention used in the project.
Source: Linters/SAST tools
Comment on lines
+8
to
+11
| import { fail, isCliFailure } from '../shared/cli-error.js' | ||
| import { printWarnings } from './support.js' | ||
| import { resolveProjectContext } from '../core/project-context.js' | ||
| import { findWorkspacePackages } from '../workspace-patterns.js' | ||
| import { findWorkspacePackages } from '../setup/workspace-patterns.js' |
Contributor
There was a problem hiding this comment.
Reorder imports to satisfy import/order.
./support.js should be placed after ../setup/workspace-patterns.js per the lint rule.
Suggested reorder
import { fail, isCliFailure } from '../shared/cli-error.js'
-import { printWarnings } from './support.js'
import { resolveProjectContext } from '../core/project-context.js'
import { findWorkspacePackages } from '../setup/workspace-patterns.js'
+import { printWarnings } from './support.js'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { fail, isCliFailure } from '../shared/cli-error.js' | |
| import { printWarnings } from './support.js' | |
| import { resolveProjectContext } from '../core/project-context.js' | |
| import { findWorkspacePackages } from '../workspace-patterns.js' | |
| import { findWorkspacePackages } from '../setup/workspace-patterns.js' | |
| import { fail, isCliFailure } from '../shared/cli-error.js' | |
| import { resolveProjectContext } from '../core/project-context.js' | |
| import { findWorkspacePackages } from '../setup/workspace-patterns.js' | |
| import { printWarnings } from './support.js' |
🧰 Tools
🪛 ESLint
[error] 9-9: ./support.js import should occur after import of ../setup/workspace-patterns.js
(import/order)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intent/src/commands/validate.ts` around lines 8 - 11, The import
statement for printWarnings from ./support.js is positioned incorrectly
according to the import/order lint rule. Move the import statement for
printWarnings from ./support.js to appear after the import statement for
findWorkspacePackages from ../setup/workspace-patterns.js, so that relative
imports to parent directories come before relative imports to the current
directory.
Source: Linters/SAST tools
Comment on lines
+2
to
+16
| import { | ||
| compileExcludePatterns, | ||
| getEffectiveExcludePatterns, | ||
| } from './excludes.js' | ||
| import { createIntentFsCache } from '../discovery/fs-cache.js' | ||
| import { rewriteLoadedSkillMarkdownDestinations } from './markdown.js' | ||
| import { resolveSkillUseFastPath } from './load-resolution.js' | ||
| import { resolveProjectContext } from './project-context.js' | ||
| import { | ||
| checkLoadAllowed, | ||
| readSkillSourcesConfig, | ||
| scanForPolicedIntents, | ||
| } from './source-policy.js' | ||
| import { ResolveSkillUseError, resolveSkillUse } from '../skills/resolver.js' | ||
| import { formatSkillUse, parseSkillUse } from '../skills/use.js' |
Contributor
There was a problem hiding this comment.
Resolve import/order violations in the top import block.
Line 6, Line 15, and Line 16 currently violate the configured ordering rule (../... imports should come before ./... imports in this setup).
Suggested reorder
import { isAbsolute, relative, resolve } from 'node:path'
+import { createIntentFsCache } from '../discovery/fs-cache.js'
+import { ResolveSkillUseError, resolveSkillUse } from '../skills/resolver.js'
+import { formatSkillUse, parseSkillUse } from '../skills/use.js'
import {
compileExcludePatterns,
getEffectiveExcludePatterns,
} from './excludes.js'
-import { createIntentFsCache } from '../discovery/fs-cache.js'
import { rewriteLoadedSkillMarkdownDestinations } from './markdown.js'
import { resolveSkillUseFastPath } from './load-resolution.js'
import { resolveProjectContext } from './project-context.js'
@@
-import { ResolveSkillUseError, resolveSkillUse } from '../skills/resolver.js'
-import { formatSkillUse, parseSkillUse } from '../skills/use.js'🧰 Tools
🪛 ESLint
[error] 6-6: ../discovery/fs-cache.js import should occur before import of ./excludes.js
(import/order)
[error] 15-15: ../skills/resolver.js import should occur before import of ./excludes.js
(import/order)
[error] 16-16: ../skills/use.js import should occur before import of ./excludes.js
(import/order)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intent/src/core/intent-core.ts` around lines 2 - 16, The import
ordering at the top of intent-core.ts violates the configured rule where parent
directory imports (../...) should be placed before current directory imports
(./...). Currently, the imports from ../discovery/fs-cache.js,
../skills/resolver.js, and ../skills/use.js are scattered throughout the block
after ./excludes.js and other ./ imports. Reorganize the import statements by
moving all parent directory imports (those starting with ../) to the top of the
import block, followed by all current directory imports (those starting with
./), while preserving the internal ordering within each group.
Source: Linters/SAST tools
Comment on lines
+9
to
+24
| import { createDependencyWalker, createPackageRegistrar } from './index.js' | ||
| import { | ||
| detectGlobalNodeModules, | ||
| nodeReadFs, | ||
| parseFrontmatter, | ||
| readScalarField, | ||
| toPosixPath, | ||
| } from './utils.js' | ||
| } from '../shared/utils.js' | ||
| import { createIntentFsCache } from './fs-cache.js' | ||
| import { detectPackageManager } from './package-manager.js' | ||
| import { | ||
| findWorkspacePackages, | ||
| findWorkspaceRoot, | ||
| } from './workspace-patterns.js' | ||
| } from '../setup/workspace-patterns.js' | ||
| import type { IntentFsCache } from './fs-cache.js' | ||
| import type { ReadFs } from './utils.js' | ||
| import type { ReadFs } from '../shared/utils.js' |
Contributor
There was a problem hiding this comment.
Fix import/order violations in scanner import grouping.
Line 9, Line 17, and Line 18 should be ordered after the ../setup/workspace-patterns.js import per the configured lint rule.
Suggested reorder
-import { createDependencyWalker, createPackageRegistrar } from './index.js'
import {
detectGlobalNodeModules,
nodeReadFs,
parseFrontmatter,
readScalarField,
toPosixPath,
} from '../shared/utils.js'
-import { createIntentFsCache } from './fs-cache.js'
-import { detectPackageManager } from './package-manager.js'
import {
findWorkspacePackages,
findWorkspaceRoot,
} from '../setup/workspace-patterns.js'
+import { createDependencyWalker, createPackageRegistrar } from './index.js'
+import { createIntentFsCache } from './fs-cache.js'
+import { detectPackageManager } from './package-manager.js'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { createDependencyWalker, createPackageRegistrar } from './index.js' | |
| import { | |
| detectGlobalNodeModules, | |
| nodeReadFs, | |
| parseFrontmatter, | |
| readScalarField, | |
| toPosixPath, | |
| } from './utils.js' | |
| } from '../shared/utils.js' | |
| import { createIntentFsCache } from './fs-cache.js' | |
| import { detectPackageManager } from './package-manager.js' | |
| import { | |
| findWorkspacePackages, | |
| findWorkspaceRoot, | |
| } from './workspace-patterns.js' | |
| } from '../setup/workspace-patterns.js' | |
| import type { IntentFsCache } from './fs-cache.js' | |
| import type { ReadFs } from './utils.js' | |
| import type { ReadFs } from '../shared/utils.js' | |
| import { | |
| detectGlobalNodeModules, | |
| nodeReadFs, | |
| parseFrontmatter, | |
| readScalarField, | |
| toPosixPath, | |
| } from '../shared/utils.js' | |
| import { | |
| findWorkspacePackages, | |
| findWorkspaceRoot, | |
| } from '../setup/workspace-patterns.js' | |
| import { createDependencyWalker, createPackageRegistrar } from './index.js' | |
| import { createIntentFsCache } from './fs-cache.js' | |
| import { detectPackageManager } from './package-manager.js' | |
| import type { IntentFsCache } from './fs-cache.js' | |
| import type { ReadFs } from '../shared/utils.js' |
🧰 Tools
🪛 ESLint
[error] 9-9: ./index.js import should occur after import of ../setup/workspace-patterns.js
(import/order)
[error] 17-17: ./fs-cache.js import should occur after import of ../setup/workspace-patterns.js
(import/order)
[error] 18-18: ./package-manager.js import should occur after import of ../setup/workspace-patterns.js
(import/order)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intent/src/discovery/scanner.ts` around lines 9 - 24, The imports in
the scanner.ts file violate the import/order linting rule. Reorganize the
imports so that the type imports (import type statements for IntentFsCache and
ReadFs) are placed at the end of the import block, after all regular imports.
Additionally, ensure that the regular imports are ordered correctly with
relative imports from the current directory (./index.js, ./fs-cache.js,
./package-manager.js) appearing before parent directory imports
(../setup/workspace-patterns.js and ../shared/utils.js). This will comply with
the configured linting rules for import ordering.
Source: Linters/SAST tools
Comment on lines
+380
to
+384
| function isIntentHook(value: unknown): boolean { | ||
| if (!value || typeof value !== 'object') return false | ||
| const serialized = JSON.stringify(value) | ||
| return serialized.includes('intent-') && serialized.includes('-gate.mjs') | ||
| } |
Contributor
There was a problem hiding this comment.
Narrow Intent-hook detection to avoid deleting unrelated hook entries.
On Line 382, isIntentHook matches by serializing the whole object and checking broad substrings. During reinstall, this can remove non-Intent hooks if any field text happens to include intent-...-gate.mjs, which is a config-integrity regression.
Suggested fix
+const INTENT_GATE_SCRIPT_RE =
+ /(?:^|[\\/])(?:old-)?intent-(claude|codex|copilot)-gate\.mjs(?:$|[?#\s'"])/i
+
function isIntentHook(value: unknown): boolean {
if (!value || typeof value !== 'object') return false
- const serialized = JSON.stringify(value)
- return serialized.includes('intent-') && serialized.includes('-gate.mjs')
+ const entry = value as Record<string, unknown>
+ const command = typeof entry.command === 'string' ? entry.command : ''
+ const args = Array.isArray(entry.args)
+ ? entry.args.filter((arg): arg is string => typeof arg === 'string')
+ : []
+
+ if (INTENT_GATE_SCRIPT_RE.test(command)) return true
+ return args.some((arg) => INTENT_GATE_SCRIPT_RE.test(arg))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function isIntentHook(value: unknown): boolean { | |
| if (!value || typeof value !== 'object') return false | |
| const serialized = JSON.stringify(value) | |
| return serialized.includes('intent-') && serialized.includes('-gate.mjs') | |
| } | |
| const INTENT_GATE_SCRIPT_RE = | |
| /(?:^|[\\/])(?:old-)?intent-(claude|codex|copilot)-gate\.mjs(?:$|[?#\s'"])/i | |
| function isIntentHook(value: unknown): boolean { | |
| if (!value || typeof value !== 'object') return false | |
| const entry = value as Record<string, unknown> | |
| const command = typeof entry.command === 'string' ? entry.command : '' | |
| const args = Array.isArray(entry.args) | |
| ? entry.args.filter((arg): arg is string => typeof arg === 'string') | |
| : [] | |
| if (INTENT_GATE_SCRIPT_RE.test(command)) return true | |
| return args.some((arg) => INTENT_GATE_SCRIPT_RE.test(arg)) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intent/src/hooks/install.ts` around lines 380 - 384, The
isIntentHook function uses overly broad substring matching on the entire
serialized object, which can incorrectly identify non-Intent hooks if they
happen to contain 'intent-' and '-gate.mjs' anywhere in their fields. Narrow the
detection logic to check only the specific hook identifier field (such as the
hook name property) rather than the entire serialized object, ensuring that only
actual Intent hooks matching the expected naming pattern are identified and
preventing false positives that could lead to deletion of unrelated hook entries
during reinstall.
| findWorkspaceRoot, | ||
| readWorkspacePatterns, | ||
| } from './workspace-patterns.js' | ||
| import { resolveProjectContext } from '../core/project-context.js' |
Contributor
There was a problem hiding this comment.
Fix import order to satisfy ESLint.
Line 14 currently violates the enforced import/order rule and will fail lint gates.
Suggested diff
import {
findPackagesWithSkills,
findWorkspaceRoot,
readWorkspacePatterns,
} from './workspace-patterns.js'
import { resolveProjectContext } from '../core/project-context.js'
+import { resolveProjectContext } from '../core/project-context.js'
+import {
+ findPackagesWithSkills,
+ findWorkspaceRoot,
+ readWorkspacePatterns,
+} from './workspace-patterns.js'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { resolveProjectContext } from '../core/project-context.js' | |
| import { resolveProjectContext } from '../core/project-context.js' | |
| import { | |
| findPackagesWithSkills, | |
| findWorkspaceRoot, | |
| readWorkspacePatterns, | |
| } from './workspace-patterns.js' |
🧰 Tools
🪛 ESLint
[error] 14-14: ../core/project-context.js import should occur before import of ./workspace-patterns.js
(import/order)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intent/src/setup/project-setup.ts` at line 14, The import statement
for resolveProjectContext from '../core/project-context.js' at line 14 violates
the ESLint import/order rule, which enforces a specific ordering for imports.
Review all import statements in the project-setup.ts file and reorder the
resolveProjectContext import to comply with the enforced import order rules,
which typically require relative parent directory imports (../) to be grouped
together and ordered correctly relative to other imports like external packages
and internal modules.
Source: Linters/SAST tools
Comment on lines
+175
to
+217
| const docs = | ||
| typeof (pkgJson.intent as Record<string, unknown> | undefined)?.docs === | ||
| 'string' | ||
| ? ((pkgJson.intent as Record<string, unknown>).docs as string) | ||
| : 'docs/' | ||
| const isMonorepo = packageDirs !== undefined | ||
| const monorepoFallbackPkg = packageDirs?.[0] | ||
| ? readPackageJson(packageDirs[0]) | ||
| : null | ||
| const repo = detectRepo( | ||
| pkgJson, | ||
| detectRepo(monorepoFallbackPkg ?? {}, basename(root)), | ||
| ) | ||
|
|
||
| let packageName = rawName | ||
| if (isMonorepo && isGenericWorkspaceName(rawName, root)) { | ||
| packageName = deriveWorkspacePackageName(root, repo, packageDirs) | ||
| } | ||
|
|
||
| // Derive srcPath: monorepos use a wildcard; single packages use the short name or fall back to root src/ | ||
| const shortName = packageName.replace(/^@[^/]+\//, '') | ||
| let srcPath = isMonorepo | ||
| ? 'packages/*/src/**' | ||
| : `packages/${shortName}/src/**` | ||
| if (!isMonorepo && existsSync(join(root, 'src'))) { | ||
| srcPath = 'src/**' | ||
| } | ||
|
|
||
| const docsPath = isMonorepo ? 'packages/*/docs/**' : docs | ||
|
|
||
| return { | ||
| PACKAGE_NAME: packageName, | ||
| PACKAGE_LABEL: packageName, | ||
| PAYLOAD_PACKAGE: packageName, | ||
| REPO: repo, | ||
| DOCS_PATH: docsPath.endsWith('**') | ||
| ? docsPath | ||
| : docsPath.replace(/\/$/, '') + '/**', | ||
| SRC_PATH: srcPath, | ||
| WATCH_PATHS: isMonorepo | ||
| ? buildWatchPaths(root, packageDirs) | ||
| : ` - '${docs.endsWith('**') ? docs : docs.replace(/\/$/, '') + '/**'}'\n - '${srcPath}'`, | ||
| } |
Contributor
There was a problem hiding this comment.
Handle remote intent.docs URLs before building path globs.
In single-package mode, a URL-valued intent.docs becomes https://.../** in DOCS_PATH/WATCH_PATHS, which is not a valid file-watch glob.
Suggested diff
- const docs =
+ const docs =
typeof (pkgJson.intent as Record<string, unknown> | undefined)?.docs ===
'string'
? ((pkgJson.intent as Record<string, unknown>).docs as string)
: 'docs/'
+ const docsIsRemote = /^https?:\/\//.test(docs)
@@
- const docsPath = isMonorepo ? 'packages/*/docs/**' : docs
+ const docsPath = isMonorepo
+ ? 'packages/*/docs/**'
+ : docsIsRemote
+ ? 'docs/**'
+ : docs
@@
- WATCH_PATHS: isMonorepo
- ? buildWatchPaths(root, packageDirs)
- : ` - '${docs.endsWith('**') ? docs : docs.replace(/\/$/, '') + '/**'}'\n - '${srcPath}'`,
+ WATCH_PATHS: isMonorepo
+ ? buildWatchPaths(root, packageDirs)
+ : docsIsRemote
+ ? ` - '${srcPath}'`
+ : ` - '${docs.endsWith('**') ? docs : docs.replace(/\/$/, '') + '/**'}'\n - '${srcPath}'`,
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const docs = | |
| typeof (pkgJson.intent as Record<string, unknown> | undefined)?.docs === | |
| 'string' | |
| ? ((pkgJson.intent as Record<string, unknown>).docs as string) | |
| : 'docs/' | |
| const isMonorepo = packageDirs !== undefined | |
| const monorepoFallbackPkg = packageDirs?.[0] | |
| ? readPackageJson(packageDirs[0]) | |
| : null | |
| const repo = detectRepo( | |
| pkgJson, | |
| detectRepo(monorepoFallbackPkg ?? {}, basename(root)), | |
| ) | |
| let packageName = rawName | |
| if (isMonorepo && isGenericWorkspaceName(rawName, root)) { | |
| packageName = deriveWorkspacePackageName(root, repo, packageDirs) | |
| } | |
| // Derive srcPath: monorepos use a wildcard; single packages use the short name or fall back to root src/ | |
| const shortName = packageName.replace(/^@[^/]+\//, '') | |
| let srcPath = isMonorepo | |
| ? 'packages/*/src/**' | |
| : `packages/${shortName}/src/**` | |
| if (!isMonorepo && existsSync(join(root, 'src'))) { | |
| srcPath = 'src/**' | |
| } | |
| const docsPath = isMonorepo ? 'packages/*/docs/**' : docs | |
| return { | |
| PACKAGE_NAME: packageName, | |
| PACKAGE_LABEL: packageName, | |
| PAYLOAD_PACKAGE: packageName, | |
| REPO: repo, | |
| DOCS_PATH: docsPath.endsWith('**') | |
| ? docsPath | |
| : docsPath.replace(/\/$/, '') + '/**', | |
| SRC_PATH: srcPath, | |
| WATCH_PATHS: isMonorepo | |
| ? buildWatchPaths(root, packageDirs) | |
| : ` - '${docs.endsWith('**') ? docs : docs.replace(/\/$/, '') + '/**'}'\n - '${srcPath}'`, | |
| } | |
| const docs = | |
| typeof (pkgJson.intent as Record<string, unknown> | undefined)?.docs === | |
| 'string' | |
| ? ((pkgJson.intent as Record<string, unknown>).docs as string) | |
| : 'docs/' | |
| const docsIsRemote = /^https?:\/\//.test(docs) | |
| const isMonorepo = packageDirs !== undefined | |
| const monorepoFallbackPkg = packageDirs?.[0] | |
| ? readPackageJson(packageDirs[0]) | |
| : null | |
| const repo = detectRepo( | |
| pkgJson, | |
| detectRepo(monorepoFallbackPkg ?? {}, basename(root)), | |
| ) | |
| let packageName = rawName | |
| if (isMonorepo && isGenericWorkspaceName(rawName, root)) { | |
| packageName = deriveWorkspacePackageName(root, repo, packageDirs) | |
| } | |
| // Derive srcPath: monorepos use a wildcard; single packages use the short name or fall back to root src/ | |
| const shortName = packageName.replace(/^@[^/]+\//, '') | |
| let srcPath = isMonorepo | |
| ? 'packages/*/src/**' | |
| : `packages/${shortName}/src/**` | |
| if (!isMonorepo && existsSync(join(root, 'src'))) { | |
| srcPath = 'src/**' | |
| } | |
| const docsPath = isMonorepo | |
| ? 'packages/*/docs/**' | |
| : docsIsRemote | |
| ? 'docs/**' | |
| : docs | |
| return { | |
| PACKAGE_NAME: packageName, | |
| PACKAGE_LABEL: packageName, | |
| PAYLOAD_PACKAGE: packageName, | |
| REPO: repo, | |
| DOCS_PATH: docsPath.endsWith('**') | |
| ? docsPath | |
| : docsPath.replace(/\/$/, '') + '/**', | |
| SRC_PATH: srcPath, | |
| WATCH_PATHS: isMonorepo | |
| ? buildWatchPaths(root, packageDirs) | |
| : docsIsRemote | |
| ? ` - '${srcPath}'` | |
| : ` - '${docs.endsWith('**') ? docs : docs.replace(/\/$/, '') + '/**'}'\n - '${srcPath}'`, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/intent/src/setup/project-setup.ts` around lines 175 - 217, The code
currently treats URL-valued intent.docs (like https://...) as file paths and
appends /** to create invalid glob patterns. Before constructing the docsPath
and WATCH_PATHS, add a check to detect if the docs value is a URL by checking if
it starts with http:// or https://. For URL values, skip the glob path
transformation (the .replace(/\/$/, '') + '/**' pattern) and handle them
appropriately, so that remote URLs are not converted into invalid file-watch
globs like https://.../**. This check should be applied to both the docsPath
assignment and the WATCH_PATHS string construction in the single-package mode
branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
intent hooks installsupport for lifecycle-hook enforcement across supported agents, with project/user scope handling, generated hook runner scripts, and agent-specific policy wiring.Includes docs and eval coverage for hooked intent discovery, plus validation updates so the hook flow is covered by the existing PR and integration gates.
Summary by CodeRabbit
New Features
intent hooks installcommand for configuring AI coding agents (Claude, Codex, Copilot) with lifecycle-hook enforcement.skillstotanstackIntentschema withid,run, andforfields.Documentation
hooks installcommand with per-agent scope guidance.