Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 1 addition & 20 deletions .claudinite-checks.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,7 @@
},
"tidy-repo",
"local/claudinite",
{
"id": "local/canon-curation",
"accept": [
{
"rule": "file-placement",
"path": ".claudinite/local/packs/canon-curation/",
"reason": "the canon home's curation local pack deliberately shares the checks/ engine lib (findings, lines, context) — the same named cross-cutting concern as the packs/ acceptance; in the home checkout the engine is the repo's own tracked tree, not a consumer mount, so the reach is inherent"
}
]
},
"local/canon-curation",
{
"id": "barriers",
"answers": {
Expand Down Expand Up @@ -99,16 +90,6 @@
}
],
"accept": [
{
"rule": "file-placement",
"path": "packs/",
"reason": "pack check modules deliberately share the checks/ engine lib — a named cross-cutting concern per skills/file-placement/SKILL.md; packs are organized by pack, not by dependency"
},
{
"rule": "file-placement",
"path": "skills/",
"reason": "a skill's check-the-work modules deliberately share the checks/ engine lib (findings.mjs) — the same named cross-cutting concern as the packs/ acceptance; checks are co-located with the skill whose action they validate, not organized by dependency"
},
{
"rule": "reference-integrity",
"path": "docs/per-project-scheduling/",
Expand Down
65 changes: 65 additions & 0 deletions packs-tests/basics/file-placement.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { test } from 'node:test';
import assert from 'node:assert/strict';
import { makeRepo, cleanup } from '../../engine-tests/helpers.mjs';
import { buildContext } from '../../engine/checks/helpers/repo-context.mjs';
import rule from '../../packs/basics/file-placement.mjs';

const run = (files) => {
const root = makeRepo({ changed: files });
try { return rule.run(buildContext({ root, mode: 'all' })); } finally { cleanup(root); }
};

const imports = (spec) => `import { finding } from '${spec}';\n`;

test('file-placement: a near reference yields no findings', () => {
assert.deepEqual(run({
'src/app/main.mjs': imports('./helper.mjs') + imports('../shared.mjs'),
}), []);
});

test('file-placement: flags a distance-3+ reach', () => {
const f = run({ 'src/app/ui/main.mjs': imports('../../../lib/deep/thing.mjs') });
assert.equal(f.length, 1);
assert.match(f[0].what, /references \.\.\/\.\.\/\.\.\/lib\/deep\/thing\.mjs at distance 5/);
});

// The plugin-contract exemption: both ends of this reference are fixed by the pack
// contract, so every pack module takes it and no project can shorten it.
test('file-placement: exempts a canon pack rule reaching the engine surface', () => {
assert.deepEqual(run({
'packs/mypack/my-rule.mjs':
imports('../../engine/checks/helpers/findings.mjs') +
imports('../../engine/scheduler/task-contract.mjs'),
}), []);
});

test('file-placement: exempts a local pack rule reaching the vendored engine', () => {
assert.deepEqual(run({
'.claudinite/local/packs/tldr/my-rule.mjs':
imports('../../../shared/engine/checks/helpers/minimal-yaml.mjs'),
}), []);
});

test('file-placement: exempts a pack skill\'s check module too', () => {
assert.deepEqual(run({
'packs/mypack/skills/myskill/my-rule.mjs':
imports('../../../../engine/checks/helpers/line-scanning.mjs'),
}), []);
});

// Narrowness: the exemption is the pack -> engine-surface direction only.
test('file-placement: still flags a pack reaching deep into its own subtree', () => {
const f = run({ 'packs/mypack/my-rule.mjs': imports('./skills/myskill/lib/deep.mjs') });
assert.equal(f.length, 1);
assert.match(f[0].what, /at distance 3/);
});

test('file-placement: still flags a pack reaching into another pack', () => {
const f = run({ 'packs/mypack/my-rule.mjs': imports('../other/deep/thing.mjs') });
assert.equal(f.length, 1);
});

test('file-placement: still flags non-pack code reaching the engine surface', () => {
const f = run({ 'tools/report/emit.mjs': imports('../../engine/checks/helpers/findings.mjs') });
assert.equal(f.length, 1);
});
29 changes: 27 additions & 2 deletions packs/basics/file-placement.mjs
Original file line number Diff line number Diff line change
@@ -1,15 +1,39 @@
import { dirname, join, normalize } from 'node:path';
import { dirname, join, normalize, sep } from 'node:path';
import { finding } from '../../engine/checks/helpers/findings.mjs';
import { engineSurface } from '../../engine/checks/helpers/module-imports.mjs';
import { SHARED_SUBDIR } from '../../engine/pack_loader/pack-registry.mjs';

const CODE_EXT = /\.(mjs|cjs|jsx?|tsx?)$/;
const CODE_REF = /(?:from\s+|require\(\s*|import\(\s*)['"](\.{1,2}\/[^'"]+)['"]/g;

// Mandated locations and test files are the doc's two exemptions: their long
// Mandated locations and test files are two of the doc's three exemptions: their long
// references are forced by a tool contract or a test-location convention.
const EXEMPT_SOURCE = (file) =>
file.startsWith('.github/') || file.startsWith('.claude/') ||
/(^|\/)(test|tests|__tests__|spec)\//.test(file) || /\.(test|spec)\./.test(file);

// The third exemption is the plugin-contract one, and unlike the two above it is a
// property of the reference, not of the source file: a pack module must import the
// engine surface to do its job at all (a rule needs `finding`, a task check needs the
// task contract), and neither end of that reference can move. The pack loader discovers
// packs at fixed paths (packs/, .claudinite/local/packs/), and the engine is vendored
// canon a consumer doesn't own to restructure. So the distance is fixed by the contract,
// not chosen: 4-5 from a pack in the canon home, 7 from a local pack beside a consumer's
// shared mount. Every pack module ever written takes it, which is what makes it
// structural rather than a placement to fix — a project can only ever waive it.
//
// `engineSurface` is the same allow list `pack-independence` enforces (packs may import
// their own files and this surface, nothing else), so "what a pack may import" and "what
// placement exempts" cannot drift apart. That leaves the rule's real work on packs
// intact: a reach into another pack, or deep into a pack's own subtree, is judged
// normally — as is ordinary code reaching *into* a pack, the same one-directional
// narrowness the mandated-location exemption has.
const PACK_MODULE = /^(?:packs|skills)\/|^\.claudinite\/(?:local\/packs|local_packs)\//;
const SHARED_PREFIX = `${SHARED_SUBDIR.split(sep).join('/')}/`;
const EXEMPT_REF = (file, resolved) => PACK_MODULE.test(file) && engineSurface(
resolved.startsWith(SHARED_PREFIX) ? resolved.slice(SHARED_PREFIX.length) : resolved,
);

function distance(fromDir, toDir) {
const a = fromDir === '.' ? [] : fromDir.split('/');
const b = toDir === '.' ? [] : toDir.split('/');
Expand Down Expand Up @@ -47,6 +71,7 @@ const rule = {
for (const { target, line } of refs) {
const resolved = normalize(join(dirname(file), target));
if (resolved.startsWith('..')) continue;
if (EXEMPT_REF(file, resolved)) continue;
const d = distance(dirname(file), dirname(resolved));
if (d >= 3) {
out.push(finding(rule, {
Expand Down
18 changes: 17 additions & 1 deletion packs/basics/skills/file-placement/SKILL.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: file-placement
description: Where a file should live — the reference-distance metric, the high-reach code smell, and the mandated-location and test-location exemptions. Use before placing, moving, or renaming a file, or when reviewing where one lives.
description: Where a file should live — the reference-distance metric, the high-reach code smell, and the mandated-location, test-location and plugin-contract exemptions. Use before placing, moving, or renaming a file, or when reviewing where one lives.
---

# File placement
Expand Down Expand Up @@ -79,6 +79,22 @@ Apply the same narrowness as the mandated-location case: the exemption covers th

**When picking a test-location convention from scratch, mirror the source tree — one test per source file at the same relative path** (`src/<area>/<name>` tested by `test/<area>/<name>.test`). The path *is* the link: a source file never has to name its own test in a comment, and a missing or misfiled test is obvious at a glance. Keep departures (whole-interaction tests, tests with no single source file to mirror) few and deliberate.

## Special case: a plugin contract's shared lib

The two exemptions above cover a file whose *own location* is fixed. A third case fixes **both ends of a reference**: a framework that discovers extension modules at a mandated path *and* publishes the shared lib those modules must import at another mandated path. Every extension takes the same import, at whatever distance the two mandated paths happen to sit apart — a plugin importing its host's plugin API, a check module importing the engine's finding constructor.

**Exempt that reference too.** It is not a placement decision at either end: the extension can't move without ceasing to be discovered, and the lib is usually code the project doesn't own to restructure. The tell is universality — *every* extension of that kind takes the reference, in every repo, so a project can only ever waive it, never fix it. A waiver each adopter has to write is a rule flagging a constant.

Apply the same narrowness as the other two:

- It covers the **extension → published lib** direction only. Code reaching *into* an extension's folder, or an extension reaching somewhere other than the published lib, is judged normally.
- It covers the framework's **declared** surface — the paths it tells extensions they may import — and nothing past it. Reaching around that boundary into internals is not exempt; that's the reach-deep-into-a-subtree smell, and the boundary the framework already drew is what makes it visible.
- **Universal, not merely common.** A dependency several files happen to share is the lift-to-a-common-ancestor case above, and it is still fixable. This exemption is for the one every extension takes by construction.

Take the surface from wherever the framework already defines it, rather than restating it. If some other rule enforces "an extension may import only X," that same X is what placement should exempt — one definition, so the two can't drift apart and disagree about the same import.

In this corpus that means a pack module under `packs/` or `.claudinite/local/packs/` importing the engine surface (everything under `engine/` — `engineSurface()` in `engine/checks/helpers/module-imports.mjs`, the allow list the `pack-independence` barrier enforces; vendored under `.claudinite/shared/` in a consuming repo). The `basics/file-placement` check implements this exemption, so a pack rule's `findings.mjs` import needs no acceptance entry. A pack reaching into *another* pack, or deep into its own subtree, is judged normally.

## Tooling acts on paths: encode act-on-able distinctions structurally

A file's path is an interface not only for developers but for automated processes — build globs, linters, and access rules often act on a file by its location alone, without reading its content. When two kinds of file must be treated differently by tooling that reads only paths, make the distinction **structural**: give each kind its own folder so the path itself carries the signal.
Expand Down
Loading