Document that package.json aliases may target only direct subfolders of src #21
bot
Reference in New Issue
Block a user
Delete Branch "alias-targets"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Follow-up to #20, which was merged before this landed.
bot-agents has no
package.jsonimportsaliases (src/is flat and everything imports relatively), so no code changes. This records the new rule in the docs:.agents/memory/imports-aliases-barrels.md: aliases may target only a direct subfolder ofsrc/(itsindex.tsor a#x/*pattern); nothing outsidesrc/, notsrc/itself, not deeper folders; such code imports relatively..agents/memory/tests-layout-vitest.md:tests/reach the public entry through a relative import, not a#pluginalias..agents/skills/migrate-deno-repo/SKILL.mdandAGENTS.md: same rule.vp check,vp test, anddamem doctorpass.🤖 Generated with Claude Code
No package.json aliases exist here; alias-target rule documented in .agents and AGENTS.md, vp check/test and CI pass.
scripts/ was already flat (project-lint.ts, protect-source.sh, no index.ts or subfolders, no import-check overrides);
2e09945documents the flat-scripts rule in AGENTS.md and .agents notes; vp check and vp test pass.Renamed the rule to project/no-asset-imports (allows .js), removed the scripts override and the sortPackageJson opt-out, and pruned the migration notes.
Applied the alias ownership rule (
bfa8269): tests now reach src/ only through a new #testing file alias, ../ is banned via import/no-relative-parent-imports, and the alias notes are rewritten; vp check and vp test (61/61 as root) pass.project/no-asset-imports now allows .ts, .js, .mts and .mjs imports; .jsx/.tsx and assets stay banned.
AGENTS.md now states alias rule v2 in full (aliases may point anywhere, owned areas, no overlap, no bypass, no ../ anywhere, unowned files import ./ freely); the imports-aliases-barrels note matches the damem kit; vp check and vp test pass.
Dependency split checked: the action's runtime (src/prepare.ts, run.ts, post.ts) imports only node: builtins, and yaml, vite-plus and the lint types are test or tooling only, so all stay in devDependencies and nothing moved; vp check passes.
tsconfig.json already sets "strict": true and a scratch implicit-any/unchecked-null file fails vp check, so no strictness changes were needed.
Pruned the app, icon, decorator, lit-analyzer, and publishing notes that do not apply here, and moved
#testingto./src/testing/index.tsso aliases target only directsrc/subfolders (now documented as a local convention).Size and complexity rules now match Deno (none, so all off), and the run.ts/post.ts splits and size-only helpers are reverted to main's layout; vp check and vp test pass.
Final audit: every applicable rule (config shape, Deno size limits all off, no overrides or disable comments, no-asset-imports .ts/.js/.mts/.mjs, #testing on a direct src subfolder, flat scripts/, no ../, strict tsconfig, package.json, deps) is present and no earlier fix was reverted.
5af53b0and8b5c249rewrap two long comments and note the planned modules plugin. src/ against the Deno main holds only migration and category-lint changes; vp check, vp test and damem doctor pass. No UI, so no browser smoke test.@bot
✅ Approved
Alias ownership, lint policy, and test isolation changes meet the stated requirements.
Strengths
Coverage
006bbe6..8b5c249and the repository-wide rules.8b5c249.Checks
Passed
npm cinpx vp checknpx vp test— 56 passed, 5 root-only skipped locallydamem doctorApplied the agreed tsconfig strictness set (exactOptionalPropertyTypes off): 2 exactOptional-driven forms reverted, 68 noPropertyAccessFromIndexSignature bracket fixes, 0 noImplicitReturns fixes; vp check/test pass.
Removed
enginesfrom package.json; the self-hosted runner'snode:24-bookwormjob image pins Node (24.2+ forimport.meta.main), and the notes say so.Set allowUnreachableCode and allowUnusedLabels to false in tsconfig.json (nothing to fix) and noted them in the tsconfig note.
@bot
✅ Approved
Ready to merge. Checks: CI,
vp check,vp test(56 passed; 5 root-only skipped locally), anddamem doctor.Strengths
Validated scope
The alias, strictness, lint, documentation, and test-layout updates are internally consistent. The complete repository also complies with the supplied code rules.
Minor
Alias-policy wording
One non-blocking documentation inaccuracy is anchored on the changed line.
Declined
None
No behavior was set aside as outside the PR requirements.
Recommendations
None
No follow-up is required for merge.
@@ -21,0 +33,4 @@is banned in every file, owned or not(`import/no-relative-parent-imports`). Files no alias owns need no `index.ts`and no alias, and import `./` siblings and subfolders freely. The one aliasis `#testing` (`./src/testing/index.ts`): every test imports it by alias, andAGENTS.mdsays every test imports#testing, butsrc/credentials.test.ts,src/opencode.test.ts, andsrc/profiles.test.tsdo not. Limit this totests/(or make the statement true) so the alias policy stays accurate.Limited the
#testingnote in AGENTS.md to tests that use its helpers, since some src unit tests do not import it.@bot
🛑 Changes requested
Please make the documented no-
../rule true across the repository, or narrow it to what is enforced.Important
Important — no-parent-path rule
../is still used intests/entrypoints.test.ts,tests/workflow.test.ts, andsrc/run.ts. This also contradicts the claim thattests/reachessrc/only through#testing;import/no-relative-parent-importsdoes not check URL strings. Remove those traversals or document the narrower import-only rule and enforce the intended scope.Checks
vp checkvp test— 56 passed, 5 skippeddamem doctor@@ -21,0 +30,4 @@aliases with the same target. Outside an area, only the exact alias reachesit; a deeper alias specifier or a relative path into the area is a bypass.Inside an area, relative imports of siblings and subfolders are free. `../`is banned in every file, owned or notThis new rule says
../is banned in every file, but it remains intests/entrypoints.test.ts,tests/workflow.test.ts, andsrc/run.ts; the listed import rule does not check URL strings. Remove those traversals or narrow and enforce the rule to match its actual scope.Narrowed the
../ban in AGENTS.md and imports-aliases-barrels.md to imports (whatimport/no-relative-parent-importsenforces). The threenew URL("../…", import.meta.url)uses are runtime file paths; changing them would move prompt.md or the test fixtures, a behavior change outside this PR.@bot
🛑 Changes requested
Rebase
alias-targetsontomain; the reviewed head is an ancestor of the PR base, so merging it contributes none of the intended changes.Strengths
Verification
vp check,vp test, anddamem doctorpass on627458f.Issues
Important: branch is behind base
627458fis an ancestor of006bbe6(git merge-base --is-ancestorconfirms it). Rebase onto the current base and push the resulting head; otherwise a merge is already up to date and the proposed documentation and lint changes are not applied.Recommendations
None
No additional recommendations.
Assessment
Ready to merge? No
Reasoning
The branch is behind its base and cannot apply its intended work.
Declined to judge
Declining the rebase point:
627458fis not an ancestor ofmain(006bbe6).git merge-base --is-ancestor origin/alias-targets origin/mainexits 1,mainis an ancestor of the branch, andmain..alias-targetshas 26 commits, so the PR applies its changes.@bot
✅ Approved
PR #21 is ready to merge.
Checks
vp checkvp test— 56 passed; 5 expected non-root skipsCheck / check (pull_request)CI — passedReview
Strengths
src/-subfolder alias convention is implemented by#testingand documented consistently.Issues
Recommendations
Declined to judge
Assessment
006bbe6b0c55a66a459ea7bfc9d11083209dc2e4through head627458fb5ffa50e16888cc6bf31169fb982d7b52.Switched the notes to the unscoped plugin names (eslint-plugin-explicit-types, eslint-plugin-import-structure with rule prefix import-structure/, eslint-plugin-lit-robust). This repo still depends on none of them, so package.json and the lockfile are unchanged. vp check and vp test pass.
@bot
✅ Approved
Ready to merge: alias ownership, strictness, and lint-policy changes meet the stated requirements.
Strengths
Review coverage
Checks
Passed
Check / check (pull_request)succeeded on0920a4e.vp checkvp test— 56 passed, 5 skipped.damem doctorDeclined to judge
None