feat: programmatic architectural enforcement via ESLint boundaries#623
feat: programmatic architectural enforcement via ESLint boundaries#623yanekyuk wants to merge 3 commits intoindexnetwork:devfrom
Conversation
📝 WalkthroughWalkthroughIntroduces ESLint linting infrastructure with a new GitHub Actions workflow that validates boundary dependencies, adopts a flat config structure with the boundaries plugin to enforce architectural layering, integrates lint-staged for pre-commit validation, and adds adapter naming convention enforcement via a new shell script. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
yanekyuk
force-pushed
the
feat/architectural-enforcement
branch
from
8c36278 to
0254d71
Compare
…ement Upgrade ESLint to v9 flat config and add eslint-plugin-boundaries to enforce strict layering rules: - Controllers can only import services, guards, types, schemas - Services cannot import other services or controllers - Adapters cannot import protocol, services, or controllers - Protocol layer can only import itself, types, schemas - Test files are excluded from boundary checks Pre-existing violations suppressed with eslint-disable + TODO comments.
…ng check - lint-staged runs ESLint on staged files before commit - Pre-commit hook blocks commits with lint violations - GitHub Actions workflow checks boundaries on PRs to dev - scripts/check-adapter-names.sh flags tech-named adapters
yanekyuk
force-pushed
the
feat/architectural-enforcement
branch
from
0254d71 to
566c3d2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
protocol/src/controllers/storage.controller.ts (1)
8-10: Direct adapter usage requires follow-up refactoring.Unlike the other controllers that import only types or error classes from protocol, this controller directly instantiates and uses
S3StorageAdapterthroughout (lines 100, 136, 212, etc.). Theeslint-disableand TODO appropriately track this, but this is a higher-priority violation to address since it involves actual adapter method calls, not just type imports. Consider creating aStorageServiceto wrap the adapter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@protocol/src/controllers/storage.controller.ts` around lines 8 - 10, The controller is violating layering by importing and calling S3StorageAdapter directly (S3StorageAdapter usage at spots like lines around 100/136/212); refactor by introducing a StorageService abstraction (e.g., IStorageService interface and a concrete S3StorageService) that wraps all adapter calls, replace direct new/usage of S3StorageAdapter inside the StorageController with an injected StorageService, update methods in StorageController to call the StorageService (preserving method names used now), and remove the eslint-disable and direct adapter import so the controller only depends on the service interface/type.scripts/check-adapter-names.sh (1)
12-19: Handle case when no adapter files exist or directory is missing.With
set -euo pipefail, the glob"$ADAPTER_DIR"/*.tswill expand literally (or error) if no.tsfiles exist, causing the loop to process a non-existent file. Add a guard to handle this edge case gracefully.🛡️ Proposed defensive check
TECH_NAMES="drizzle|redis|bullmq|s3|resend|composio|postgres|pgvector|ioredis" +if [ ! -d "$ADAPTER_DIR" ]; then + echo "Adapter directory not found: $ADAPTER_DIR" + exit 1 +fi + +shopt -s nullglob for file in "$ADAPTER_DIR"/*.ts; do + [ -e "$file" ] || continue basename=$(basename "$file")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-adapter-names.sh` around lines 12 - 19, Add a guard before the for-loop that checks the ADAPTER_DIR exists and contains at least one .ts file so the glob won't be left unexpanded under set -euo pipefail; e.g., test [ -d "$ADAPTER_DIR" ] && ls "$ADAPTER_DIR"/*.ts >/dev/null 2>&1 before executing the loop over "$ADAPTER_DIR"/*.ts, and skip the loop (no change to VIOLATIONS) if the test fails; keep existing symbols ADAPTER_DIR, TECH_NAMES, basename, and VIOLATIONS unchanged so the rest of the script logic (the grep check and error counting) runs only when files are present.protocol/src/services/tool.service.ts (1)
33-37: Move tool collaborator wiring out ofToolService.This file now opts out of the service boundary for both
contactServiceandIntegrationService. SinceToolServiceis already assemblingToolDeps, it would be cleaner to receive those collaborators from a bootstrap/helper instead of importing sibling services directly, otherwise every new tool dependency will need anotherboundaries/dependenciessuppression.As per coding guidelines, "Services must not import other services. Use events, queues, or shared lib/graphs instead for inter-service communication."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@protocol/src/services/tool.service.ts` around lines 33 - 37, ToolService currently violates layering by importing contactService and IntegrationService directly; remove those imports and instead accept the collaborators via the existing ToolDeps assembly so ToolService doesn't reference sibling services. Update the ToolDeps type (and the ToolService constructor/factory) to include fields for contactService and integrationService, remove the boundaries/dependencies eslint disables in this file, and wire the actual contactService and IntegrationService instances in the bootstrap/helper that builds ToolDeps (e.g., the module that constructs ToolDeps), ensuring all callers instantiate ToolService with the new dependencies rather than importing the services inside ToolService.protocol/src/controllers/mcp.handler.ts (1)
12-52: Treat MCP runtime creation as bootstrap, not controller logic.This import block pulls the composition root, graph factories, agents, and MCP server bootstrap straight into the HTTP handler, so the controller is doing more than transport/auth and the boundaries rule has to be suppressed almost line-by-line. Moving the graph/server assembly next to
protocol-init.tswould keepmcpHandler()focused and avoid drift with the similar wiring inprotocol/src/services/tool.service.ts.As per coding guidelines, "Controllers: HTTP only with guards (e.g., auth) and Zod validation; delegate to services or graphs."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@protocol/src/controllers/mcp.handler.ts` around lines 12 - 52, Summary: mcpHandler currently performs bootstrap/composition instead of being an HTTP-only controller. Move the runtime/graph/server assembly out of the controller by creating (or reusing) a bootstrap factory in protocol-init.ts that composes createDefaultProtocolDeps, createMcpServer, the graph factories (IntentGraphFactory, ProfileGraphFactory, OpportunityGraphFactory, HydeGraphFactory, IndexGraphFactory, IndexMembershipGraphFactory, IntentIndexGraphFactory, NegotiationGraphFactory), and agents (HydeGenerator, LensInferrer, NegotiationProposer, NegotiationResponder) into a single exported function (e.g., createProtocolRuntime or createMcpRuntime) that returns the scoped deps/HydeGraphDatabase/McpAuthResolver/ToolDeps needed; then update mcpHandler/mcp.handler.ts to remove those imports and instead call the new bootstrap function (or accept injected runtime) so mcpHandler only handles auth/validation and delegates work to the composed service, and remove the per-line boundaries disable comments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/lint.yml:
- Around line 34-52: The current filter only captures messages where m.ruleId
=== "boundaries/dependencies" and misses parser/fatal errors (m.ruleId ===
null); update the processing of results in the "Check architectural boundaries"
step to also treat messages with m.fatal === true as violations (e.g., include
m.fatal in the filter alongside the existing ruleId check), ensure those fatal
messages are collected into the violations array and printed with filePath, line
and message, and keep the existing process.exit(1) when any violations
(including fatal/parser errors) are present.
In `@protocol/eslint.config.mjs`:
- Around line 69-84: Remove "init" from the services allowlist in the ESLint
rule and refactor chat.service.ts to eliminate its lazy require of
protocol-init/createDefaultProtocolDeps: change chat.service.ts to accept the
protocol dependencies (the object returned by createDefaultProtocolDeps) via
constructor or factory injection (e.g., add a ProtocolDeps parameter to the
ChatService constructor or to its factory function), update all call sites to
pass those deps from the composition root (where createDefaultProtocolDeps is
used), and remove any require/import of "protocol-init" from the service so the
protocol layer remains self-contained.
In `@protocol/package.json`:
- Around line 70-82: Update the `@eslint/js` dependency in package.json so it
matches the ESLint 9 major version: replace the current "@eslint/js": "^10.0.1"
entry with a 9.x range (e.g. "@eslint/js": "^9.0.0") to align with the existing
"eslint": "^9.0.0" entry and avoid the version mismatch.
In `@protocol/src/lib/protocol/graphs/opportunity.graph.ts`:
- Around line 2659-2663: Remove the layering-violating dynamic import fallback
and make the notifier explicit: delete the import(...) fallback in
opportunity.graph.ts so the code only uses the injected this.queueNotification
(typed as QueueOpportunityNotificationFn | undefined), update the
OpportunityGraph/constructor signature (or factory) to accept a
QueueOpportunityNotificationFn via constructor injection for consumers, and then
update all call sites (protocol/src/lib/protocol/tools/index.ts,
protocol/src/services/tool.service.ts,
protocol/src/services/opportunity.service.ts and any other creators) to pass the
concrete queueOpportunityNotification implementation (from
src/queues/notification.queue) into the graph's constructor; ensure interfaces
in src/lib/protocol/interfaces/ expose the notifier type if not already so the
protocol layer has no parent-directory imports.
---
Nitpick comments:
In `@protocol/src/controllers/mcp.handler.ts`:
- Around line 12-52: Summary: mcpHandler currently performs
bootstrap/composition instead of being an HTTP-only controller. Move the
runtime/graph/server assembly out of the controller by creating (or reusing) a
bootstrap factory in protocol-init.ts that composes createDefaultProtocolDeps,
createMcpServer, the graph factories (IntentGraphFactory, ProfileGraphFactory,
OpportunityGraphFactory, HydeGraphFactory, IndexGraphFactory,
IndexMembershipGraphFactory, IntentIndexGraphFactory, NegotiationGraphFactory),
and agents (HydeGenerator, LensInferrer, NegotiationProposer,
NegotiationResponder) into a single exported function (e.g.,
createProtocolRuntime or createMcpRuntime) that returns the scoped
deps/HydeGraphDatabase/McpAuthResolver/ToolDeps needed; then update
mcpHandler/mcp.handler.ts to remove those imports and instead call the new
bootstrap function (or accept injected runtime) so mcpHandler only handles
auth/validation and delegates work to the composed service, and remove the
per-line boundaries disable comments.
In `@protocol/src/controllers/storage.controller.ts`:
- Around line 8-10: The controller is violating layering by importing and
calling S3StorageAdapter directly (S3StorageAdapter usage at spots like lines
around 100/136/212); refactor by introducing a StorageService abstraction (e.g.,
IStorageService interface and a concrete S3StorageService) that wraps all
adapter calls, replace direct new/usage of S3StorageAdapter inside the
StorageController with an injected StorageService, update methods in
StorageController to call the StorageService (preserving method names used now),
and remove the eslint-disable and direct adapter import so the controller only
depends on the service interface/type.
In `@protocol/src/services/tool.service.ts`:
- Around line 33-37: ToolService currently violates layering by importing
contactService and IntegrationService directly; remove those imports and instead
accept the collaborators via the existing ToolDeps assembly so ToolService
doesn't reference sibling services. Update the ToolDeps type (and the
ToolService constructor/factory) to include fields for contactService and
integrationService, remove the boundaries/dependencies eslint disables in this
file, and wire the actual contactService and IntegrationService instances in the
bootstrap/helper that builds ToolDeps (e.g., the module that constructs
ToolDeps), ensuring all callers instantiate ToolService with the new
dependencies rather than importing the services inside ToolService.
In `@scripts/check-adapter-names.sh`:
- Around line 12-19: Add a guard before the for-loop that checks the ADAPTER_DIR
exists and contains at least one .ts file so the glob won't be left unexpanded
under set -euo pipefail; e.g., test [ -d "$ADAPTER_DIR" ] && ls
"$ADAPTER_DIR"/*.ts >/dev/null 2>&1 before executing the loop over
"$ADAPTER_DIR"/*.ts, and skip the loop (no change to VIOLATIONS) if the test
fails; keep existing symbols ADAPTER_DIR, TECH_NAMES, basename, and VIOLATIONS
unchanged so the rest of the script logic (the grep check and error counting)
runs only when files are present.
🪄 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: c978cd8f-07d7-4010-aff1-ba0d4b75da00
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockprotocol/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.github/workflows/lint.ymlpackage.jsonprotocol/eslint.config.mjsprotocol/package.jsonprotocol/src/controllers/chat.controller.tsprotocol/src/controllers/debug.controller.tsprotocol/src/controllers/integration.controller.tsprotocol/src/controllers/mcp.handler.tsprotocol/src/controllers/storage.controller.tsprotocol/src/controllers/tool.controller.tsprotocol/src/controllers/user.controller.tsprotocol/src/lib/protocol/graphs/opportunity.graph.tsprotocol/src/services/integration.service.tsprotocol/src/services/tool.service.tsscripts/check-adapter-names.shscripts/hooks/pre-commit
| - name: Check architectural boundaries | ||
| run: | | ||
| cd protocol | ||
| bunx eslint src/ --format json 2>/dev/null | bun -e ' | ||
| const results = JSON.parse(await Bun.stdin.text()); | ||
| const violations = results | ||
| .flatMap(r => r.messages.filter(m => m.ruleId === "boundaries/dependencies")) | ||
| .map((m, i, _, r = results.find(r2 => r2.messages.includes(m))) => | ||
| `${r?.filePath}:${m.line} - ${m.message}` | ||
| ); | ||
| if (violations.length > 0) { | ||
| console.log("❌ Architectural boundary violations found:\n"); | ||
| violations.forEach(v => console.log(` ${v}`)); | ||
| console.log(`\n${violations.length} violation(s) total.`); | ||
| process.exit(1); | ||
| } else { | ||
| console.log("✅ No architectural boundary violations."); | ||
| } | ||
| ' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat <<'JSON' | bun -e '
const results = JSON.parse(await Bun.stdin.text());
const violations = results
.flatMap(r => r.messages.filter(m => m.ruleId === "boundaries/dependencies"))
.map((m, i, _, r = results.find(r2 => r2.messages.includes(m))) =>
`${r?.filePath}:${m.line} - ${m.message}`
);
if (violations.length > 0) process.exit(1);
console.log("workflow snippet exits 0");
'
[
{
"filePath": "src/broken.ts",
"messages": [
{ "ruleId": null, "fatal": true, "line": 1, "message": "Parsing error: Unexpected token" }
]
}
]
JSONRepository: indexnetwork/index
Length of output: 105
🏁 Script executed:
cat -n .github/workflows/lint.yml | sed -n '30,55p'Repository: indexnetwork/index
Length of output: 1308
🏁 Script executed:
# Search for any tests or documentation about the workflow behavior
fd -t f '*.ts' -o '*.js' -o '*.md' | xargs rg -l 'eslint|lint.yml' | head -20Repository: indexnetwork/index
Length of output: 1058
🏁 Script executed:
# Look for ESLint configuration in the protocol directory
fd -t f '.eslintrc*' 'eslint.config*' -o '*.json' | rg -B 1 -A 1 'eslint|boundaries'Repository: indexnetwork/index
Length of output: 193
🏁 Script executed:
# Check if there's eslint configuration in protocol directory
ls -la protocol/ | head -20Repository: indexnetwork/index
Length of output: 1248
🏁 Script executed:
# Look for ESLint rules configuration, especially boundaries rule
fd -t f . protocol | rg -l 'eslint.*config|boundaries' | head -10Repository: indexnetwork/index
Length of output: 69
🏁 Script executed:
cat -n protocol/eslint.config.mjs | head -100Repository: indexnetwork/index
Length of output: 4165
🏁 Script executed:
# Search for boundaries rule configuration
rg -A 5 -B 5 'boundaries' protocol/eslint.config.mjsRepository: indexnetwork/index
Length of output: 1652
🏁 Script executed:
# Look for any test files that might validate the workflow
fd -t f '.spec.ts' 'test.ts' | xargs rg -l 'fatal|parser.*error' 2>/dev/null | head -10Repository: indexnetwork/index
Length of output: 610
🏁 Script executed:
# Look for any other workflow files or scripts that might handle eslint output
fd -t f 'workflow' '.yml' '.yaml' | xargs rg -l 'eslint' 2>/dev/nullRepository: indexnetwork/index
Length of output: 1258
🏁 Script executed:
# Search for how eslint fatal errors are typically handled
rg -i 'fatal|parser.*error' protocol/ | head -20Repository: indexnetwork/index
Length of output: 2439
🏁 Script executed:
# Check if there's any documentation about the architecture check
cat docs/superpowers/specs/2026-04-02-architectural-enforcement-design.md | head -150Repository: indexnetwork/index
Length of output: 4968
Fail this job on ESLint fatal/parser errors.
The step filters ESLint JSON output to only catch ruleId === "boundaries/dependencies" violations. Parser/fatal errors have ruleId: null and are not caught by this filter, allowing malformed TypeScript to pass the check.
Suggested guard
const results = JSON.parse(await Bun.stdin.text());
+ const fatals = results
+ .flatMap(r => r.messages
+ .filter(m => m.fatal)
+ .map(m => `${r.filePath}:${m.line} - ${m.message}`));
const violations = results
.flatMap(r => r.messages.filter(m => m.ruleId === "boundaries/dependencies"))
.map((m, i, _, r = results.find(r2 => r2.messages.includes(m))) =>
`${r?.filePath}:${m.line} - ${m.message}`
);
+ if (fatals.length > 0) {
+ console.log("❌ ESLint fatal errors found:\n");
+ fatals.forEach(v => console.log(` ${v}`));
+ process.exit(1);
+ }
if (violations.length > 0) {🧰 Tools
🪛 actionlint (1.7.11)
[error] 35-35: shellcheck reported issue in this script: SC2016:info:2:53: Expressions don't expand in single quotes, use double quotes for that
(shellcheck)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/lint.yml around lines 34 - 52, The current filter only
captures messages where m.ruleId === "boundaries/dependencies" and misses
parser/fatal errors (m.ruleId === null); update the processing of results in the
"Check architectural boundaries" step to also treat messages with m.fatal ===
true as violations (e.g., include m.fatal in the filter alongside the existing
ruleId check), ensure those fatal messages are collected into the violations
array and printed with filePath, line and message, and keep the existing
process.exit(1) when any violations (including fatal/parser errors) are present.
| // Services → adapters, protocol, init, events, queues, schemas, types | ||
| // Service-to-service NOT allowed — use events/queues | ||
| { | ||
| from: { type: "services" }, | ||
| allow: { | ||
| to: { | ||
| type: [ | ||
| "adapters", | ||
| "protocol", | ||
| "init", | ||
| "events", | ||
| "queues", | ||
| "schemas", | ||
| "types", | ||
| ], | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 --type ts 'createDefaultProtocolDeps|protocol-init' protocol/src/servicesRepository: indexnetwork/index
Length of output: 748
Remove "init" from the services allowlist and refactor chat.service.ts to break the circular dependency.
The ESLint rule currently permits services → protocol-init, which violates the architecture principle that the protocol layer be fully self-contained. This isn't hypothetical: chat.service.ts (lines 55–57) already imports createDefaultProtocolDeps from protocol-init via lazy require to work around a circular dependency. Remove "init" from the allowlist, then refactor chat.service.ts to receive protocol dependencies via constructor injection rather than pulling the composition root directly into the service layer.
ESLint config change
type: [
"adapters",
"protocol",
- "init",
"events",
"queues",
"schemas",
"types",📝 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.
| // Services → adapters, protocol, init, events, queues, schemas, types | |
| // Service-to-service NOT allowed — use events/queues | |
| { | |
| from: { type: "services" }, | |
| allow: { | |
| to: { | |
| type: [ | |
| "adapters", | |
| "protocol", | |
| "init", | |
| "events", | |
| "queues", | |
| "schemas", | |
| "types", | |
| ], | |
| }, | |
| // Services → adapters, protocol, init, events, queues, schemas, types | |
| // Service-to-service NOT allowed — use events/queues | |
| { | |
| from: { type: "services" }, | |
| allow: { | |
| to: { | |
| type: [ | |
| "adapters", | |
| "protocol", | |
| "events", | |
| "queues", | |
| "schemas", | |
| "types", | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@protocol/eslint.config.mjs` around lines 69 - 84, Remove "init" from the
services allowlist in the ESLint rule and refactor chat.service.ts to eliminate
its lazy require of protocol-init/createDefaultProtocolDeps: change
chat.service.ts to accept the protocol dependencies (the object returned by
createDefaultProtocolDeps) via constructor or factory injection (e.g., add a
ProtocolDeps parameter to the ChatService constructor or to its factory
function), update all call sites to pass those deps from the composition root
(where createDefaultProtocolDeps is used), and remove any require/import of
"protocol-init" from the service so the protocol layer remains self-contained.
| "@eslint/js": "^10.0.1", | ||
| "@langchain/langgraph-cli": "^0.0.38", | ||
| "@types/bun": "^1.3.5", | ||
| "@types/busboy": "^1.5.4", | ||
| "@types/node": "^20.10.5", | ||
| "@types/node-cron": "^3.0.11", | ||
| "@types/uuid": "^10.0.0", | ||
| "@typescript-eslint/eslint-plugin": "^6.15.0", | ||
| "@typescript-eslint/parser": "^6.15.0", | ||
| "drizzle-kit": "^0.31.9", | ||
| "eslint": "^8.56.0", | ||
| "eslint": "^9.0.0", | ||
| "eslint-import-resolver-typescript": "^4.4.4", | ||
| "eslint-plugin-boundaries": "^6.0.2", | ||
| "typescript": "^5.3.3", | ||
| "typescript-eslint": "^8.58.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What version of @eslint/js is compatible with eslint 9?
💡 Result:
Any version of @eslint/js in the 9.x series (such as ^9.0.0 or the latest 9.x release) is compatible with ESLint 9. @eslint/js versions are kept in lockstep with ESLint releases, as evidenced by ESLint v9 release notes consistently upgrading @eslint/js to matching 9.x versions (e.g., v9.0.0 with @eslint/js@9.0.0, v9.39.3 with @eslint/js@9.39.3). The official migration guide for ESLint v9 instructs installing @eslint/js without specifying a particular minor/patch version, implying semver ^9.0.0 compatibility. @eslint/js 10.x is for ESLint 10.
Citations:
- 1: https://eslint.org/docs/latest/use/migrate-to-9.0.0
- 2: https://github.com/eslint/eslint/releases
- 3: https://github.com/eslint/eslint/releases/tag/v9.20.0
- 4: https://eslint.org/blog/2026/02/eslint-v9.39.3-released/
- 5: https://www.npmjs.com/package/@eslint/js
- 6: eslint/eslint@1f66734
- 7: https://raw.githubusercontent.com/eslint/eslint/main/CHANGELOG.md
Fix @eslint/js version to match ESLint 9 major version.
@eslint/js@^10.0.1 is incompatible with eslint@^9.0.0. The @eslint/js package is released in lockstep with ESLint major versions: @eslint/js@9.x for ESLint 9.x and @eslint/js@10.x for ESLint 10.x. Change to @eslint/js@^9.0.0 to resolve the mismatch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@protocol/package.json` around lines 70 - 82, Update the `@eslint/js` dependency
in package.json so it matches the ESLint 9 major version: replace the current
"@eslint/js": "^10.0.1" entry with a 9.x range (e.g. "@eslint/js": "^9.0.0") to
align with the existing "eslint": "^9.0.0" entry and avoid the version mismatch.
| // TODO: fix layering violation — protocol layer should not import queues; pass notifier via constructor injection | ||
| const notifier: QueueOpportunityNotificationFn | undefined = | ||
| this.queueNotification ?? | ||
| // eslint-disable-next-line boundaries/dependencies | ||
| (await import('../../../queues/notification.queue').then((m) => m.queueOpportunityNotification)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make queueNotification explicit instead of falling back to a parent-layer import.
The provided call sites in protocol/src/lib/protocol/tools/index.ts, protocol/src/services/tool.service.ts, and protocol/src/services/opportunity.service.ts still omit queueNotification, so this suppressed dynamic import remains the normal production path. That means src/lib/protocol is still reaching into src/queues; the architecture is being bypassed, not enforced.
Direction of change
- private queueNotification?: QueueOpportunityNotificationFn,
+ private queueNotification: QueueOpportunityNotificationFn,
...
- const notifier: QueueOpportunityNotificationFn | undefined =
- this.queueNotification ??
- (await import('../../../queues/notification.queue').then((m) => m.queueOpportunityNotification));
- if (!notifier) throw new Error('Opportunity notification not configured');
+ const notifier = this.queueNotification;Call sites outside this range will also need to inject queueNotification.
As per coding guidelines, "Protocol layer must be fully self-contained with zero imports from parent directories. Receive adapters via constructor injection through interfaces in src/lib/protocol/interfaces/."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@protocol/src/lib/protocol/graphs/opportunity.graph.ts` around lines 2659 -
2663, Remove the layering-violating dynamic import fallback and make the
notifier explicit: delete the import(...) fallback in opportunity.graph.ts so
the code only uses the injected this.queueNotification (typed as
QueueOpportunityNotificationFn | undefined), update the
OpportunityGraph/constructor signature (or factory) to accept a
QueueOpportunityNotificationFn via constructor injection for consumers, and then
update all call sites (protocol/src/lib/protocol/tools/index.ts,
protocol/src/services/tool.service.ts,
protocol/src/services/opportunity.service.ts and any other creators) to pass the
concrete queueOpportunityNotification implementation (from
src/queues/notification.queue) into the graph's constructor; ensure interfaces
in src/lib/protocol/interfaces/ expose the notifier type if not already so the
protocol layer has no parent-directory imports.
1 participant
Summary
typescript-eslintv8+)eslint-plugin-boundaries: Enforces strict layering rules in protocol — controllers→services→adapters, protocol layer isolation, no cross-service importslib/protocol/tools/index.ts(adapter injection), 6 controllers (routed through services), andchat.graph.ts(removed service import)scripts/check-adapter-names.shflags adapters named after technology instead of conceptlint-stagedruns ESLint on staged files before commitbun run lint+ adapter naming check on PRs todevEnforcement Stack
Boundary Rules Enforced
Test files (
*.spec.ts,*.test.ts,tests/) are excluded from boundary checks.Known Pre-existing Issues
no-explicit-any,no-unused-vars) — unrelated to this PReslint-disablecomments for known protocol→service violations incontact.tools.tsandintegration.tools.ts(tracked for follow-up refactoring)Test Plan
cd protocol && bun run lint— zero boundary violations./scripts/check-adapter-names.sh— passescd protocol && tsc --noEmit— zero type errorsimport { x } from "../adapters/database.adapter"to a controller → ESLint error appearscd protocol && bun test— all tests passSummary by CodeRabbit