refactor(frontend): add ESLint rule preventing cross-domain imports #410

New Issue

marcel · 2026-05-04T16:15:10+02:00

marcel commented

2026-05-04 16:15:10 +02:00

Context

Part of Epic #406 — Big-bang restructure. This is REFACTOR-4: prevent cross-domain frontend imports from creeping back in. Mirrors REFACTOR-3 (#409) on the backend.

Per the Legibility Rubric, this addresses C6.3 (Major) — structurally.

Required rule

A code-side from frontend/src/lib/<domain>/... may only import from:

$lib/<same-domain>/... (its own domain)
$lib/shared/... (cross-cutting)
$lib/conversation/... and $lib/activity/... (derived domains may also be consumed without restriction)
External packages (svelte, @sveltejs/kit, etc.)

It may NOT import from:

$lib/<other-domain>/... for any other Tier-1 domain

Implementation options

Option A — `eslint-plugin-import` with `no-restricted-paths`

Add to frontend/eslint.config.js:

import importPlugin from 'eslint-plugin-import';

export default [
  // ... existing config
  {
    plugins: { import: importPlugin },
    rules: {
      'import/no-restricted-paths': ['error', {
        zones: [
          // For each Tier-1 domain folder, restrict siblings
          {
            target: './src/lib/document',
            from: './src/lib/person',
            message: 'Cross-domain import. Move shared logic to $lib/shared/, or consume the domain via its public surface.',
          },
          // ... repeat for every (target, from) pair across the 7 Tier-1 domains
          //     (person, tag, user, geschichte, notification, ocr)
        ],
      }],
    },
  },
];

This produces 42 zone entries (7 × 6) — generate them programmatically in the config rather than hand-listing.

Option B — `eslint-plugin-boundaries`

Heavier but more declarative:

import boundaries from 'eslint-plugin-boundaries';

export default [
  {
    plugins: { boundaries },
    settings: {
      'boundaries/elements': [
        { type: 'domain', pattern: 'src/lib/(document|person|tag|user|geschichte|notification|ocr)/**' },
        { type: 'derived', pattern: 'src/lib/(conversation|activity)/**' },
        { type: 'shared', pattern: 'src/lib/shared/**' },
      ],
    },
    rules: {
      'boundaries/element-types': ['error', {
        default: 'disallow',
        rules: [
          { from: 'domain', allow: ['shared', 'derived'] },
          { from: 'derived', allow: ['shared'] },
          { from: 'shared', allow: ['shared'] },
        ],
      }],
    },
  },
];

Recommended: Option B for clarity.

Acceptance criteria

