fix(extract-conventions): switch event handler detection from interface to decorator by NTCoding · Pull Request #283 · NTCoding/living-architecture

NTCoding · 2026-04-08T20:29:14Z

Summary

Switch event handler ESLint rule from implementsInterface('EventHandlerDef') to hasDecorator('EventHandlerContainer') — the golden path now requires only a decorator, no interfaces
Change extraction config from fromGenericArg (reading IEventHandler<T>) to fromProperty (reading subscribedEvents instance property directly)
Add hasStringLiteralArrayValue predicate that rejects non-string literals like [42, true] (the old hasLiteralArrayValue accepted any literal type)
Remove dead exports (hasLiteralValue, hasLiteralArrayValue) that had no consumers
Replace type guard functions with in-operator narrowing on typed unions (SD-019)

Closes #271

Test plan

All 168 tests pass with 100% coverage
pnpm verify passes (lint, typecheck, build, test, knip, depcruise)
New test cases: non-string literal arrays, mixed string/number arrays, decorator-based detection

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Decorator-based detection for event handler rules.
- PR review retrieval switched to GraphQL for more reliable thread data.
Bug Fixes
- Extraction now sources subscribed events from an instance property.
- Validation tightened to require subscribed events as arrays of string literals.
Tests
- Expanded and updated test coverage for extraction and linting rules.
Chores
- Updated dev workflow tooling version and CLI invocation.

…ace to decorator (#271) Change extraction to read subscribedEvents from class property instead of generic type args. Switch ESLint rule trigger from implements EventHandlerDef to @EventHandlerContainer decorator. Fix dev-workflow-v2 plugin compatibility with updated agentic-workflow-builder API. Co-Authored-By: Claude Haiku 4.5 <[email protected]>

…s, not any literals hasLiteralArrayValue() accepted non-string literals like [42, true] but the error message promised "array of string literals". Added hasStringLiteralArrayValue() that checks typeof element.value === 'string' and switched the event-handler rule to use it. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

…cement - Replace type guard functions with in-operator narrowing on typed unions (SD-019) - Strengthen ESLint rule test assertion from toBeDefined to toHaveProperty (TS-003) - Add hasStringLiteralArrayValue predicate that rejects non-string literals (RFC-008) - Switch event-handler rule to use string-specific validation Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

…heck Remove hasLiteralValue and hasLiteralArrayValue (dead exports superseded by hasStringLiteralArrayValue). Accept ComponentRule | undefined in test assertion helpers to satisfy TypeScript index access. Added edge case test for mixed string/number array rejection. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Test said "6 required component types" but there are 7 (api, useCase, domainOp, event, eventHandler, eventPublisher, ui). Also added missing 'glob' key to the requiredKeys assertion array. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

coderabbitai · 2026-04-08T20:29:29Z

📝 Walkthrough

Walkthrough

Extraction and linting for event handlers were switched from interface-based detection to decorator/property-based detection: extraction reads subscribedEvents from an instance property, ESLint now targets @EventHandlerContainer, AST predicates added/changed, tests updated, and some tooling/CLI manifest and entrypoint adjustments applied.

Changes

Cohort / File(s)	Summary
Extraction config & tests `packages/riviere-extract-conventions/src/default-extraction.config.json`, `packages/riviere-extract-conventions/src/default-config.spec.ts`	`eventHandler.subscribedEvents` extraction switched from `fromGenericArg` (IEventHandler) to `fromProperty` (`subscribedEvents`). Tests refactored to use `ComponentRule` and `narrowToDetectionRule`, added `assertExtractionConfig`, updated expected keys/lengths and a new extraction test.
ESLint rule & tests `packages/riviere-extract-conventions/src/eslint/event-handler-requires-subscribed-events.cjs`, `packages/riviere-extract-conventions/src/eslint/event-handler-requires-subscribed-events.spec.ts`	Rule trigger changed from `implements EventHandlerDef` to `hasDecorator(node,'EventHandlerContainer')`. Validation narrowed to require arrays of string literals; messaging/tests updated and added invalid cases for non-string elements. Test fixtures updated to use decorator annotations.
AST predicate utilities `packages/riviere-extract-conventions/src/eslint/interface-ast-predicates.cjs`	Added exported `hasDecorator(node, decoratorName)`. Removed `hasLiteralValue`/`hasLiteralArrayValue`. `hasStringLiteralArrayValue` tightened to require `Literal` elements with `typeof element.value === 'string'`.
Developer workflow / CLI `tools/dev-workflow-v2/.claude-plugin/plugin.json`, `tools/dev-workflow-v2/commands/workflow.md`, `tools/dev-workflow-v2/src/shell/cli.ts`	Plugin manifest version bumped `0.0.9` → `0.0.13`. CLI docs set `CLAUDE_SESSION_ID` env and switched entrypoint to `src/shell/cli.ts`. `cli.ts` now passes `processDeps: createDefaultProcessDeps()` to workflow CLI.
GitHub PR feedback client & tests `tools/dev-workflow-v2/src/features/.../github/get-pr-feedback.ts`, `.../get-pr-feedback.spec.ts`	Switched from `gh pr view --json reviewThreads` to a two-step GraphQL approach (`repo view` then `api graphql`); updated parsing to handle `reviewThreads.nodes` and `comments.nodes` and adjusted tests/expectations accordingly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

fix(conventions,extract): fixes required after testing phase 11 changes on demo app #244: Touches event handler extraction; directly related to changing where subscribedEvents is sourced.
feat(riviere-extract-conventions): add default extraction config with decorator detection rules #60: Related changes to extraction and ESLint predicate/rule logic for decorator-based detection and validation.
feat(riviere-extract): create riviere-extract-conventions package with decorators #59: Adds/aligns decorator-aware utilities and tests used by the updated ESLint rule and extraction behavior.

Suggested labels

Review effort 3/5

Poem

🐰 Hops and nibble, code so neat,
Decorators guide each handler's beat,
I read the property, no generics to chase,
Tests and rules now point to the place,
A little rabbit cheers this cleaner race! 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	❓ Inconclusive	Several changes appear tangential to issue `#271`: version bump in plugin.json, workflow CLI entrypoint changes, and GitHub API client refactoring from REST to GraphQL may lack clear connection to event handler detection requirements.	Clarify whether the dev-workflow and GitHub API changes are dependencies of this PR or should be addressed in a separate PR focused on issue `#271` requirements.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: switching event handler detection from interface-based (`implementsInterface`) to decorator-based (`hasDecorator`) validation.
Linked Issues check	✅ Passed	All coding requirements from issue `#271` are met: extraction now reads from instance property instead of generic arg [271], ESLint rule triggers on `@EventHandlerContainer` decorator instead of interface [271], hasDecorator helper added [271], and validation enforces string literal arrays [271].

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch issue-271-golden-path-conventions-rel

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.17.0)

tools/dev-workflow-v2/.claude-plugin/plugin.json

┌──────────────┐
│ Opengrep CLI │
└──────────────┘

�[32m✔�[39m �[1mOpengrep OSS�[0m
�[32m✔�[39m Basic security coverage for first-party code vulnerabilities.

�[1m Loading rules from local config...�[0m

tools/dev-workflow-v2/src/features/workflow/infra/external-clients/github/get-pr-feedback.ts

┌──────────────┐
│ Opengrep CLI │
└──────────────┘

�[32m✔�[39m �[1mOpengrep OSS�[0m
�[32m✔�[39m Basic security coverage for first-party code vulnerabilities.

�[1m Loading rules from local config...�[0m

tools/dev-workflow-v2/src/features/workflow/infra/external-clients/github/get-pr-feedback.spec.ts

┌──────────────┐
│ Opengrep CLI │
└──────────────┘

�[32m✔�[39m �[1mOpengrep OSS�[0m
�[32m✔�[39m Basic security coverage for first-party code vulnerabilities.

�[1m Loading rules from local config...�[0m

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

cloudflare-workers-and-pages · 2026-04-08T20:30:25Z

Deploying living-architecture with Cloudflare Pages

Latest commit:	`e4cad2c`
Status:	✅ Deploy successful!
Preview URL:	https://b5e7b129.living-architecture.pages.dev
Branch Preview URL:	https://issue-271-golden-path-conven.living-architecture.pages.dev

View logs

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@packages/riviere-extract-conventions/src/eslint/interface-ast-predicates.cjs`:
- Around line 17-24: hasDecorator currently only detects identifier-form
decorators and misses call-form decorators (e.g., `@EventHandlerContainer`()),
causing latent failures; update function hasDecorator to extract the decorator
name the same way getDecoratorName does in require-component-decorator.cjs:
handle both d.expression.type === 'Identifier' (use d.expression.name) and
d.expression.type === 'CallExpression' with callee as an Identifier (use
d.expression.callee.name), include safe null/undefined checks for nested
properties, and then compare the extracted name to decoratorName so both
identifier and call forms are recognized.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: bb4b3ae1-b8ca-4fd4-b217-85390acc2f8d

📥 Commits

Reviewing files that changed from the base of the PR and between 37bc929 and f2863a8.

⛔ Files ignored due to path filters (1)

pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (8)

packages/riviere-extract-conventions/src/default-config.spec.ts
packages/riviere-extract-conventions/src/default-extraction.config.json
packages/riviere-extract-conventions/src/eslint/event-handler-requires-subscribed-events.cjs
packages/riviere-extract-conventions/src/eslint/event-handler-requires-subscribed-events.spec.ts
packages/riviere-extract-conventions/src/eslint/interface-ast-predicates.cjs
tools/dev-workflow-v2/.claude-plugin/plugin.json
tools/dev-workflow-v2/commands/workflow.md
tools/dev-workflow-v2/src/shell/cli.ts

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@tools/dev-workflow-v2/src/features/workflow/infra/external-clients/github/get-pr-feedback.ts`:
- Line 26: The graphqlResponseSchema definition is a single extremely long line
and should be reformatted for readability: break the nested z.object call into
multiple lines and indent each level (top-level data, repository, pullRequest,
reviewThreads, nodes) so the schema hierarchy is clear; reference
graphqlResponseSchema and graphqlThreadNodeSchema when restructuring so you keep
nodes: z.array(graphqlThreadNodeSchema) intact and preserve the exact shape and
types while only changing whitespace and line breaks.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 90efedf9-f5b1-4b89-8619-6d678489cadf

📥 Commits

Reviewing files that changed from the base of the PR and between f2863a8 and e4cad2c.

📒 Files selected for processing (3)

tools/dev-workflow-v2/.claude-plugin/plugin.json
tools/dev-workflow-v2/src/features/workflow/infra/external-clients/github/get-pr-feedback.spec.ts
tools/dev-workflow-v2/src/features/workflow/infra/external-clients/github/get-pr-feedback.ts

sonarqubecloud · 2026-04-08T21:11:46Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

= and others added 5 commits April 8, 2026 20:34

coderabbitai Bot requested changes Apr 8, 2026

View reviewed changes

Comment thread packages/riviere-extract-conventions/src/eslint/interface-ast-predicates.cjs

fix(dev-workflow-v2): use GraphQL API for PR review threads

e4cad2c

coderabbitai Bot requested changes Apr 8, 2026

View reviewed changes

Comment thread tools/dev-workflow-v2/src/features/workflow/infra/external-clients/github/get-pr-feedback.ts

coderabbitai Bot approved these changes Apr 8, 2026

View reviewed changes

NTCoding merged commit bc2e1eb into main Apr 8, 2026
17 checks passed

coderabbitai Bot mentioned this pull request Apr 10, 2026

fix(extract): restore component enrichment coverage #287

Merged

Navigation Menu

Repository navigation

fix(extract-conventions): switch event handler detection from interface to decorator - #283

fix(extract-conventions): switch event handler detection from interface to decorator#283
NTCoding merged 6 commits into
mainfrom
issue-271-golden-path-conventions-rel

NTCoding commented Apr 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 8, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

❌ Failed checks (1 inconclusive)

Uh oh!

cloudflare-workers-and-pages Bot commented Apr 8, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented Apr 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

NTCoding commented Apr 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

❌ Failed checks (1 inconclusive)

Uh oh!

cloudflare-workers-and-pages Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying living-architecture with Cloudflare Pages

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented Apr 8, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

NTCoding commented Apr 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 8, 2026 •

edited

Loading

cloudflare-workers-and-pages Bot commented Apr 8, 2026 •

edited

Loading