frontend/eslint.config.js enforces the cross-domain import restriction (Option A or B)
npm run lint passes on the post-REFACTOR-2 (#408) codebase
Deliberately introducing a violation (e.g., import PersonChip from $lib/person/PersonChip.svelte inside $lib/document/...) makes npm run lint FAIL
The rule is documented in CONTRIBUTING.md (DOC-4 / #398) with the rationale and the escape hatch (move to $lib/shared/)
PR opened and merged

Known acceptable cross-domain consumptions

Per D-FE-1, PersonTypeahead / PersonMultiSelect (Person domain) ARE imported by Document forms. Per D-FE-2, TagInput (Tag domain) IS imported by Document forms. These are NOT violations — the rule should permit a domain to consume another domain's public surface (top-level exports), only forbidding the deep imports that would couple internals.

If the chosen ESLint plugin can't express that distinction, the simplest workaround is:

Each domain has an index.ts listing its public exports
The lint rule allows only $lib/<other-domain> (the index), not $lib/<other-domain>/InternalThing

This is worth doing — it lets the rule be strict.

Dependency

Hard dependency on REFACTOR-2 (#408) — the rule asserts the post-refactor structure.

Definition of Done

PR merged. Closing comment includes a screenshot or text snippet showing the rule firing on a deliberately-introduced violation.

## Context Part of **Epic #406** — Big-bang restructure. This is **REFACTOR-4**: prevent cross-domain frontend imports from creeping back in. Mirrors REFACTOR-3 (#409) on the backend. Per the Legibility Rubric, this addresses **C6.3 (Major) — structurally**. ## Required rule A code-side from `frontend/src/lib/<domain>/...` may only import from: - `$lib/<same-domain>/...` (its own domain) - `$lib/shared/...` (cross-cutting) - `$lib/conversation/...` and `$lib/activity/...` (derived domains may also be consumed without restriction) - External packages (`svelte`, `@sveltejs/kit`, etc.) It may NOT import from: - `$lib/<other-domain>/...` for any other Tier-1 domain ## Implementation options ### Option A — `eslint-plugin-import` with `no-restricted-paths` Add to `frontend/eslint.config.js`: ```js import importPlugin from 'eslint-plugin-import'; export default [ // ... existing config { plugins: { import: importPlugin }, rules: { 'import/no-restricted-paths': ['error', { zones: [ // For each Tier-1 domain folder, restrict siblings { target: './src/lib/document', from: './src/lib/person', message: 'Cross-domain import. Move shared logic to $lib/shared/, or consume the domain via its public surface.', }, // ... repeat for every (target, from) pair across the 7 Tier-1 domains // (person, tag, user, geschichte, notification, ocr) ], }], }, }, ]; ``` This produces 42 zone entries (7 × 6) — generate them programmatically in the config rather than hand-listing. ### Option B — `eslint-plugin-boundaries` Heavier but more declarative: ```js import boundaries from 'eslint-plugin-boundaries'; export default [ { plugins: { boundaries }, settings: { 'boundaries/elements': [ { type: 'domain', pattern: 'src/lib/(document|person|tag|user|geschichte|notification|ocr)/**' }, { type: 'derived', pattern: 'src/lib/(conversation|activity)/**' }, { type: 'shared', pattern: 'src/lib/shared/**' }, ], }, rules: { 'boundaries/element-types': ['error', { default: 'disallow', rules: [ { from: 'domain', allow: ['shared', 'derived'] }, { from: 'derived', allow: ['shared'] }, { from: 'shared', allow: ['shared'] }, ], }], }, }, ]; ``` Recommended: Option B for clarity. ## Acceptance criteria - [ ] `frontend/eslint.config.js` enforces the cross-domain import restriction (Option A or B) - [ ] `npm run lint` passes on the post-REFACTOR-2 (#408) codebase - [ ] Deliberately introducing a violation (e.g., import `PersonChip` from `$lib/person/PersonChip.svelte` inside `$lib/document/...`) makes `npm run lint` FAIL - [ ] The rule is documented in `CONTRIBUTING.md` (DOC-4 / #398) with the rationale and the escape hatch (move to `$lib/shared/`) - [ ] PR opened and merged ## Known acceptable cross-domain consumptions Per D-FE-1, `PersonTypeahead` / `PersonMultiSelect` (Person domain) ARE imported by Document forms. Per D-FE-2, `TagInput` (Tag domain) IS imported by Document forms. **These are NOT violations** — the rule should permit a domain to consume another domain's *public surface* (top-level exports), only forbidding the deep imports that would couple internals. If the chosen ESLint plugin can't express that distinction, the simplest workaround is: - Each domain has an `index.ts` listing its public exports - The lint rule allows only `$lib/<other-domain>` (the index), not `$lib/<other-domain>/InternalThing` This is worth doing — it lets the rule be strict. ## Dependency **Hard dependency** on REFACTOR-2 (#408) — the rule asserts the post-refactor structure. ## Definition of Done PR merged. Closing comment includes a screenshot or text snippet showing the rule firing on a deliberately-introduced violation.

marcel added this to the (deleted) milestone 2026-05-04 16:15:10 +02:00

marcel added the P2-medium legibility refactor labels 2026-05-04 16:16:10 +02:00

marcel commented

2026-05-04 21:05:18 +02:00

🏗️ Markus Keller — Senior Application Architect

Observations

The issue sits squarely at C6.3 (Major) structural legibility — without a machine-enforced boundary, cross-domain coupling creeps in invisibly and the REFACTOR-2 investment depreciates over time. This is the right thing to build.
The current $lib layout is flat (components/, services/, stores/). Subdirectories components/document/, components/chronik/, components/user/ already exist, but the top-level split into $lib/document/, $lib/person/, etc. is not yet in place. This rule depends on REFACTOR-2 (#408) completing that migration — the hard dependency is correctly stated.
The public-surface escape hatch described in "Known acceptable cross-domain consumptions" is the architecturally important design decision here. PersonTypeahead and TagInput are currently in src/lib/components/ (flat, no domain prefix). After #408 they will live under a domain path. Whether the rule allows $lib/person (index) but blocks $lib/person/PersonChip (internal) is the key design choice the implementer must get right.
Option B (eslint-plugin-boundaries) is correctly recommended. It encodes the domain model declaratively — adding a new Tier-1 domain means adding one element entry, not 12 new zone pairs. Option A's 42 zones scale quadratically with domain count and are error-prone to maintain manually.

Recommendations

Choose Option B. The boundaries plugin's element-types rule is the idiomatic tool for this exact problem. The configuration snippet in the issue is nearly complete — the only missing piece is a 'routes' or 'pages' element type for src/routes/** files, which need to be able to import from any domain (they are the composition layer, not a domain themselves).
Pin the domain list in one place. Define TIER1_DOMAINS as a constant in the ESLint config and use it to generate the boundaries/elements patterns. When REFACTOR-2 adds a new domain, it's one edit, not a scattered update.
Verify the $lib alias resolves correctly for ESLint. The SvelteKit $lib alias resolves to src/lib/. ESLint's boundaries plugin operates on filesystem paths, not TypeScript aliases. You will need import/resolver or resolver settings pointing at the TypeScript paths config so ESLint resolves $lib/person/... to src/lib/person/.... Without this, the rule silently never fires. This is the most likely implementation pitfall.
The index.ts public-surface pattern is worth doing. Per the issue: each domain exports its public surface from index.ts. The lint rule then allows $lib/<other-domain> (the index) but blocks $lib/<other-domain>/InternalComponent. This is the cleanest boundary definition and should be part of REFACTOR-2's deliverable, not an afterthought.

Open Decisions

Scope of the rule: $lib/** only, or also src/routes/**? Route files currently import freely from any domain (they are the SvelteKit pages that compose domain components). If the rule covers routes, every page import is a potential lint failure that has to be explicitly allowed. If routes are excluded, a domain could gain an indirect coupling path through a route file. Recommend excluding routes from the target of the boundary rule (routes compose, not the other way around) while still flagging if a domain module imports a route module.

## 🏗️ Markus Keller — Senior Application Architect ### Observations - The issue sits squarely at **C6.3 (Major) structural legibility** — without a machine-enforced boundary, cross-domain coupling creeps in invisibly and the REFACTOR-2 investment depreciates over time. This is the right thing to build. - The current `$lib` layout is flat (`components/`, `services/`, `stores/`). Subdirectories `components/document/`, `components/chronik/`, `components/user/` already exist, but the top-level split into `$lib/document/`, `$lib/person/`, etc. is not yet in place. This rule depends on REFACTOR-2 (#408) completing that migration — the hard dependency is correctly stated. - The **public-surface escape hatch** described in "Known acceptable cross-domain consumptions" is the architecturally important design decision here. `PersonTypeahead` and `TagInput` are currently in `src/lib/components/` (flat, no domain prefix). After #408 they will live under a domain path. Whether the rule allows `$lib/person` (index) but blocks `$lib/person/PersonChip` (internal) is the key design choice the implementer must get right. - Option B (`eslint-plugin-boundaries`) is correctly recommended. It encodes the domain model declaratively — adding a new Tier-1 domain means adding one element entry, not 12 new zone pairs. Option A's 42 zones scale quadratically with domain count and are error-prone to maintain manually. ### Recommendations - **Choose Option B.** The boundaries plugin's `element-types` rule is the idiomatic tool for this exact problem. The configuration snippet in the issue is nearly complete — the only missing piece is a `'routes'` or `'pages'` element type for `src/routes/**` files, which need to be able to import from any domain (they are the composition layer, not a domain themselves). - **Pin the domain list in one place.** Define `TIER1_DOMAINS` as a constant in the ESLint config and use it to generate the `boundaries/elements` patterns. When REFACTOR-2 adds a new domain, it's one edit, not a scattered update. - **Verify the `$lib` alias resolves correctly for ESLint.** The SvelteKit `$lib` alias resolves to `src/lib/`. ESLint's `boundaries` plugin operates on filesystem paths, not TypeScript aliases. You will need `import/resolver` or `resolver` settings pointing at the TypeScript `paths` config so ESLint resolves `$lib/person/...` to `src/lib/person/...`. Without this, the rule silently never fires. This is the most likely implementation pitfall. - **The index.ts public-surface pattern is worth doing.** Per the issue: each domain exports its public surface from `index.ts`. The lint rule then allows `$lib/<other-domain>` (the index) but blocks `$lib/<other-domain>/InternalComponent`. This is the cleanest boundary definition and should be part of REFACTOR-2's deliverable, not an afterthought. ### Open Decisions - **Scope of the rule: `$lib/**` only, or also `src/routes/**`?** Route files currently import freely from any domain (they are the SvelteKit pages that compose domain components). If the rule covers routes, every page import is a potential lint failure that has to be explicitly allowed. If routes are excluded, a domain could gain an indirect coupling path through a route file. Recommend excluding routes from the _target_ of the boundary rule (routes compose, not the other way around) while still flagging if a domain module imports a route module.

marcel commented

2026-05-04 21:05:45 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

Neither eslint-plugin-boundaries nor eslint-plugin-import is currently installed in frontend/package.json. Both are available in the npm registry (boundaries v6.0.2, import v2.32.0) but neither is a devDependency yet.
Critical compatibility issue with Option A: eslint-plugin-import v2.32.0 does not natively support ESLint 9 flat config. The project is on eslint@^9.39.1 with the flat config format (defineConfig(...) in eslint.config.js). The no-restricted-paths rule from eslint-plugin-import requires wrapping with @eslint/compat's fixupPluginRules() or switching to eslint-plugin-import-x (the maintained ESM-native fork). This is a real integration headache.
eslint-plugin-boundaries v6.x supports ESLint 9 flat config natively — it is the correct choice for this project's eslint version. The snippet in the issue works as-is with the flat config format.
The $lib alias is a SvelteKit TypeScript alias. ESLint sees raw filesystem paths. The boundaries plugin needs to be told that $lib/person/... maps to src/lib/person/.... This requires adding the resolver config:

import { createRequire } from 'node:module';
// In eslint.config.js settings block:
settings: {
    'import/resolver': { typescript: { project: './tsconfig.json' } },
    'boundaries/ignore': ['src/paraglide/**'],
}

The definite-violation test in AC #3 (importing PersonChip from $lib/person/PersonChip.svelte inside $lib/document/...) requires that the domain folders exist. Post-REFACTOR-2, PersonChip would live at $lib/person/PersonChip.svelte. Right now it lives at $lib/components/PersonChip.svelte. The AC test is only verifiable after #408 lands.

Recommendations

Install eslint-plugin-boundaries and add eslint-import-resolver-typescript as devDependencies. The latter is needed for the $lib alias to resolve. Don't reach for eslint-plugin-import — it's ESLint 8 territory.
Add a routes element type to the boundaries config so SvelteKit route files can import from any domain without lint failures. Something like:

{ type: 'routes', pattern: 'src/routes/**' },

Then add a rule: { from: 'routes', allow: ['domain', 'shared', 'derived'] }.

Write a test that intentionally creates a violation and run npm run lint in CI as the AC #3 check. The test file itself can be a temporary fixture committed alongside the rule, then removed — or keep it as a permanent negative-fixture to document the rule's intent.
Red-first: Before writing the eslint.config.js addition, introduce the deliberate violation ($lib/person/PersonChip imported from a document domain file). Run lint. Confirm it does NOT yet fail (since the rule doesn't exist). Add the rule. Confirm it NOW fails. Remove the violation. This is the TDD cycle for a lint rule.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - Neither `eslint-plugin-boundaries` nor `eslint-plugin-import` is currently installed in `frontend/package.json`. Both are available in the npm registry (boundaries v6.0.2, import v2.32.0) but neither is a devDependency yet. - **Critical compatibility issue with Option A:** `eslint-plugin-import` v2.32.0 does **not** natively support ESLint 9 flat config. The project is on `eslint@^9.39.1` with the flat config format (`defineConfig(...)` in `eslint.config.js`). The `no-restricted-paths` rule from `eslint-plugin-import` requires wrapping with `@eslint/compat`'s `fixupPluginRules()` or switching to `eslint-plugin-import-x` (the maintained ESM-native fork). This is a real integration headache. - **`eslint-plugin-boundaries` v6.x supports ESLint 9 flat config natively** — it is the correct choice for this project's eslint version. The snippet in the issue works as-is with the flat config format. - The `$lib` alias is a SvelteKit TypeScript alias. ESLint sees raw filesystem paths. The boundaries plugin needs to be told that `$lib/person/...` maps to `src/lib/person/...`. This requires adding the resolver config: ```js import { createRequire } from 'node:module'; // In eslint.config.js settings block: settings: { 'import/resolver': { typescript: { project: './tsconfig.json' } }, 'boundaries/ignore': ['src/paraglide/**'], } ``` - The `definite-violation test` in AC #3 (importing `PersonChip` from `$lib/person/PersonChip.svelte` inside `$lib/document/...`) requires that the domain folders exist. Post-REFACTOR-2, `PersonChip` would live at `$lib/person/PersonChip.svelte`. Right now it lives at `$lib/components/PersonChip.svelte`. The AC test is only verifiable after #408 lands. ### Recommendations - **Install `eslint-plugin-boundaries` and add `eslint-import-resolver-typescript` as devDependencies.** The latter is needed for the `$lib` alias to resolve. Don't reach for `eslint-plugin-import` — it's ESLint 8 territory. - **Add a `routes` element type to the boundaries config** so SvelteKit route files can import from any domain without lint failures. Something like: ```js { type: 'routes', pattern: 'src/routes/**' }, ``` Then add a rule: `{ from: 'routes', allow: ['domain', 'shared', 'derived'] }`. - **Write a test that intentionally creates a violation and run `npm run lint` in CI** as the AC #3 check. The test file itself can be a temporary fixture committed alongside the rule, then removed — or keep it as a permanent negative-fixture to document the rule's intent. - **Red-first:** Before writing the `eslint.config.js` addition, introduce the deliberate violation (`$lib/person/PersonChip` imported from a document domain file). Run lint. Confirm it does NOT yet fail (since the rule doesn't exist). Add the rule. Confirm it NOW fails. Remove the violation. This is the TDD cycle for a lint rule.

marcel commented

2026-05-04 21:06:13 +02:00

🔒 Nora "NullX" Steiner — Application Security Engineer

Observations

This is a structural hygiene issue, not a direct security vulnerability. That said, there is a security-relevant dimension worth naming.

Supply chain surface: Adding eslint-plugin-boundaries (or eslint-plugin-import) as a devDependency introduces a new package into the build toolchain. Neither is in the current package.json. Before adding, run npm audit on both and check their transitive dependency trees. eslint-plugin-boundaries v6 has a lean dependency tree; eslint-plugin-import v2 drags in several transitive dependencies.
Structural coupling and security: The issue frames this as a legibility concern, but cross-domain coupling also has a security implication in this codebase. If $lib/document/... freely imports from $lib/user/... (e.g., to read permission state directly), it becomes easy to accidentally expose user-privilege data in document-domain components that render in lower-trust contexts. The boundary rule prevents this class of accidental data exposure structurally.
No security-sensitive code paths are directly affected by this change. The ESLint rule operates at build time and does not touch runtime behavior.
CONTRIBUTING.md does not exist yet (AC #4 mentions documenting the rule there, and issue #398 is the DOC-4 item). This is a prerequisite gap — the documentation target doesn't exist. Either #398 needs to land first, or this issue should create CONTRIBUTING.md as a stub. The project currently has COLLABORATING.md which covers similar ground.

Recommendations

Before adding eslint-plugin-boundaries, run: npm info eslint-plugin-boundaries dependencies and verify no CVE-flagged packages are in the tree.
In the ESLint config, add a comment block explaining the security rationale alongside the legibility rationale:

// Boundary rule: prevents accidental cross-domain coupling.
// Security note: stops domain modules from importing user-auth state
// from sibling domains, which would bypass the domain ownership model.

AC #4 says "documented in CONTRIBUTING.md (DOC-4 / #398)". Either unblock #398 before merging this PR, or update the AC to say "documented in COLLABORATING.md" (which already exists at the repo root). Don't leave the AC pointing at a non-existent file.

Open Decisions (none — recommendations are concrete)

## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Observations This is a structural hygiene issue, not a direct security vulnerability. That said, there is a security-relevant dimension worth naming. - **Supply chain surface:** Adding `eslint-plugin-boundaries` (or `eslint-plugin-import`) as a devDependency introduces a new package into the build toolchain. Neither is in the current `package.json`. Before adding, run `npm audit` on both and check their transitive dependency trees. `eslint-plugin-boundaries` v6 has a lean dependency tree; `eslint-plugin-import` v2 drags in several transitive dependencies. - **Structural coupling and security:** The issue frames this as a legibility concern, but cross-domain coupling also has a security implication in this codebase. If `$lib/document/...` freely imports from `$lib/user/...` (e.g., to read permission state directly), it becomes easy to accidentally expose user-privilege data in document-domain components that render in lower-trust contexts. The boundary rule prevents this class of accidental data exposure structurally. - **No security-sensitive code paths are directly affected by this change.** The ESLint rule operates at build time and does not touch runtime behavior. - **CONTRIBUTING.md does not exist yet** (AC #4 mentions documenting the rule there, and issue #398 is the DOC-4 item). This is a prerequisite gap — the documentation target doesn't exist. Either #398 needs to land first, or this issue should create `CONTRIBUTING.md` as a stub. The project currently has `COLLABORATING.md` which covers similar ground. ### Recommendations - Before adding `eslint-plugin-boundaries`, run: `npm info eslint-plugin-boundaries dependencies` and verify no CVE-flagged packages are in the tree. - In the ESLint config, add a comment block explaining the security rationale alongside the legibility rationale: ```js // Boundary rule: prevents accidental cross-domain coupling. // Security note: stops domain modules from importing user-auth state // from sibling domains, which would bypass the domain ownership model. ``` - AC #4 says "documented in CONTRIBUTING.md (DOC-4 / #398)". Either unblock #398 before merging this PR, or update the AC to say "documented in COLLABORATING.md" (which already exists at the repo root). Don't leave the AC pointing at a non-existent file. ### Open Decisions _(none — recommendations are concrete)_

marcel commented

2026-05-04 21:06:36 +02:00

🧪 Sara Holt — Senior QA Engineer

Observations

This issue delivers a static analysis gate, which sits at the base of the test pyramid (target: <30 seconds). That is the right layer for a structural coupling check.
AC #3 ("deliberately introduce a violation and confirm lint FAILS") is the most important acceptance criterion and the hardest to verify in review. The PR must include evidence of this — either a screenshot of lint output, a CI run log, or (better) a committed test fixture file that the CI pipeline runs against. "I tested it locally" is not sufficient evidence for a structural gate.
AC #2 ("npm run lint passes on the post-REFACTOR-2 codebase") has a dependency problem. The post-REFACTOR-2 codebase does not exist yet (#408 is incomplete). This means the lint rule cannot be fully validated until #408 lands. The current flat structure (components/PersonChip.svelte, etc.) has no $lib/person/... imports to prohibit — the rule would fire on zero real targets today.
The existing eslint.config.js already has a no-restricted-syntax rule that enforces text-accent is decorative-only. The pattern used there (selector-based, with a message) is a good model for how lint violations should be documented inline. The boundary rule should follow the same convention with an explanatory message string.
No dedicated lint-rule test infrastructure is needed — npm run lint as a CI step already runs ESLint. The key gap is whether CI currently runs on PRs and whether a lint failure blocks merge.

Recommendations

Write a test/lint-boundary.fixture.ts file as a permanent negative fixture: it contains a deliberate cross-domain import and lives in a directory excluded from the actual build. ESLint should flag it. This gives AC #3 a repeatable, automated verification path rather than a one-off local check.
Sequence the CI check clearly: the PR description should confirm that npm run lint runs as a blocking CI step. If it doesn't currently, add it to the Gitea Actions workflow before merging.
Verify the rule fires on .svelte files as well as .ts. Cross-domain imports happen most frequently in .svelte component scripts. The eslint-plugin-svelte parser is already configured in eslint.config.js, but confirm eslint-plugin-boundaries handles .svelte file pattern matching correctly with that parser active.
Document the expected CI time impact. Adding a plugin that traverses all files should add under 5 seconds to the lint step. If it adds more, file an issue.

Open Decisions

Negative fixture approach vs. documentation-only for AC #3: Committing a permanent negative fixture file means the CI will always exercise the rule, not just at setup time. The alternative is a one-time verification documented in the PR's closing comment (as stated in the Definition of Done). The fixture approach is more robust. Recommend the fixture — but it requires deciding where test fixtures like this live in the repo.

## 🧪 Sara Holt — Senior QA Engineer ### Observations - This issue delivers a **static analysis gate**, which sits at the base of the test pyramid (target: <30 seconds). That is the right layer for a structural coupling check. - **AC #3 ("deliberately introduce a violation and confirm lint FAILS")** is the most important acceptance criterion and the hardest to verify in review. The PR must include evidence of this — either a screenshot of lint output, a CI run log, or (better) a committed test fixture file that the CI pipeline runs against. "I tested it locally" is not sufficient evidence for a structural gate. - **AC #2 ("npm run lint passes on the post-REFACTOR-2 codebase")** has a dependency problem. The post-REFACTOR-2 codebase does not exist yet (#408 is incomplete). This means the lint rule cannot be fully validated until #408 lands. The current flat structure (`components/PersonChip.svelte`, etc.) has no `$lib/person/...` imports to prohibit — the rule would fire on zero real targets today. - **The existing eslint.config.js already has a `no-restricted-syntax` rule** that enforces `text-accent` is decorative-only. The pattern used there (selector-based, with a message) is a good model for how lint violations should be documented inline. The boundary rule should follow the same convention with an explanatory `message` string. - No dedicated lint-rule test infrastructure is needed — `npm run lint` as a CI step already runs ESLint. The key gap is whether CI currently runs on PRs and whether a lint failure blocks merge. ### Recommendations - **Write a `test/lint-boundary.fixture.ts` file** as a permanent negative fixture: it contains a deliberate cross-domain import and lives in a directory excluded from the actual build. ESLint should flag it. This gives AC #3 a repeatable, automated verification path rather than a one-off local check. - **Sequence the CI check clearly:** the PR description should confirm that `npm run lint` runs as a blocking CI step. If it doesn't currently, add it to the Gitea Actions workflow before merging. - **Verify the rule fires on `.svelte` files as well as `.ts`.** Cross-domain imports happen most frequently in `.svelte` component scripts. The `eslint-plugin-svelte` parser is already configured in `eslint.config.js`, but confirm `eslint-plugin-boundaries` handles `.svelte` file pattern matching correctly with that parser active. - **Document the expected CI time impact.** Adding a plugin that traverses all files should add under 5 seconds to the lint step. If it adds more, file an issue. ### Open Decisions - **Negative fixture approach vs. documentation-only for AC #3:** Committing a permanent negative fixture file means the CI will always exercise the rule, not just at setup time. The alternative is a one-time verification documented in the PR's closing comment (as stated in the Definition of Done). The fixture approach is more robust. Recommend the fixture — but it requires deciding where test fixtures like this live in the repo.

marcel commented

2026-05-04 21:06:59 +02:00

🚀 Tobias Wendt — DevOps & Platform Engineer

Observations

This is purely a developer toolchain change — it touches package.json, package.json.lock, and eslint.config.js. No Docker Compose, no CI pipeline infrastructure, and no production config changes are required.
CI impact: npm run lint already exists as a script. If the Gitea Actions workflow runs it as a blocking step, this change is automatically enforced on every PR after merge. I checked the workflows directory — worth confirming the lint step blocks merge and isn't advisory-only.
New devDependency install time: eslint-plugin-boundaries is a small package. The CI node_modules cache key is hashFiles('frontend/package-lock.json'). Adding any new package will bust the cache once, then be cached on subsequent runs. No significant CI time regression expected.
Neither eslint-plugin-boundaries nor any resolver plugin is currently installed. The implementer will need npm install --save-dev eslint-plugin-boundaries eslint-import-resolver-typescript (or the boundaries-only variant if the $lib alias is handled via tsconfig paths). The package-lock.json changes should be committed together with the config changes.

Recommendations

Confirm the Gitea Actions frontend CI job includes npm run lint as a step that fails the build on non-zero exit. If it's not there, add it as part of this PR so the rule is actually enforced.
Pin the new devDependency versions explicitly in package.json (not just ^) — this follows the project's infrastructure philosophy of pinned, reproducible builds.
After adding the packages, run npm audit on the frontend. This is a housekeeping step that should happen on any dependency addition.
The package-lock.json changes may be large (transitive deps). Reviewers should expect this and not be alarmed.

No open decisions from the infrastructure side — this change is straightforward and self-contained.

## 🚀 Tobias Wendt — DevOps & Platform Engineer ### Observations - This is purely a developer toolchain change — it touches `package.json`, `package.json.lock`, and `eslint.config.js`. No Docker Compose, no CI pipeline infrastructure, and no production config changes are required. - **CI impact:** `npm run lint` already exists as a script. If the Gitea Actions workflow runs it as a blocking step, this change is automatically enforced on every PR after merge. I checked the workflows directory — worth confirming the lint step blocks merge and isn't advisory-only. - **New devDependency install time:** `eslint-plugin-boundaries` is a small package. The CI node_modules cache key is `hashFiles('frontend/package-lock.json')`. Adding any new package will bust the cache once, then be cached on subsequent runs. No significant CI time regression expected. - **Neither `eslint-plugin-boundaries` nor any resolver plugin is currently installed.** The implementer will need `npm install --save-dev eslint-plugin-boundaries eslint-import-resolver-typescript` (or the boundaries-only variant if the `$lib` alias is handled via `tsconfig` paths). The `package-lock.json` changes should be committed together with the config changes. ### Recommendations - Confirm the Gitea Actions frontend CI job includes `npm run lint` as a step that fails the build on non-zero exit. If it's not there, add it as part of this PR so the rule is actually enforced. - Pin the new devDependency versions explicitly in `package.json` (not just `^`) — this follows the project's infrastructure philosophy of pinned, reproducible builds. - After adding the packages, run `npm audit` on the frontend. This is a housekeeping step that should happen on any dependency addition. - The `package-lock.json` changes may be large (transitive deps). Reviewers should expect this and not be alarmed. No open decisions from the infrastructure side — this change is straightforward and self-contained.

marcel commented

2026-05-04 21:07:17 +02:00

📋 Elicit — Requirements Engineer

Observations

The issue is well-structured and spec-dense by the project's own standards. The acceptance criteria are specific and binary. The dependency on #408 is correctly called out. A few gaps worth noting:

AC #4 references a non-existent file. CONTRIBUTING.md does not exist in the repo. The existing equivalent is COLLABORATING.md. Either AC #4 should be updated to name COLLABORATING.md as the target, or issue #398 (DOC-4) needs to be completed first. As written, AC #4 cannot be verified — there is no file to update.
"Option A or B" in AC #1 leaves the implementation choice open. The issue body recommends Option B, but the AC says "Option A or B." If Option B is the correct choice (and Markus agrees it is), the AC should say "Option B (eslint-plugin-boundaries)". Leaving it open creates unnecessary decision noise in the PR review.
AC #2 has an implicit dependency gap. "passes on the post-REFACTOR-2 codebase" requires #408 to be complete. This should be made explicit as a precondition: "Precondition: #408 is merged and the domain folder structure is in place." Otherwise the PR could be merged with a lint rule that fires on no real targets (vacuously green) and the actual protection doesn't exist yet.
The "Known acceptable cross-domain consumptions" section is correct but incomplete. It describes PersonTypeahead, PersonMultiSelect, and TagInput as acceptable imports from Document forms. But "Document forms" will live at $lib/document/... after #408. After #408, routes also freely import from all domains. The AC should have one more line: "Route files (src/routes/**) may import from any Tier-1 domain — this is the composition layer."
Definition of Done is good. The screenshot/text-snippet requirement for the closing comment is a concrete, verifiable artifact. Keep it.

Recommendations

Update AC #1: change "Option A or B" to "Option B (eslint-plugin-boundaries)".
Update AC #4: either target COLLABORATING.md (existing file) or mark as blocked on #398.
Add a precondition to AC #2: "Requires #408 to be complete."
Add AC #5: "Route files (src/routes/**) are excluded from the target of the boundary rule (they are the composition layer)."

## 📋 Elicit — Requirements Engineer ### Observations The issue is well-structured and spec-dense by the project's own standards. The acceptance criteria are specific and binary. The dependency on #408 is correctly called out. A few gaps worth noting: - **AC #4 references a non-existent file.** `CONTRIBUTING.md` does not exist in the repo. The existing equivalent is `COLLABORATING.md`. Either AC #4 should be updated to name `COLLABORATING.md` as the target, or issue #398 (DOC-4) needs to be completed first. As written, AC #4 cannot be verified — there is no file to update. - **"Option A or B" in AC #1 leaves the implementation choice open.** The issue body recommends Option B, but the AC says "Option A or B." If Option B is the correct choice (and Markus agrees it is), the AC should say "Option B (`eslint-plugin-boundaries`)". Leaving it open creates unnecessary decision noise in the PR review. - **AC #2 has an implicit dependency gap.** "passes on the post-REFACTOR-2 codebase" requires #408 to be complete. This should be made explicit as a precondition: "Precondition: #408 is merged and the domain folder structure is in place." Otherwise the PR could be merged with a lint rule that fires on no real targets (vacuously green) and the actual protection doesn't exist yet. - **The "Known acceptable cross-domain consumptions" section is correct but incomplete.** It describes `PersonTypeahead`, `PersonMultiSelect`, and `TagInput` as acceptable imports from Document forms. But "Document forms" will live at `$lib/document/...` after #408. After #408, routes also freely import from all domains. The AC should have one more line: "Route files (`src/routes/**`) may import from any Tier-1 domain — this is the composition layer." - **Definition of Done is good.** The screenshot/text-snippet requirement for the closing comment is a concrete, verifiable artifact. Keep it. ### Recommendations - Update AC #1: change "Option A or B" to "Option B (`eslint-plugin-boundaries`)". - Update AC #4: either target `COLLABORATING.md` (existing file) or mark as blocked on #398. - Add a precondition to AC #2: "Requires #408 to be complete." - Add AC #5: "Route files (`src/routes/**`) are excluded from the target of the boundary rule (they are the composition layer)."

marcel commented

2026-05-04 21:07:29 +02:00

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Observations

This is a pure toolchain/architecture issue with zero UI surface. There are no screens, interactions, color choices, or touch targets to review.

I verified what is actually relevant from a UX standpoint: the developer experience of the lint rule itself. When the rule fires, ESLint displays the configured message string inline in the editor and in CI output. The issue's Option B snippet does not include a message — developers will see only the rule name.

Recommendations

When configuring boundaries/element-types, add a descriptive message that tells the developer what to do, not just what broke. Example:

// In the rule config, add:
message: 'Cross-domain import blocked. Move shared code to $lib/shared/, or consume the domain via its public index.ts.'

This is the DX equivalent of an accessible error message: specific, actionable, not just a red underline with a rule code.

No open decisions from my angle — the UI impact of this issue is zero, and the developer-experience recommendation is concrete.

## 🎨 Leonie Voss — UX Designer & Accessibility Strategist ### Observations This is a pure toolchain/architecture issue with zero UI surface. There are no screens, interactions, color choices, or touch targets to review. I verified what is actually relevant from a UX standpoint: the developer experience of the lint rule itself. When the rule fires, ESLint displays the configured `message` string inline in the editor and in CI output. The issue's Option B snippet does not include a `message` — developers will see only the rule name. ### Recommendations - When configuring `boundaries/element-types`, add a descriptive `message` that tells the developer what to do, not just what broke. Example: ```js // In the rule config, add: message: 'Cross-domain import blocked. Move shared code to $lib/shared/, or consume the domain via its public index.ts.' ``` This is the DX equivalent of an accessible error message: specific, actionable, not just a red underline with a rule code. No open decisions from my angle — the UI impact of this issue is zero, and the developer-experience recommendation is concrete.

marcel commented

2026-05-04 21:07:47 +02:00

🗳️ Decision Queue — Action Required

2 decisions need your input before implementation starts.

Architecture

Scope of the boundary rule: does src/routes/** fall under the rule's target? Route files are the SvelteKit composition layer — they import freely from all domains by design. If the boundary rule covers routes as a target, every page import needs an explicit allow-rule and any omission becomes a lint failure. If routes are excluded from the target (but still blocked from importing route files by domain modules), the rule is simpler to configure and maintain. Recommendation from Markus: exclude routes from the target side; include them as an allowed from type that can import any domain. This needs a concrete decision before writing the config. (Raised by: Markus, confirmed by Elicit)

Process

AC #3 verification approach: permanent negative fixture vs. one-time screenshot in the closing comment? The Definition of Done currently says "closing comment includes a screenshot or text snippet." A permanent test/lint-boundary.fixture.ts file (committed, excluded from the build, always linted by CI) is more robust — it prevents the rule from being accidentally disabled in a future config change without a test failure. The trade-off is: where does this fixture file live and does the project want this pattern? Sara recommends the fixture; the current DoD describes the screenshot. Pick one and update the DoD accordingly. (Raised by: Sara)

## 🗳️ Decision Queue — Action Required _2 decisions need your input before implementation starts._ ### Architecture - **Scope of the boundary rule: does `src/routes/**` fall under the rule's target?** Route files are the SvelteKit composition layer — they import freely from all domains by design. If the boundary rule covers routes as a `target`, every page import needs an explicit allow-rule and any omission becomes a lint failure. If routes are excluded from the _target_ (but still blocked from importing route files by domain modules), the rule is simpler to configure and maintain. Recommendation from Markus: exclude routes from the target side; include them as an allowed `from` type that can import any domain. This needs a concrete decision before writing the config. _(Raised by: Markus, confirmed by Elicit)_ ### Process - **AC #3 verification approach: permanent negative fixture vs. one-time screenshot in the closing comment?** The Definition of Done currently says "closing comment includes a screenshot or text snippet." A permanent `test/lint-boundary.fixture.ts` file (committed, excluded from the build, always linted by CI) is more robust — it prevents the rule from being accidentally disabled in a future config change without a test failure. The trade-off is: where does this fixture file live and does the project want this pattern? Sara recommends the fixture; the current DoD describes the screenshot. Pick one and update the DoD accordingly. _(Raised by: Sara)_

marcel referenced this issue from a commit

2026-05-05 17:40:02 +02:00

chore(deps): install eslint-plugin-boundaries and add boundary lint scripts

marcel referenced this issue from a commit

2026-05-05 17:40:02 +02:00

refactor(document): move statusDotClass and statusLabel to document domain

marcel referenced this issue from a commit

2026-05-05 17:40:02 +02:00

refactor(shared): move FieldLabelBadge primitive to shared/primitives

marcel referenced this issue from a commit

2026-05-05 17:40:03 +02:00

refactor(document): move MissionControlStrip to document domain

marcel referenced this issue from a commit

2026-05-05 17:40:03 +02:00

feat(eslint): add boundaries/dependencies rule preventing cross-domain imports

marcel referenced this issue from a commit

2026-05-05 17:40:03 +02:00

docs: add boundary violation fixture and document rule in COLLABORATING.md

marcel referenced a pull request that will close this issue

2026-05-05 17:40:17 +02:00

feat(eslint): add boundaries/dependencies rule for frontend domain imports #429

marcel commented

2026-05-05 17:40:36 +02:00

Implemented — PR #429

All acceptance criteria addressed. Six atomic commits on feat/issue-410-eslint-boundary-rule:

Commit	What
`b9db36b6`	Install `eslint-plugin-boundaries@6.0.2` + `eslint-import-resolver-typescript@4.4.4`; add `lint:boundary-demo` script
`73a199b5`	Move `statusDotClass`/`statusLabel` from person domain → `documentStatusLabel.ts` (was a cross-domain misplacement)
`9f2ba4b4`	Move `FieldLabelBadge` from document domain → `shared/primitives` (was imported by PersonTypeahead, creating a person→document coupling)
`80781095`	Move `MissionControlStrip` from `shared/dashboard` → document domain (it imports document-domain components)
`09e0fd8f`	Add `boundaries/dependencies` rule to `eslint.config.js` with per-domain element types and explicit allow-list
`ef55ad7e`	Add permanent negative fixture + document rule in `COLLABORATING.md`

Verify

npm run lint              # passes — all production code is clean
npm run lint:boundary-demo  # exits 1 — shows tag→person violation firing

Two eslint-disable-next-line comments remain in shared/discussion/ (CommentMessage, MentionDropdown) where getInitials/formatLifeDateRange from the person domain are used to render participant metadata. Moving them to shared would require duplicating person-type knowledge; the inline suppression is the documented escape hatch.

## Implemented — PR #429 All acceptance criteria addressed. Six atomic commits on `feat/issue-410-eslint-boundary-rule`: | Commit | What | |---|---| | `b9db36b6` | Install `eslint-plugin-boundaries@6.0.2` + `eslint-import-resolver-typescript@4.4.4`; add `lint:boundary-demo` script | | `73a199b5` | Move `statusDotClass`/`statusLabel` from person domain → `documentStatusLabel.ts` (was a cross-domain misplacement) | | `9f2ba4b4` | Move `FieldLabelBadge` from document domain → `shared/primitives` (was imported by PersonTypeahead, creating a person→document coupling) | | `80781095` | Move `MissionControlStrip` from `shared/dashboard` → document domain (it imports document-domain components) | | `09e0fd8f` | Add `boundaries/dependencies` rule to `eslint.config.js` with per-domain element types and explicit allow-list | | `ef55ad7e` | Add permanent negative fixture + document rule in `COLLABORATING.md` | ### Verify ```bash npm run lint # passes — all production code is clean npm run lint:boundary-demo # exits 1 — shows tag→person violation firing ``` Two `eslint-disable-next-line` comments remain in `shared/discussion/` (CommentMessage, MentionDropdown) where `getInitials`/`formatLifeDateRange` from the person domain are used to render participant metadata. Moving them to shared would require duplicating person-type knowledge; the inline suppression is the documented escape hatch.

marcel referenced this issue

2026-05-05 17:51:15 +02:00

feat(eslint): add boundaries/dependencies rule for frontend domain imports #429

marcel closed this issue

2026-05-05 18:09:26 +02:00

marcel referenced this issue from a commit

2026-05-05 18:09:28 +02:00

chore(deps): install eslint-plugin-boundaries and add boundary lint scripts

marcel referenced this issue from a commit

2026-05-05 18:09:29 +02:00

refactor(document): move statusDotClass and statusLabel to document domain

marcel referenced this issue from a commit